[FFmpeg-cvslog] r22997 - trunk/libavcodec/rawdec.c

[Reimar Döffinger]

On Sat, May 01, 2010 at 01:44:02PM +0200, Michael Niedermayer wrote:
> On Sat, May 01, 2010 at 09:10:03AM +0200, Reimar D?ffinger wrote:
> > On Sat, May 01, 2010 at 12:09:42AM +0200, michael wrote:
> > > Author: michael
> > > Date: Sat May  1 00:09:42 2010
> > > New Revision: 22997
> > > 
> > > Log:
> > > avi bgr24 padding fix.
> > > Fixes issue1901
> > > 
> > > Modified:
> > >    trunk/libavcodec/rawdec.c
> > > 
> > > Modified: trunk/libavcodec/rawdec.c
> > > ==============================================================================
> > > --- trunk/libavcodec/rawdec.c	Fri Apr 30 23:43:29 2010	(r22996)
> > > +++ trunk/libavcodec/rawdec.c	Sat May  1 00:09:42 2010	(r22997)
> > > @@ -152,6 +152,8 @@ static int raw_decode(AVCodecContext *av
> > >          memcpy(frame->data[1], avctx->palctrl->palette, AVPALETTE_SIZE);
> > >          avctx->palctrl->palette_changed = 0;
> > >      }
> > > +    if(avctx->pix_fmt==PIX_FMT_BGR24 && ((frame->linesize[0]+3)&~3)*avctx->height <= buf_size)
> > > +        frame->linesize[0] = (frame->linesize[0]+3)&~3;
> > 
> > Nice hack :-), but shouldn't it use FFALIGN?
> 
> iam all for factorizng code, but when the call is biger than the code
> itself iam not sure. As is anyone knowing C should understand the code
> with FFALIGN() the person must understand what FFALIGN does as well

Size is not the only criteria for readability, and I consider FFALIGN better
there than (+)&~ where in addition I have to check that the numbers are right
and () at the right place because else it'll go completely wrong.
Either way using FFALIGN in one half and not using it in the other certainly is
the worst possible way one could do it.