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

[The Wanderer]

Michael Niedermayer wrote:

> On Sat, Aug 02, 2008 at 04:09:47AM +0200, Michael Niedermayer wrote:
> 
>> On Fri, Aug 01, 2008 at 09:20:46PM -0400, The Wanderer wrote:

>>> But he removed it from a .c file, not from a header. I suspect I
>>> believe that the two should not necessarily be treated the same
>>> way.
>>> 
>>> My thoughts on this are (apparently) still evolving, but I think
>>> the central question here revolves around whether a given header
>>> file is *guaranteed*, either by documentation (or specification,
>>> etc.) or as a side effect of its own requirements, to need a
>>> given other header file - i.e., will audioconvert.h never cease
>>> to need some symbol from avcodec.h and thus need to include it,
>>> or may it potentially some day not need any such symbol and
>>> consequently cease to include that header?
>> 
>> currently audioconvert.h needs the audio sample format enum and
>> audioconvert.c needs it too, and its provided by avcodec.h after a
>> 5 sec look, audioconvert.c also use av_free() and uint8_t at that
>> point one can start a long discussion if stdint.h and mem.h should
>> be included or avcodec.h or nothing or all 3.
> 
> after a little longer ;)
> 
> it also uses lrintf & snprintf thus also needs one way or another
> stdio.h and math.h with  _XOPEN_SOURCE >= 600 || _ISOC99_SOURCE
> defined before inclusion of math.h
> 
> so strict direct inclusion rule would require for audioconvert.c (145
> lines)
> #include "audioconvert.h" (the prototypes)
> #include "avcodec.h" (sample format enum)

Actually, because audioconvert.h needs that same enum, unless I'm 
misremembering my C these two lines would have to come in the reverse order.

Furthermore, I would say that because audioconvert.h will need that enum
for just as long as audioconvert.c does, the include to support the enum
should be in the header rather than in the .c file. (This means that if
the enum does ever become unneeded but other things remained necessary,
the include line would have to be moved from the header to the source
file, which some might view as unnecessary churn.)

> #include <stdio.h> (snprintf)
> #include "libavutil/mem.h" (av_free / av_malloc)
> #include <stdint.h> (uint8_t)
> #define _ISOC99_SOURCE
> #include <math.h> (lrintf)

The other things you list are more complicated. I think that strictly
speaking, my position would indeed require that all of them be listed
explicitly - *but* there is a way to avoid that. Nothing in my scheme
requires that the header which directly defines a particular symbol be
included directly; it is entirely acceptable to include any header which
is guaranteed to pull in that definition by some means (this is exactly
why including audioconvert.h would make it unnecessary to include
avcodec.h for the enum).

It would be possible to create a header file and say "this header file
is guaranteed to always provide all of these commonly-needed external
symbols" (and implement that by including various other headers), and
then just include that file. Indeed, I sort of thought that that was
exactly what common.h was for.

This would have the downside of pulling in all of those symbols for
things which need just a subset, but then that's a problem which applies
to any header intended to be used in multiple files.

I'm fairly sure there are problems with this which I haven't noticed; if
anyone would care to present them, I could try to knock them down.

-- 
       The Wanderer

Warning: Simply because I argue an issue does not mean I agree with any
side of it.

Secrecy is the beginning of tyranny.