[FFmpeg-devel] [PATCH 6/8] avcodec/dump_extradata_bsf: Remove unnecessary header

[Andreas Rheinhardt]

Anton Khirnov:
> Quoting Andreas Rheinhardt (2020-04-21 04:31:51)
>> Since 33d18982fa03feb061c8f744a4f0a9175c1f63ab (the commit that
>> introduced the new bsf API) allocating an enlarged buffer in case
>> extradata needs to be added to a packet is done via av_new_packet(),
>> so that libavutil/mem.h is no longer needed.
>>
>> Furthermore, remove libavutil/log.h. This function uses something
>> provided by it (an AVClass), yet it does so only for AVOptions, not
>> for logging purposes (there is no av_log() here), so only including
>> libavutil/opt.h seems appropriate.
> 
> IMO each file should include the headers for all the definitions it uses
> directly. I.e. if a file uses AVClass, it should include log.h.
> 
My test for inclusion is more like "What API does this file use and
which header provides it?" In this case: This bsf only uses the AVOption
API, not the logging API. That the former is built on top of a structure
that is also used by the latter (and that happens to be provided by the
header for the latter) does not imply (for me) that one should include
the header for the latter.

Another example where our opinions will likely differ is the following:
The muxing/demuxing APIs use AVPackets a lot and for this reason
avformat.h includes enough so that AVPackets are directly usable for
everyone including avformat.h. That's fine by me (although I'd really
like to see the amount of stuff unnecessarily included (it currently
includes avcodec.h) reduced), but not for you I presume. Your approach
would instead include packet.h (and stdint.h) almost everywhere.

Furthermore, what counts as "use" for you (and others, of course)? Right
now we have a lot of dependencies among headers and lots of files
include lots of unnecessary stuff (which also means that it is easy to
forget to add a header when making use of it for the first time because
it might already be included indirectly), because our headers include
other headers to avoid forward declarations even when all they use from
another header are actually incomplete types (we only use forward
declarations when it is unavoidable because of cyclic dependencies or in
order to hide a private struct). Should we try to cut the dependencies
among headers and thereby force users to include what they use directly
by using more incomplete types/forward declarations in our headers?

> But I suppose it does not matter much in pracice (in this case
> especially), so feel free to ignore me.
> 
- Andreas

PS: Examples where we probably agree on exist, too: I don't think that
mem.h should be included basically everywhere via avcodec.h (via
libavutil/avutil.h and libavutil/common.h -- there might be other
paths); and cpu.h should be removed from avcodec.h, but that probably
requires a public announcement and grace period first.