[FFmpeg-devel] [PATCH, v3] lavc/pthread_frame: update context in child thread in multi-thread mode

[Hendrik Leppkes]

On Thu, Jun 27, 2019 at 1:56 PM Linjie Fu <linjie.fu at intel.com> wrote:
>
> Currently in ff_thread_decode_frame, context is updated from child thread
> to user thread, and user thread releases the context in avcodec_close()
> when decode finishes.
>
> However, when resolution/format changes, ff_get_format is called, and
> hwaccel_uninit() and hwaccel_init will be used to destroy and re-create
> the context. Due to the async between user-thread and child-thread,
> user-thread updates its context from child earlier than the context
> is refreshed in child-thread. And it will lead to:
>     1. memory leak in child-thread.
>     2. double free in user-thread while calling avcodec_close().
>
> Can be reproduced with a resolution change case, and use -vframes
> to terminate the decode between the dynamic resolution changing 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
>
> 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 out.yuv
>
> Move update_context_from_thread from ff_thread_decode_frame(user thread)
> to frame_worker_thread(child thread), update the context in child thread
> right after the context is refreshed to avoid the async issue.
>

I think the undelying issue that Michael mentioned remains - you are
changing the user context from a worker thread, at which point the
user might be accessing his context simultaneously. You cannot prevent
that with a mutex, since the user does not use your mutex.

Additionally, the user context should reflect the state of the last
frame that was output to the user, if a worker thread changes it
immediately as it sees the size change, wouldn't it be possible for
frames of the old size to be output after the context was already
updated? That sounds like trouble to me.

- Hendrik