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 _______________________________________________ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
