[FFmpeg-devel] [PATCH] change semantic of CONFIG_*

[Aurelien Jacobs]

Diego Biurrun wrote:

> On Tue, Jan 13, 2009 at 12:01:43PM +0100, Aurelien Jacobs wrote:
> > 
> > Attached patch replaces each occurrence of #ifdef CONFIG_ by #if CONFIG_
> > and ensure CONFIG_* are always defined to 0/1.
> > I guess this one should please everyone.
> 
> This is much better, yes.
> 
> > --- libavcodec/vcr1.c	(revision 16570)
> > +++ libavcodec/vcr1.c	(working copy)
> > @@ -32,6 +32,7 @@
> >  
> >  /* Disable the encoder. */
> >  #undef CONFIG_VCR1_ENCODER
> > +#define CONFIG_VCR1_ENCODER 0
> 
> This looks like a hack that should be eliminated.
> No, I'm not saying it's your fault but now that we have noticed
> it, we should find a better solution.

That's right, but unrelated to this patch.
IIRC, there is almost nothing more than a few empty funcs in this
encoder, so it may be best to totally remove the "code".

> > --- libavcodec/libfaad.c	(revision 16570)
> > +++ libavcodec/libfaad.c	(working copy)
> > @@ -35,12 +35,13 @@
> >  
> >  /*
> > - * when CONFIG_LIBFAADBIN is defined the libfaad will be opened at runtime
> > + * when CONFIG_LIBFAADBIN is true the libfaad will be opened at runtime
> 
> s/the// while you're at it..

Done.

> > -//#undef CONFIG_LIBFAADBIN
> > +//#undef  CONFIG_LIBFAADBIN
> 
> IMO pointless, but no, I do not care..

Right. Fixed.

> > --- libavcodec/mpegaudiodec.c	(revision 16570)
> > +++ libavcodec/mpegaudiodec.c	(working copy)
> > @@ -36,7 +36,7 @@
> >  
> >  /* define USE_HIGHPRECISION to have a bit exact (but slower) mpeg
> >     audio decoder */
> > -#ifdef CONFIG_MPEGAUDIO_HP
> > +#if CONFIG_MPEGAUDIO_HP
> >  #   define USE_HIGHPRECISION
> >  #endif
> 
> This is another hack that should be avoided: The indirection is
> pointless.

Right, but unrelated to this patch.

> > --- libavcodec/msmpeg4.c	(revision 16570)
> > +++ libavcodec/msmpeg4.c	(working copy)
> > @@ -79,7 +79,7 @@
> >  
> > -#ifdef CONFIG_ENCODERS //strangely gcc includes this even if it is not references
> > +#if CONFIG_ENCODERS //strangely gcc includes this even if it is not references
> 
> referenceD, while you're at it..

Fixed.

> > @@ -109,7 +109,7 @@
> > -#if defined(CONFIG_WMV3_DECODER)||defined(CONFIG_VC1_DECODER)
> > +#if CONFIG_WMV3_DECODER||CONFIG_VC1_DECODER
> 
> Add spaces around the ||.

Done.

> > --- cmdutils.c	(revision 16570)
> > +++ cmdutils.c	(working copy)
> > @@ -30,19 +30,19 @@
> >  
> >  #include "config.h"
> >  #include "libavformat/avformat.h"
> > -#ifdef CONFIG_AVFILTER
> > +#if CONFIG_AVFILTER
> >  #include "libavfilter/avfilter.h"
> >  #endif
> >  #include "libavdevice/avdevice.h"
> >  #include "libswscale/swscale.h"
> > -#ifdef CONFIG_POSTPROC
> > +#if CONFIG_POSTPROC
> >  #include "libpostproc/postprocess.h"
> >  #endif
> >  #include "libavutil/avstring.h"
> >  #include "libavcodec/opt.h"
> >  #include "cmdutils.h"
> >  #include "version.h"
> > -#ifdef CONFIG_NETWORK
> > +#if CONFIG_NETWORK
> >  #include "libavformat/network.h"
> >  #endif
> 
> These #ifs look pointless, I think we can drop them.

Maybe, yes. But also unrelated to this patch.

With such big patch, I prefer avoiding any unnecessary unrelated
changes, even when it looks mostly safe.
If one of those unrelated change happen to cause trouble in the
future, it's much harder to find out the source of the problem
if it's hidden inside a huge commit.

Aurel