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

[Michael Niedermayer]

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

[...]

-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

When you are offended at any man's fault, turn to yourself and study your
own failings. Then you will forget your anger. -- Epictetus
-------------- 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/1d210f1a/attachment.asc>