[FFmpeg-devel] Moving if(constant expression) to preprocessor?

[Axel Holzinger]

M?ns Rullg?rd wrote:
> Sent: Sunday, September 19, 2010 7:00 PM
> To: FFmpeg development discussions and patches
> Subject: Re: [FFmpeg-devel] Moving if(constant expression) to 
> preprocessor?
> 
> "Axel Holzinger" <aholzinger at gmx.de> writes:
> 
> > M?ns Rullg?rd and nice are mutually exclusive. Does this help?
> 
> Please refrain from ad hominem attacks.

That was just a replique on your attack on boost, so forget it.

> >> > I got inspired by their BOOST_PP_IF macro.
> >> >
> >> > It's more generic as it would be needed here. So reduced 
> to what's 
> >> > needed it could look like this (compiler specific #fdefs
removed 
> >> > for
> >> > clarity):
> >> 
> >> What compiler-specifics would be involved here?  FFmpeg uses only

> >> standard C99 features in non-optional code.
> 
> You did not answer this question.

None, not necessary.

> > Here is the code that works and that's tested with gcc:
> >
> > #define AV_COND_IIF(bit, t, f) AV_COND_IIF_I(bit, t, f) #define 
> > AV_COND_IIF_I(bit, t, f) AV_COND_IIF_II(AV_COND_IIF_ ## bit(t, f))

> > #define AV_COND_IIF_II(id) id #define AV_COND_IIF_0(t, f) f
#define 
> > AV_COND_IIF_1(t, f) t #define AV_COND_BOOL(x) AV_COND_BOOL_I(x) 
> > #define AV_COND_BOOL_I(x) AV_COND_BOOL_ ## x #define 
> AV_COND_BOOL_0 0 
> > #define AV_COND_BOOL_1 1 #define AV_COND_IF(cond, t, f) 
> > AV_COND_IIF(AV_COND_BOOL(cond), t, f)
> 
> That is quite a bit more obfuscated than it needs to be.  The 
> following actually works:
> 
> $ cat foo.c
> #define AV_IF_0(t, f) f
> #define AV_IF_1(t, f) t
> #define AV_IF(c, t, f)  AV_IF2(c, t, f)
> #define AV_IF2(c, t, f) AV_IF_##c(t, f)

Smaller and simpler, perfect. I'm fine with that and works for me.

> #define REGISTER_THING(X, x) { extern int x##_thing; 
> AV_IF(CONFIG_##X, register_thing(&x##_thing),); }
> 
> #define CONFIG_FOO 1
> #define CONFIG_BAR 0
> 
> void init(void) {
>     REGISTER_THING(FOO, foo);
>     REGISTER_THING(BAR, bar);
> }
> $ gcc -E foo.c
> void init(void) {
>     { extern int foo_thing; register_thing(&foo_thing); };
>     { extern int bar_thing; ; };
> }
> $
> 
> Even so, there is no good reason to change the code.  We have 
> already told you why.
> 
> > Usage:
> > Before:
> > #define REGISTER_HWACCEL(X,x) { \
> >           extern AVHWAccel x##_hwaccel; \
> >           if(CONFIG_##X##_HWACCEL) 
> av_register_hwaccel(&x##_hwaccel);
> > }
> >
> > With the macro:
> > #define REGISTER_HWACCEL(X,x) { \
> >           extern AVHWAccel x##_hwaccel; \
> >           AV_COND_IF(CONFIG_##X##_HWACCEL,
> > (av_register_hwaccel(&x##_hwaccel)),) ; }
> >
> >> However, it is far more convoluted than the current code
> >
> > What?
> 
> 10 obfuscated macros are more convoluted than none.

There are a lot more multiline obfuscated macros already in FFmpeg.
The singleline ones aren't that complicated. And you even got it more
simple. So go ahead and use them.

> >> for no gain,
> >
> > It makes it possible to completely disable optimisations.
> 
> I do not consider that a gain.

Isn't having the oportunity to have the choice a gain?

> > To date the configure script offers to disable optimisations, but 
> > doesn't do so.  One could call this a bug, at least it relies on 
> > compiler specifics that are not mandated by C99.
> 
> We rely on a lot of things outside the scope of C99, for 
> example linkers.

Come on, that's silly. Relying on dead code elimination is really
obfuscating, because you can't understand from reading the code that
this only works with optimisation. Having a conditional preprocessor
if makes it much more clear, that the code will not be compiled, if
the condition is not met.

> >> and Reimar's comments are valid too.
> >
> > I think Reimar's comments are beside the point. The only thing the

> > compiler does, if a runtime if is used instead of a macro, 
> > is that it checks the syntax of the call, nothing else.
> 
> I consider it a good thing having the syntax checked.

It's far more probable that a potential issue is inside a function
call than in the function call itself, whereas most of the function
calls we're talking about (the ones in the register macros) don't have
parameters at all. So I still tend to think that this not really an
argument and is beside the point.

Axel