[FFmpeg-cvslog] r14484 - in trunk/libavcodec: audioconvert.c audioconvert.h

[Uoti Urpala]

On Fri, 2008-08-01 at 14:42 +0200, Michael Niedermayer wrote:
> On Fri, Aug 01, 2008 at 05:20:33AM +0300, Uoti Urpala wrote:
> > On Fri, 2008-08-01 at 03:27 +0200, Michael Niedermayer wrote:
> > > On Thu, Jul 31, 2008 at 05:00:40PM -0400, The Wanderer wrote:

> [...]

> > > Some more concrete examples
> > > avcodec.h uses int64_t, if it would #include inttypes.h it would be between
> > > unuseable to a PITA in any environment that does not have inttypes.h.
> > 
> > It does already include inttypes.h indirectly through libavutil headers.
> 
> interresting, how much time did i took you to find that out? How much do you
> think it might cause a average developer who debugs a problem related to it?

Not much time because I already thought it did and I just verified that.
I think it's usually not too hard to find out what gets included by
looking at precompiled source for example. That "average developer"
question depends on the details of the problem.

> > > The rule of including headers in headers is a absolute pain for some use
> > > cases to deal with. The advantage of this is purely "because its the right
> > > way".
> > 
> > There was a thread about this on mplayer-dev-eng not too long ago that
> > explained much more significant advantages. The main point is that it's
> > realistic to keep the #include statements in a .c or .h file (mostly) in
> > sync with what is needed _within that file_; 
> 
> really? then explain me why they are totally OUT of sync in libav*, since this
> new idiotic system was adopted?

Out of sync how?

> > it's not realistic to keep
> > them in sync with indirect dependencies, at least not without automated
> > tools that aren't used in practice.
> 
> It worked in mplayer under arpi without a single issue i can remember.
> 
> So all the pretty arguments stand in stark contrast to actual practice.

MPlayer files included lots of unnecessary headers after dependencies
changed.

If you include headers in .c files only then in practice you need to add
the indirectly needed headers by trial and error. This mostly produces a
result that works originally, but it's already somewhat inefficient,
especially if you don't immediately know which other header you should
include for a particular missing symbol. If there are conditional
dependencies then they're very likely to be missed if they aren't needed
on the developer's own system. The case when the indirect dependencies
of a header _change_ is a major problem if you don't handle them
directly in the header itself. You'd need to change every file where the
header ends up being included. Some of those files may not be compiled
on your system, and even if they are in case of removed dependencies
it's hard to know whether they're still indirectly needed by some
_other_ included header.

> > > The only reason why its not causing a problem is plain because its not
> > > done, our headers luckily do in general not include all the headers they
> > > somehow depend on.
> > 
> > I think they include more than you thought (I think pretty much
> > everything ends up including the <inttypes.h> you talked about above for
> > example). More like it doesn't cause much of a problem even though it IS
> > done.
> 
> So it doesnt concern you that even i as maintainer of libav* has lost track of
> which header includes which? Well i guess as long as "the right way" is
> followed everyone will be happy ...

There's usually no need to know exactly which headers end up being
included. If you do need to know it's not too hard to find out. If the
set of headers is conditional on configuration then that list might not
have everything that _could_ get included; but the alternative without
headers including each other would most likely be that those conditional
headers wouldn't be included even when needed, which is worse.

> > > > cetera. If a header needs something then it should provide that thing,
> > > > either by defining it directly or (more practically) by including
> > > > another header which already defines it. A header which fails to do so
> > > > is, IMO, broken.
> > > 
> > > Do you have a argument beyond "is broken" vs. "is the right way"?
> > 
> > See above.
> 
> above says that there was some thread on some mplayer ML, if thats all you
> have thats not much.

I did also give specific reasons, but going through all the same things
again a few months later is boring...