[FFmpeg-devel] [PATCH] avcodec/bsf: Avoid allocation for AVBSFInternal

[Andreas Rheinhardt]

James Almer:
> On 8/10/2020 7:11 PM, Nicolas George wrote:
>> James Almer (12020-08-10):
>>> Personally, i don't like it. It's extra complexity to save a single 8 or
>>> 12 byte allocation that happens once during bsf alloc. It's kind of a
>>> pointless micro-optimization.
>>
>> I do not agree at all.
>>
>> First, it is not extra complexity, it actually makes the code simpler:
>> less mutually dependant allocations that can lead to leaks if they are
>> not handled properly, better guarantees, for no more code.
> 
> It adds an extra struct and makes the code harder to read. Might as well
> just do
> 
> ctx = av_mallocz(sizeof(*ctx) + sizeof(AVBSFInternal));
> ctx->internal = &ctx[1];

This is not ok due to padding/alignment.

> 
> if removing one tiny allocation in an incredibly cold function is so
> important. Less code, same result.

I don't see an obfuscation/complication (and also no problem with
maintainability); I see a very simple method to save an allocation. And
I actually think that it simplifies the code, because now one doesn't
have to worry at all any more whether internal has been properly
allocated or not in av_bsf_free().
Do you remember 31aafdac2404c5e01b21e53255db3fb5ed53c7c9
(https://ffmpeg.org/pipermail/ffmpeg-devel/2019-October/251816.html)
where you fixed the bug that avformat_free_context() gets called upon
failure to allocate the AVFormatInternal despite avformat_free_context()
requiring the internal to be successfully allocated? That issue would
have never even existed if one allocated the context and its internal in
one piece.

- Andreas