[FFmpeg-devel] CONFIG_W64_DEMUXER and dead-code elimination

[wm4]

On Sat, 23 Apr 2016 14:52:12 +0200
Reimar Döffinger <Reimar.Doeffinger at gmx.de> wrote:

> On 23.04.2016, at 13:21, wm4 <nfxjfg at googlemail.com> wrote:
> 
> > On Sat, 23 Apr 2016 01:16:31 +0200
> > Hendrik Leppkes <h.leppkes at gmail.com> wrote:
> >   
> >> On Sat, Apr 23, 2016 at 1:02 AM, Bruce Dawson
> >> <brucedawson-at-google.com at ffmpeg.org> wrote:  
> >>> I've noticed that when CONFIG_W64_DEMUXER is defined to zero that ffmpeg
> >>> compiles in a reference to ff_w64_guid_data but doesn't not link w64.o
> >>> (which defines that symbol).
> >>> 
> >>> This normally works because most optimizers discard the reference
> >>> to ff_w64_guid_data early enough to not cause a linker failure. However
> >>> this assumption means that /Od (debug, unoptimized) and /GL (Link Time Code
> >>> Generation - super optimized) builds with VC++ don't work.
> >>>   
> >> 
> >> We require dead code elimination to be available in all build modes,
> >> therefor such build settings that do not have it are not supported.
> >> This is not the only place that uses this, and its a design decision
> >> to rely on it, therefor we won't be accepting patches to change that
> >> at this time, sorry.  
> > 
> > Why not? Someone just volunteered to cleanup this non-sense, so why not
> > let them?  
> 
> Because it ends up with a horrible ifdef mess and also means that disabled parts will not even be checked for syntax.
> I.e. the reasons why we did it this way.

Pretty weak considering there are 3961 #ifs in the code (this excludes
header file inclusion guards). This is because we readily allow
disabling _any_ component because someone might not need it. We also
have a big ifdef mess to switch between enabling and disabling
deprecated APIs and their implementation or use within ffmpeg.

There's even --enable-random, which is supposed to help with finding
some of the configurations which don't even compile, out of
the practically infinite number possible configurations.

On the other hand, mere compilation failures on other architectures
(which this idiom is supposed to catch) are easily detected by the
numerous fate instances we run.

In conclusion, this change would be pretty harmless compared to the
ifdef mess we have for _way_ less legitimate use-cases.

The add to this, we don't even need to use #ifdefs. Some sort of
selection macro could achieve the same thing:

#define CONCAT(a, b) a ## b

#define VALUE0(a, b) a
#define VALUE1(a, b) b
#define TOK(v) CONCAT(VALUE, v)

#define SELECT(v, a, b) TOK(v)(a, b)

#define HAVE_SOMETHING 1

SELECT(HAVE_SOMETHING, yes, no)

So syntactically we could get something relatively elegant.

I also want to remind that the reliance on DCE makes it a nightmare
trying to compile an actually debugable ffmpeg binary.

Though I agree with nev's comment that we should make a conscious
decision to change this, rather than accepting random patches which
just make things inconsistent.

> > We may have been arguing that every compiler supports this level of
> > DCE, but experience has been clearly showing that this is not always the
> > case due to various reasons.  
> 
> I don't that was quite the argument. It is more a question if the rare cases are really worth the cost, especially when also the compilers could be changed to support this method of code configuration.

The compilers have no reason to change here, as ffmpeg essentially
violates the C standard with this idiom.