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

[Paul B Mahol]

On Sat, May 11, 2024 at 5:28 AM James Almer <jamrial at gmail.com> wrote:

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

In that case frame is always not part of frame pool and thus not writable,
any next
filter that needs writable frames will just allocate and sometimes even
copy whole frame data.
The original idea is to avoid that copy.


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