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

James Almer jamrial at gmail.com
Tue Aug 11 01:27:47 EEST 2020


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];

if removing one tiny allocation in an incredibly cold function is so
important. Less code, same result.
And i *really* want to emphasize just how cold this function is,
especially ever since av_bsf_flush() was introduced, and even more so if
av_bsf_close() is also committed.

> 
> Second, this is precisely the kind of "micro" optimization we should be
> doing everywhere, because "micro" times "a lot" starts to be
> significant, and FFmpeg has not reached its status by wasting "micro"
> optimizations.

Skipping a 12 byte allocation does nothing on any system with a C
library worth its salt, where it will be cached and ready to be returned
by malloc.

Can we not get overtly clever with code obfuscation? Lets optimize
things where things need to be optimized, while also ensuring code is
readable and maintainable not just for us, but for any developer that
stumbles upon current code in the future as well.

> 
> Regards,
> 
> 
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel at ffmpeg.org
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
> 
> To unsubscribe, visit link above, or email
> ffmpeg-devel-request at ffmpeg.org with subject "unsubscribe".
> 



More information about the ffmpeg-devel mailing list