[FFmpeg-devel] CONFIG_W64_DEMUXER and dead-code elimination

[Matt Oliver]

On 23 April 2016 at 23:46, wm4 <nfxjfg at googlemail.com> wrote:

> 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.
>

Fixing one case isnt really an option as there are many DCE cases
throughout ffmpeg that all have the same issues (as previously stated).
Currently it is not possible to build ffmpeg with msvc using a debug or lto
build as it will just result in dozens if not hundreds of linker errors
(depending on the compiler version and options used). And the configuration
options here dont really matter as there are fundamentally missing
functions (h264_vaapi etc.) that are not available on windows and so
generate link errors no matter what you do.

Obviously changing things would require a consensus from all the devs.
Personally I would like to see the DCE stuff removed as currently as stated
debug or lto builds will fail with msvc and these are both options that are
support by the configure script yet wont actually build without errors
until DCE is removed. Ive also seen similar issues when using icl as well
so although some may state there advantages to DCE it is fundamentally
breaking things on certain platforms so it should be decided what is more
important.