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

[Alex Zhukov]

good point
will fix

On Fri, Nov 11, 2011 at 7:10 AM, Michael Niedermayer <michaelni at gmx.at> wrote:
> 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
>
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel at ffmpeg.org
> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>
>