| Message ID | 1577436455-24883-1-git-send-email-linjie.fu@intel.com |
|---|---|
| State | Accepted |
| Headers | show |
Hi, > -----Original Message----- > From: Fu, Linjie <linjie.fu@intel.com> > Sent: Friday, December 27, 2019 16:48 > To: ffmpeg-devel@ffmpeg.org > Cc: Fu, Linjie <linjie.fu@intel.com> > Subject: [PATCH] lavc/pthread_frame: Update user context in > ff_frame_thread_free > > Resolution/format changes lead to re-initialization of hardware > accelerations(vaapi/dxva2/..) with new hwaccel_priv_data in > the worker-thread. But hwaccel_priv_data in user context won't > be updated until the resolution changing frame is output. > > A termination with "-vframes" just after the reinit will lead to: > 1. memory leak in worker-thread. > 2. double free in user-thread. > > Update user context in ff_frame_thread_free with the last thread > submit_packet() was called on. > > To reproduce: > ffmpeg -hwaccel vaapi(dxva2) -v verbose -i > fate-suite/h264/reinit-large_420_8-to-small_420_8.h264 -pix_fmt nv12 > -f rawvideo -vsync passthrough -vframes 47 -y out.yuv > > Signed-off-by: Linjie Fu <linjie.fu@intel.com> > --- > libavcodec/pthread_frame.c | 7 +++++++ > 1 file changed, 7 insertions(+) > > diff --git a/libavcodec/pthread_frame.c b/libavcodec/pthread_frame.c > index 36ac0ac..8bdd735 100644 > --- a/libavcodec/pthread_frame.c > +++ b/libavcodec/pthread_frame.c > @@ -657,6 +657,13 @@ void ff_frame_thread_free(AVCodecContext *avctx, > int thread_count) > > park_frame_worker_threads(fctx, thread_count); > > + if (fctx->prev_thread && avctx->internal->hwaccel_priv_data != > + fctx->prev_thread->avctx->internal->hwaccel_priv_data) { > + if (update_context_from_thread(avctx, fctx->prev_thread->avctx, 1) < > 0) { > + av_log(avctx, AV_LOG_ERROR, "Failed to update user thread.\n"); > + } > + } > + > if (fctx->prev_thread && fctx->prev_thread != fctx->threads) > if (update_context_from_thread(fctx->threads->avctx, fctx- > >prev_thread->avctx, 0) < 0) { > av_log(avctx, AV_LOG_ERROR, "Final thread update failed\n"); > -- > 2.7.4 Ping. This patch helps to fix the decoding crashes. - linjie
Hi, > -----Original Message----- > From: ffmpeg-devel <ffmpeg-devel-bounces@ffmpeg.org> On Behalf Of Fu, > Linjie > Sent: Monday, December 30, 2019 09:45 > To: ffmpeg-devel@ffmpeg.org > Subject: Re: [FFmpeg-devel] [PATCH] lavc/pthread_frame: Update user > context in ff_frame_thread_free > > Hi, > > > -----Original Message----- > > From: Fu, Linjie <linjie.fu@intel.com> > > Sent: Friday, December 27, 2019 16:48 > > To: ffmpeg-devel@ffmpeg.org > > Cc: Fu, Linjie <linjie.fu@intel.com> > > Subject: [PATCH] lavc/pthread_frame: Update user context in > > ff_frame_thread_free > > > > Resolution/format changes lead to re-initialization of hardware > > accelerations(vaapi/dxva2/..) with new hwaccel_priv_data in > > the worker-thread. But hwaccel_priv_data in user context won't > > be updated until the resolution changing frame is output. > > > > A termination with "-vframes" just after the reinit will lead to: > > 1. memory leak in worker-thread. > > 2. double free in user-thread. > > > > Update user context in ff_frame_thread_free with the last thread > > submit_packet() was called on. > > > > To reproduce: > > ffmpeg -hwaccel vaapi(dxva2) -v verbose -i > > fate-suite/h264/reinit-large_420_8-to-small_420_8.h264 -pix_fmt nv12 > > -f rawvideo -vsync passthrough -vframes 47 -y out.yuv > > > > Signed-off-by: Linjie Fu <linjie.fu@intel.com> > > --- > > libavcodec/pthread_frame.c | 7 +++++++ > > 1 file changed, 7 insertions(+) > > > > diff --git a/libavcodec/pthread_frame.c b/libavcodec/pthread_frame.c > > index 36ac0ac..8bdd735 100644 > > --- a/libavcodec/pthread_frame.c > > +++ b/libavcodec/pthread_frame.c > > @@ -657,6 +657,13 @@ void ff_frame_thread_free(AVCodecContext > *avctx, > > int thread_count) > > > > park_frame_worker_threads(fctx, thread_count); > > > > + if (fctx->prev_thread && avctx->internal->hwaccel_priv_data != > > + fctx->prev_thread->avctx->internal->hwaccel_priv_data) { > > + if (update_context_from_thread(avctx, fctx->prev_thread->avctx, 1) > < > > 0) { > > + av_log(avctx, AV_LOG_ERROR, "Failed to update user thread.\n"); > > + } > > + } > > + > > if (fctx->prev_thread && fctx->prev_thread != fctx->threads) > > if (update_context_from_thread(fctx->threads->avctx, fctx- > > >prev_thread->avctx, 0) < 0) { > > av_log(avctx, AV_LOG_ERROR, "Final thread update failed\n"); > > -- > > 2.7.4 > > Ping. > This patch helps to fix the decoding crashes. > Ping. - linjie
Hi, > -----Original Message----- > From: ffmpeg-devel <ffmpeg-devel-bounces@ffmpeg.org> On Behalf Of Fu, > Linjie > Sent: Monday, January 6, 2020 17:24 > To: FFmpeg development discussions and patches <ffmpeg- > devel@ffmpeg.org> > Subject: Re: [FFmpeg-devel] [PATCH] lavc/pthread_frame: Update user > context in ff_frame_thread_free > > Hi, > > > -----Original Message----- > > From: ffmpeg-devel <ffmpeg-devel-bounces@ffmpeg.org> On Behalf Of > Fu, > > Linjie > > Sent: Monday, December 30, 2019 09:45 > > To: ffmpeg-devel@ffmpeg.org > > Subject: Re: [FFmpeg-devel] [PATCH] lavc/pthread_frame: Update user > > context in ff_frame_thread_free > > > > Hi, > > > > > -----Original Message----- > > > From: Fu, Linjie <linjie.fu@intel.com> > > > Sent: Friday, December 27, 2019 16:48 > > > To: ffmpeg-devel@ffmpeg.org > > > Cc: Fu, Linjie <linjie.fu@intel.com> > > > Subject: [PATCH] lavc/pthread_frame: Update user context in > > > ff_frame_thread_free > > > > > > Resolution/format changes lead to re-initialization of hardware > > > accelerations(vaapi/dxva2/..) with new hwaccel_priv_data in > > > the worker-thread. But hwaccel_priv_data in user context won't > > > be updated until the resolution changing frame is output. > > > > > > A termination with "-vframes" just after the reinit will lead to: > > > 1. memory leak in worker-thread. > > > 2. double free in user-thread. > > > > > > Update user context in ff_frame_thread_free with the last thread > > > submit_packet() was called on. > > > > > > To reproduce: > > > ffmpeg -hwaccel vaapi(dxva2) -v verbose -i > > > fate-suite/h264/reinit-large_420_8-to-small_420_8.h264 -pix_fmt nv12 > > > -f rawvideo -vsync passthrough -vframes 47 -y out.yuv > > > > > > Signed-off-by: Linjie Fu <linjie.fu@intel.com> > > > --- > > > libavcodec/pthread_frame.c | 7 +++++++ > > > 1 file changed, 7 insertions(+) > > > > > > diff --git a/libavcodec/pthread_frame.c b/libavcodec/pthread_frame.c > > > index 36ac0ac..8bdd735 100644 > > > --- a/libavcodec/pthread_frame.c > > > +++ b/libavcodec/pthread_frame.c > > > @@ -657,6 +657,13 @@ void ff_frame_thread_free(AVCodecContext > > *avctx, > > > int thread_count) > > > > > > park_frame_worker_threads(fctx, thread_count); > > > > > > + if (fctx->prev_thread && avctx->internal->hwaccel_priv_data != > > > + fctx->prev_thread->avctx->internal->hwaccel_priv_data) > { > > > + if (update_context_from_thread(avctx, fctx->prev_thread->avctx, > 1) > > < > > > 0) { > > > + av_log(avctx, AV_LOG_ERROR, "Failed to update user thread.\n"); > > > + } > > > + } > > > + > > > if (fctx->prev_thread && fctx->prev_thread != fctx->threads) > > > if (update_context_from_thread(fctx->threads->avctx, fctx- > > > >prev_thread->avctx, 0) < 0) { > > > av_log(avctx, AV_LOG_ERROR, "Final thread update failed\n"); > > > -- > > > 2.7.4 > > > > Ping. > > This patch helps to fix the decoding crashes. > > > > Ping. > Ping.
Quoting Linjie Fu (2019-12-27 09:47:35) > Resolution/format changes lead to re-initialization of hardware > accelerations(vaapi/dxva2/..) with new hwaccel_priv_data in > the worker-thread. But hwaccel_priv_data in user context won't > be updated until the resolution changing frame is output. > > A termination with "-vframes" just after the reinit will lead to: > 1. memory leak in worker-thread. > 2. double free in user-thread. > > Update user context in ff_frame_thread_free with the last thread > submit_packet() was called on. > > To reproduce: > ffmpeg -hwaccel vaapi(dxva2) -v verbose -i > fate-suite/h264/reinit-large_420_8-to-small_420_8.h264 -pix_fmt nv12 > -f rawvideo -vsync passthrough -vframes 47 -y out.yuv > > Signed-off-by: Linjie Fu <linjie.fu@intel.com> > --- > libavcodec/pthread_frame.c | 7 +++++++ > 1 file changed, 7 insertions(+) > > diff --git a/libavcodec/pthread_frame.c b/libavcodec/pthread_frame.c > index 36ac0ac..8bdd735 100644 > --- a/libavcodec/pthread_frame.c > +++ b/libavcodec/pthread_frame.c > @@ -657,6 +657,13 @@ void ff_frame_thread_free(AVCodecContext *avctx, int thread_count) > > park_frame_worker_threads(fctx, thread_count); > > + if (fctx->prev_thread && avctx->internal->hwaccel_priv_data != > + fctx->prev_thread->avctx->internal->hwaccel_priv_data) { > + if (update_context_from_thread(avctx, fctx->prev_thread->avctx, 1) < 0) { > + av_log(avctx, AV_LOG_ERROR, "Failed to update user thread.\n"); > + } > + } > + > if (fctx->prev_thread && fctx->prev_thread != fctx->threads) > if (update_context_from_thread(fctx->threads->avctx, fctx->prev_thread->avctx, 0) < 0) { > av_log(avctx, AV_LOG_ERROR, "Final thread update failed\n"); > -- > 2.7.4 Not an ideal solution - we shouldn't be leaving stale pointers around in the first place - but I suppose it's good enough for now. Will push unless someone objects.
diff --git a/libavcodec/pthread_frame.c b/libavcodec/pthread_frame.c index 36ac0ac..8bdd735 100644 --- a/libavcodec/pthread_frame.c +++ b/libavcodec/pthread_frame.c @@ -657,6 +657,13 @@ void ff_frame_thread_free(AVCodecContext *avctx, int thread_count) park_frame_worker_threads(fctx, thread_count); + if (fctx->prev_thread && avctx->internal->hwaccel_priv_data != + fctx->prev_thread->avctx->internal->hwaccel_priv_data) { + if (update_context_from_thread(avctx, fctx->prev_thread->avctx, 1) < 0) { + av_log(avctx, AV_LOG_ERROR, "Failed to update user thread.\n"); + } + } + if (fctx->prev_thread && fctx->prev_thread != fctx->threads) if (update_context_from_thread(fctx->threads->avctx, fctx->prev_thread->avctx, 0) < 0) { av_log(avctx, AV_LOG_ERROR, "Final thread update failed\n");
Resolution/format changes lead to re-initialization of hardware accelerations(vaapi/dxva2/..) with new hwaccel_priv_data in the worker-thread. But hwaccel_priv_data in user context won't be updated until the resolution changing frame is output. A termination with "-vframes" just after the reinit will lead to: 1. memory leak in worker-thread. 2. double free in user-thread. Update user context in ff_frame_thread_free with the last thread submit_packet() was called on. To reproduce: ffmpeg -hwaccel vaapi(dxva2) -v verbose -i fate-suite/h264/reinit-large_420_8-to-small_420_8.h264 -pix_fmt nv12 -f rawvideo -vsync passthrough -vframes 47 -y out.yuv Signed-off-by: Linjie Fu <linjie.fu@intel.com> --- libavcodec/pthread_frame.c | 7 +++++++ 1 file changed, 7 insertions(+)