[FFmpeg-devel] [PATCH] libopenjpegdec: respect JP2 color space, fix ticket 1179

[Michael Niedermayer]

On Thu, May 03, 2012 at 12:30:00PM -0600, Michael Bradshaw wrote:
> On Thu, May 3, 2012 at 10:05 AM, Michael Niedermayer <michaelni at gmx.at> wrote:
> > On Thu, May 03, 2012 at 06:57:34AM -0600, Michael Bradshaw wrote:
> >> On Thu, May 3, 2012 at 6:08 AM, Michael Niedermayer <michaelni at gmx.at> wrote:
> >> > On Wed, May 02, 2012 at 12:20:25PM -0600, Michael Bradshaw wrote:
> >> >> Attached patch fixes ticket 1179 for me (mostly*) by respecting the
> >> >> JP2 color space, if it is set. It also adds support for yuv444p. It
> >> >> still has to guess the color space for J2K frames.
> >> >>
> >> >> Feel free to be nitpicky about the patch; for example there were times
> >> >> I debated between a switch and a for with an if, trying do what is
> >> >> consistent with the rest of ffmpeg, and it's possible I may done
> >> >> something inconsistent.
> >> >>
> >> >> *I say mostly because it fixes it for JP2 frames, but J2K frames still
> >> >> have the potential to be guessed incorrectly (i.e. yuv444p and rgb24
> >> >> can get mixed up). However, it is not possible, as far as I know, to
> >> >> correctly guess between yuv444p and rgb24 in a J2K frame without
> >> >> hinting from the user, so this patch fixes it as much as can be
> >> >> automatically guessed. I tried passing a "-pix_fmt yuv444p" as an
> >> >> input option to see if the user could hint the input pixel format, but
> >> >> ffmpeg rejected it ("Option pixel_format not found."). Should I add a
> >> >> "-pix_fmt" option for the decoder so the user can hint the input pixel
> >> >> format?
> >> > [...]
> >> >> -    switch (compRatio) {
> >> >> -    case 0111111: goto libopenjpeg_yuv444_rgb;
> >> >> -    case 0111212: return PIX_FMT_YUV440P;
> >> >> -    case 0112121: goto libopenjpeg_yuv422;
> >> >> -    case 0112222: goto libopenjpeg_yuv420;
> >> >> -    default: goto libopenjpeg_rgb;
> >> >> +    switch (descriptor.nb_components) {
> >> >> +    case 4: match = match && descriptor.comp[3].depth_minus1 + 1 == image->comps[3].prec &&
> >> >> +                             descriptor.log2_chroma_w + 1 == image->comps[3].dx &&
> >> >> +                             descriptor.log2_chroma_h + 1 == image->comps[3].dy;
> >> >> +    case 3: match = match && descriptor.comp[2].depth_minus1 + 1 == image->comps[2].prec &&
> >> >> +                             descriptor.log2_chroma_w + 1 == image->comps[2].dx &&
> >> >> +                             descriptor.log2_chroma_h + 1 == image->comps[2].dy;
> >> >> +    case 2: match = match && descriptor.comp[1].depth_minus1 + 1 == image->comps[1].prec &&
> >> >> +                             descriptor.log2_chroma_w + 1 == image->comps[1].dx &&
> >> >> +                             descriptor.log2_chroma_h + 1 == image->comps[1].dy;
> >> >> +    case 1: match = match && descriptor.comp[0].depth_minus1 + 1 == image->comps[0].prec;
> >> >
> >> >> +                             1 == image->comps[0].dx &&
> >> >> +                             1 == image->comps[0].dy;
> >> >
> >> > this looks a bit odd, i dont think this does what you intended it to
> >> > do
> >>
> >> May I ask which part you're referring to? The whole switch statement
> >
> >
> > Sorry for the slow awnser, i refered to the ; that possibly was meant
> > to be a &&
> 
> Oh wow, I totally missed that one. Yes, it was supposed to be a &&.
> Thanks. Attached patch fixes that, as well as left-shifting
> log2_chroma instead of adding it.

>  libopenjpegdec.c |  135 ++++++++++++++++++++++++++++++-------------------------
>  1 file changed, 74 insertions(+), 61 deletions(-)
> ec2e04707d23e088edc3c98f3a2a51dc30e1d311  0001-libopenjpegdec-respect-JP2-color-space-fix-ticket-11.patch
> From 74035d5a1e8a91258071986e5699cd86369916d4 Mon Sep 17 00:00:00 2001
> From: Michael Bradshaw <mbradshaw at sorensonmedia.com>
> Date: Thu, 3 May 2012 12:27:04 -0600
> Subject: [PATCH] libopenjpegdec: respect JP2 color space, fix ticket 1179

should be ok if it has been tested with a representative set of jp2
images

[...]
-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

Complexity theory is the science of finding the exact solution to an
approximation. Benchmarking OTOH is finding an approximation of the exact
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 198 bytes
Desc: Digital signature
URL: <http://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20120503/4563da7c/attachment.asc>