[Ffmpeg-devel] [PATCH] video4linux2 input
Diego Biurrun
diego
Sun Feb 5 16:12:01 CET 2006
On Sun, Feb 05, 2006 at 05:00:29PM +0100, Aurelien Jacobs wrote:
> On Sun, 5 Feb 2006 15:41:53 +0100
> Diego Biurrun <diego at biurrun.de> wrote:
>
> > On Wed, Feb 01, 2006 at 03:15:02PM +0100, Michael Niedermayer wrote:
> > >
> > > 3. simply unneeded comments which make reading more difficult:
> > >
> > > h261.c: // QCIF
> > > h261.c- if (width == 176 && height == 144)
> > > h261.c- return 0;
> > > h261.c: // CIF
> > > h261.c- else if (width == 352 && height == 288)
> > > h261.c- return 1;
> > > h261.c: // ERROR
> > > h261.c- else
> > > h261.c- return -1;
> >
> > The last comment is redundant but I would disagree about the other two,
> > not everybody remembers CIF/QCIF resolutions offhand... IMO this is a
> > good example of how two small comments make the code more readable. If
> > you care about vertical space you can put them at the end of the line...
>
> I disagree.
> It should probably read something like this:
>
> typedef enum {
> QCIF = 0,
> CIF = 1,
> } res_desc_t;
>
> if (width == 176 && height == 144)
> return QCIF;
> else if (width == 352 && height == 288)
> return CIF;
> else
> return -1;
I think this is orthogonal to what I wrote, your solution is more
elegant, but without the enum I believe the comments help.
Diego
More information about the ffmpeg-devel
mailing list