[FFmpeg-devel] [PATCH] yuv pixel formats support in openjpeg decoder + 10bit support

[Michael Niedermayer]

On Fri, Nov 11, 2011 at 03:31:50AM -0800, Alex Zhukov wrote:
> On Thu, Nov 10, 2011 at 5:20 PM, Michael Niedermayer <michaelni at gmx.at> wrote:
> > On Thu, Nov 10, 2011 at 02:38:15PM -0800, Alex Zhukov wrote:
> >> added yuv pixel formats support to libopenjpeg decoder
> >> supports 8/10bit yuv420/422
> >>
> >> sample 10bit yuv j2k can be found at
> >> http://dl.dropbox.com/u/1109725/10bit_yuv_j2k.mxf
> >> note that the sample was cut with dd to be 10MB so last frame is broken
> >
> > your code breaks rgb24 j2k
> 
> code path for rgb24 would be:
> compRatio = 0111111 //all components of equal size
> 
> libopenjpeg_yuv444_rgb: //it can be either yuv444 or rgb
> switch (c0.prec)
> case 8: return PIX_FMT_RGB24; // if precision is 8bpp assume it's rgb24
> 
> why would it break rgb24 j2k?

because you remove the code that supports rgb24:
     }
-
-    for(y = 0; y < avctx->height; y++) {
-        index = y*avctx->width;
-        img_ptr = picture->data[0] + y*picture->linesize[0];
-        for(x = 0; x < avctx->width; x++, index++) {
-            *img_ptr++ = image->comps[0].data[index] >> adjust[0];
-            if(image->numcomps > 2 && check_image_attributes(image)) {
-                *img_ptr++ = image->comps[1].data[index] >> adjust[1];
-                *img_ptr++ = image->comps[2].data[index] >> adjust[2];
-                if(has_alpha)
-                    *img_ptr++ = image->comps[3].data[index] >> adjust[3];
-            }
-        }
-    }
-
+

also i tried yesterday:
./ffmpeg -i matrixbench_mpeg2.mpg -vcodec j2k -strict experimental test.avi

and tried to decode with and without your patch with ffplay, it failed
with your patch but worked without


> 
> >
> > [...]
> >> @@ -39,14 +40,80 @@ typedef struct {
> >>      AVFrame image;
> >>  } LibOpenJPEGContext;
> >>
> >> -static int check_image_attributes(opj_image_t *image)
> >> +static enum PixelFormat check_image_attributes(AVCodecContext *avctx, opj_image_t *image)
> >>  {
> >> -    return image->comps[0].dx == image->comps[1].dx &&
> >> -           image->comps[1].dx == image->comps[2].dx &&
> >> -           image->comps[0].dy == image->comps[1].dy &&
> >> -           image->comps[1].dy == image->comps[2].dy &&
> >> -           image->comps[0].prec == image->comps[1].prec &&
> >> -           image->comps[1].prec == image->comps[2].prec;
> >> +    opj_image_comp_t c0 = image->comps[0];
> >> +    opj_image_comp_t c1 = image->comps[1];
> >> +    opj_image_comp_t c2 = image->comps[2];
> >
> >> +    int compRatio = c0.dx << 15 | c0.dy << 12;
> >> +    compRatio |= c1.dx << 9 | c1.dy << 6;
> >> +    compRatio |= c2.dx << 3 | c2.dy << 0;
> >
> > can these fields have more than 3 bit ? if yes, then this should
> > allocate more space to them
> 
> well these are component size ratios
> i have not seen a pixel format where components will have any ratio
> other than 1/1 or 1/2
> that is not to say there isn't such one of course
> tell me if i'm wrong
> i think leaving it the way it is will serve all practical purposes

Its exploitable as it allows to write arbitrary data outside the
arrays as the wrong pixel format is selected, a too small buffer
allocated and then more data written in it.

[...]

-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

I know you won't believe me, but the highest form of Human Excellence is
to question oneself and others. -- Socrates
-------------- 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/20111111/4bbb7227/attachment.asc>