On 8/10/2020 8:12 PM, Andreas Rheinhardt wrote: > 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.
libdav1d would have broke by now if that was the case. Do you know a system where this could fail? > >> >> 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(). I don't deny it simplifies the code in regards to freeing the context in failure paths, but adding a third struct does not make things clearer at all. So Mark's suggestion to add AVBSFContext into AVBSFInternal may be a better approach. > 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 > [email protected] > https://ffmpeg.org/mailman/listinfo/ffmpeg-devel > > To unsubscribe, visit link above, or email > [email protected] with subject "unsubscribe". > _______________________________________________ ffmpeg-devel mailing list [email protected] https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email [email protected] with subject "unsubscribe".
