[FFmpeg-devel] [PATCH] MOV YUV2 fourcc -> PIX_FMT_YUYV422 mapping

Reimar Döffinger Reimar.Doeffinger
Thu Apr 2 14:21:13 CEST 2009


On Thu, Apr 02, 2009 at 05:41:14PM +0530, Jai Menon wrote:
> On 4/2/09, Reimar D?ffinger <Reimar.Doeffinger at gmx.de> wrote:
> >  > @@ -40,8 +41,16 @@
> >
> > >  static int raw_encode(AVCodecContext *avctx,
> >  >                              unsigned char *frame, int buf_size, void *data)
> >  >  {
> >  > -    return avpicture_layout((AVPicture *)data, avctx->pix_fmt, avctx->width,
> >  > +    int ret = avpicture_layout((AVPicture *)data, avctx->pix_fmt, avctx->width,
> >  >                                                 avctx->height, frame, buf_size);
> >  > +
> >  > +    if(avctx->codec_tag == AV_RL32("yuv2") && ret > 0 &&
> >  > +       avctx->pix_fmt   == PIX_FMT_YUYV422) {
> >
> > > +        int x;
> >  > +        for(x = 1; x < avctx->height*avctx->width*2; x += 2)
> >  > +            frame[x] ^= 0x80;
> >  > +    }
> >
> >  Hm, avctx->height*avctx->width*2 just recalculates the size of the
> >  encoded frame, but ret already contains that.
> >  Wouldn't it be better to just use "ret" then?
> 
> The readability of that is questionable :) , especially for a fringe
> fourcc like this. But I could change it everybody likes it this
> way....

Is the readability still questionable if you write
int frame_size = avpicture_layout(...);
if (frame_size > 0 &&
    avctx->codec_tag == AV_RL32("yuv2") &&
    avctx->pix_fmt   == PIX_FMT_YUYV422) {
    int x;
    for(x = 1; x < frame_size; x += 2) frame[x] ^= 0x80;
}

?
Writing avctx->height*avctx->width*2 duplicates knowledge that exist
already in avpicture_layout, and the returned values is already checked
to not cause buffer overflows and might in the future e.g. be fixed to do
something sensible for odd "width" values - whoever does this is very
unlikely to remember to change this code as well.



More information about the ffmpeg-devel mailing list