[FFmpeg-devel] [PATCH, RFC, v2] lavc/phtread_frame: update context in child thread in multi-thread mode

[Fu, Linjie]

> -----Original Message-----
> From: ffmpeg-devel [mailto:ffmpeg-devel-bounces at ffmpeg.org] On Behalf
> Of Michael Niedermayer
> Sent: Thursday, June 27, 2019 00:29
> To: FFmpeg development discussions and patches <ffmpeg-
> devel at ffmpeg.org>
> Subject: Re: [FFmpeg-devel] [PATCH, RFC, v2] lavc/phtread_frame: update
> context in child thread in multi-thread mode
> 
> On Wed, Jun 26, 2019 at 04:24:52PM -0400, Linjie Fu wrote:
> > Currently in ff_thread_decode_frame, context is updated from child
> thread
> > to main thread, and main thread releases the context in avcodec_close()
> > when decode finishes.
> >
> > However, when resolution/format change in vp9, ff_get_format was called,
> > and hwaccel_uninit() and hwaccel_init will be used to destroy and re-
> create
> > the context. Due to the async between main-thread and child-thread,
> > main-thread updated its context from child earlier than the context was
> > refreshed in child-thread. And it will lead to:
> >     1. memory leak in child-thread.
> >     2. double free in main-thread while calling avcodec_close().
> >
> > Can be reproduced with a resolution change case in vp9, and use -vframes
> > to terminate the decode between the dynamic resolution change frames:
> >
> > ffmpeg -hwaccel vaapi -hwaccel_device /dev/dri/renderD128 -v
> > verbose -i ./test2360_1672_4980.ivf -pix_fmt p010le -f rawvideo -vsync
> > passthrough -vframes 6 -y out.yuv
> >
> > Move update_context_from_thread from ff_thread_decode_frame(main
> thread)
> > to frame_worker_thread(child thread), update the context in child thread
> > right after the context was updated to avoid the async issue.
> >
> > Signed-off-by: Linjie Fu <linjie.fu at intel.com>
> > ---
> > Request for Comments, not quite familiar with the constraints.
> > Thanks in advance.
> 
> i dont think i fully understand the problem you are trying to fix but
> this patch looks like it writes into the users context without any
> lock while the user can access it.
> Thats looks like a race condition unless I am missing something

Yes that's what I'm concerned and seeking for advice.
One possible way I thought is to add a new lock, and lock in both frame_worker_thread
and submit_packet(user) wwhen it attmepts to update context from user to child thread.

> What is very noticable though is that you seem to talk about vp9
> why is this vp9 specific and does not affect other codecs ?

Actually it's not codec specific.
It happens as long as context refreshed because of resolution/format change in
child thread but failed to update in main thread correctly.
Same issue exists in h264 as well if decode terminate during resolution changing
(-vframes 45 in this example):

ffmpeg -hwaccel vaapi -hwaccel_device /dev/dri/renderD128 -v verbose -i ./reinit-large_420_8-to-small_420_8.h264 -pix_fmt nv12 -f rawvideo -vsync passthrough -vframes 45 -y md5.yuv

http://fate-suite.ffmpeg.org/h264/reinit-large_420_8-to-small_420_8.h264
And this patch fixed it as well.

It seems I should not restrict this issue to vp9 in commit message.

- Linjie