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

[Anton Khirnov]

Quoting Andreas Rheinhardt (2021-02-04 12:06:15)
> 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.)

I don't think we have an official project policy on this, but I would
be in favor of something like:

    All installed headers are only guaranteed to provide those
    identifiers that are explicitly declared in them. Users must not
    rely on an installed header #include'ing any other specific headers,
    as those can change at any time.

+exceptions for av*.h, but ideally I'd remove those too in the long
run - people should just include what they need and avoid monsterheaders

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

I am in favor of moving MKTAG if you can think of a good place for it.
(also it should be properly namespaced, but not necessarily now)

-- 
Anton Khirnov