[FFmpeg-devel] [PATCH]VDPAU patch for VC1 decoding, round 1

[Aurelien Jacobs]

Michael Niedermayer wrote:

> On Thu, Jan 08, 2009 at 12:10:40AM +0000, M?ns Rullg?rd wrote:
> > Aurelien Jacobs <aurel at gnuage.org> writes:
> > 
> > > Diego Biurrun wrote:
> > >
> > >> On Wed, Jan 07, 2009 at 10:50:39PM +0100, Carl Eugen Hoyos wrote:
> > >> >
> > >> > --- libavcodec/vc1.c	(revision 16481)
> > >> > +++ libavcodec/vc1.c	(working copy)
> > >> > @@ -4317,3 +4338,35 @@
> > >> > +
> > >> > +#if ENABLE_WMV3_VDPAU_DECODER
> > >> 
> > >> Using ENABLE_ here is still wrong...
> > >
> > > I disagree. Using #if ENABLE_ is better than #ifdef CONFIG_.
> > > It is more readable, shorter, and more consistent especially
> > > in cases such as:
> > >   #if   ENABLE_A
> > >   #elif ENABLE_B
> > >   #endif
> > > or
> > >   #if ENABLE_A || ENABLE_B
> > >
> > > Moreover, #if gives a chance to the compiler to complain if
> > > there is a typo in the symbol name. With #ifdef, such a
> > > typo could stay unnoticed for a long time.
> > >
> > > Now I expect some answer about consistency because the code
> > > supposedly contains more occurence of #ifdef CONFIG_ than
> > > #if ENABLE_.
> > > The attached patch solves this by replacing all usage of
> > > CONFIG_ by ENABLE_.
> > > With this applied, we could also drop all CONFIG_ from
> > > config.h.
> > 
> > I object.  The CONFIG/HAVE distinction serves a purpose.  I never
> > liked the ENABLE ones stomping all over that in the first place.
> 
> what about changing
> #ifdef HAVE_THIS (un)defined
> to
> #if HAVE_THIS (0/1)

I fully support this.

> #ifdef CONFIG_THIS (un)defined
> to
> #if CONFIG_THIS (0/1)
> 
> and then replacing the ENABLE by CONFIG/HAVE as appropriate
> ?

Those days, I tend to prefer the ENABLE naming over the CONFIG one.
But I really have no strong opinion about this. If people strongly
prefer CONFIG, then go for CONFIG.
The only important point IMO is the 0/1 vs. (un)defined.

Aurel