[FFmpeg-devel] [PATCH 3/5] avcodec/bsf/hevc_mp4toannexb: Don't realloc when creating new extradata

[Andreas Rheinhardt]

James Almer:
> On 2/17/2024 11:44 PM, Andreas Rheinhardt wrote:
>> AVCodecParameters.extradata is supposed to be allocated with
>> av_malloc(); av_realloc() and its wrappers do not guarantee
>> the proper alignment. Therefore parse the extradata twice:
>> Once to check its validity and to determine the eventual size
>> and a second time to actually write the new extradata.
> 
> Why would av_malloc alignment be needed for extradata?
> I see the doxy says "Must be allocated with av_malloc()" but I'm fairly
> sure that was meant to be "Must be allocated with av_malloc() family of
> functions", like its AVCodecContext counterpart. The idea is that
> library users don't use normal malloc as extradata will be freed with
> av_free(), and not because it will be accessed by SIMD code.
> 

I thought that potential accesses by SIMD code was the point? After all,
the value of AV_INPUT_BUFFER_PADDING_SIZE (which is used for both packet
data and extradata) is chosen so big to accommodate reading via SIMD.
You incremented the constant for this very purpose in 6e80079a28.
(Granted, I don't think we have code where extradata is being parsed by
SIMD.)

Apart from that, given its differing alignment, I am not sure that
av_realloc() is really part of the av_malloc() family of functions. We
should probably replace "av_malloc() family" by "compatible with
av_free()" wherever we want to allow av_realloc(), too.

Anyway, there is another advantage of this patch:
>>
>> (Of course, not reallocating the buffer is beneficial in itself.)
>>
>> Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt at outlook.com>