[FFmpeg-devel] [PATCH] avfilter/src_movie: Remove align dimension to fix crash

[James Almer]

On 5/10/2024 6:26 PM, Paul B Mahol wrote:
> On Fri, May 10, 2024 at 10:56 AM Zhao Zhili <quinkblack at foxmail.com> wrote:
> 
>> From: Zhao Zhili <zhilizhao at tencent.com>
>>
>> The alignment is handled by ff_default_get_video_buffer2. We
>> shouldn't use the aligned width/height as FFFramePool width/height.
>> It cause recreate FFFramePool inside ff_default_get_video_buffer2.
>> The recreate of FFFramePool together with multi-threads decoding
>> leading to data race and crash, for example,
>> ./ffplay -f lavfi -i movie=foo.mp4,drawtext=text=bar
>> ---
>>   libavfilter/src_movie.c | 1 -
>>   1 file changed, 1 deletion(-)
>>
>> diff --git a/libavfilter/src_movie.c b/libavfilter/src_movie.c
>> index e2cdcf17db..5099012100 100644
>> --- a/libavfilter/src_movie.c
>> +++ b/libavfilter/src_movie.c
>> @@ -187,7 +187,6 @@ static int get_buffer(AVCodecContext *avctx, AVFrame
>> *frame, int flags)
>>
>>       switch (avctx->codec_type) {
>>       case AVMEDIA_TYPE_VIDEO:
>> -        avcodec_align_dimensions2(avctx, &w, &h, linesize_align);
>>
> 
> Left unused linesize_align[].
> 
> With locking introduced, cant you remove copy check hack above and force
> new_pool every time?

Can't this just always use avcodec_default_get_buffer2() (So in short, 
not set a custom get_buffer2() callback at all), since it also has its 
own buffer pool? Much like it's being used for AV_CODEC_CAP_DR1 
decoders, it can be used for all of them.

ff_default_get_{video,audio}_buffer() don't seem like they bring any 
benefit here. In fact, not using them may be an improvement considering 
they allocate a whole new AVFrame forcing the callback to copy props, 
move the reference and then free the superfluous AVFrame afterwards.