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

[James Almer]

On 8/10/2020 7:34 PM, Nicolas George wrote:
> James Almer (12020-08-10):
>> 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 exactly what the code does

I know, that's why i suggested it. Two lines, no extra structs, same result.

> , except without the undefined
> behaviors, because what you just wrote is completely invalid.

What is undefined in that? Should probably cast &ctx[1] to
AVBSFInternal* to silence a warning, but it works.

> 
>> 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.
> 
> You missed the part where Andreas said this was a proof of concept, and
> the same should be done for all similar cases. IIRC, you like
> consistency.

I also like readability, and ctx = malloc(sizeof(*ctx)); ctx->internal =
malloc(sizeof(*ctx->internal)); is the approach that most clearly
transmits intent on functions where removing the second allocation
doesn't bring any real benefit.

> 
>> 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.
> 
> It does save some memory.
> 
>> Can we not get overtly clever with code obfuscation?
> 
> There is no obfuscation, just a very standard an common practice.

It may be a common practice, but it's still obfuscation to a casual reader.

For that matter, i was told about the approach used with
AVCodecHWConfigInternal in libavcodec/hwconfig.h, which is similar to
this but doesn't introduce a third struct. What do you think?

> 
> 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".
>