[FFmpeg-devel] [PATCH 1/4] Remove unnecessary mem.h inclusions

[Andreas Rheinhardt]

Anton Khirnov:
> Quoting Andreas Rheinhardt (2021-02-04 01:05:05)
>> Thanks for the report. I have only checked for whether the relevant
>> translation unit uses any of the alloc/free functions (because
>> DECLARE_ALIGNED is already provided by mem_internal.h), but I have not
>> taken into account stuff that is included by the headers included by
>> mem.h. In this case, one needs string.h (which is included in
>> libavutil/common.h which in turn is included in avutil.h which is
>> included in mem.h). Will look over everything again.
>>
>> - Andreas
>>
>> PS: It actually seems that the only thing provided by avutil.h that
>> mem.h uses is size_t. Do we allow to remove included headers from
>> installed headers at major version bumps?
> 
> I would say yes, IIRC it has happened before.
> Certainly nobody should depend on our headers to provide any random
> system headers.
> 
Removing avutil.h affects not only system headers, but which of our
headers are included as it includes
#include "common.h"
#include "error.h"
#include "rational.h"
#include "version.h"
#include "macros.h"
#include "mathematics.h"
#include "log.h"
#include "pixfmt.h"
(And removing avutil.h will also remove avutil.h, of course.)

Other installed headers include too much, too: audio_fifo.h includes
avutil.h and fifo.h, but needs only attributes.h; eval.h unnecessarily
includes avutil.h; display.h includes common.h without needing it.
fifo.h includes avutil.h and attributes.h, but needs only stddef.h.
Several other headers include version.h despite the relevant FF_API
check for which it has been included no longer existing. (If we don't
remove version.h, sooner or later all headers will contain it.)

And while I played around a bit with this night, I found something odd:
libavutil/error.h is broken: It relies on MKTAG without providing the
header for it. MKTAG is in common.h which also includes mem.h which uses
an AVERROR code in an inline function. So the simple solution of
including common.h at the beginning of error.h doesn't work, as
AVERROR(EINVAL) will still be undefined in said function. I see two
other solutions: Move MKTAG (and probably a bit other stuff) to a new
header and include that in error.h; include common.h, but not at the
beginning of error.h, but only after the definition of AVERROR. Given
that I dislike monster headers like common.h, I prefer the first approach.
(I already mentioned this on IRC, but those who only follow the ML
should be informed, too.)

- Andreas