[FFmpeg-devel,RFC] lavc/vp9dec: fix the multi-thread HWAccel decode error

Submitted by Linjie Fu on June 11, 2019, 7:19 p.m.

Details

Message ID 1560280770-3760-1-git-send-email-linjie.fu@intel.com
State New
Headers show

Commit Message

Linjie Fu June 11, 2019, 7:19 p.m.
Fix the multi-thread HWAccel decode error for vp9.

VP9 supports the resolution changing. In multi-thread mode, worker-threads
destroy and free the first created VAAPIDecodeContext if resolution change
happens and create new one. Other threads still request to destroy previous
and demand for new context. This leads to decode failures and memory issue.

Modify to call hwaccel_uninit/hwaccel_init by ff_thread_get_format only
when the first-come thread detected the resolution changing.

Signed-off-by: Linjie Fu <linjie.fu@intel.com>
---
Fix the same issue vaapi_vp8 decoding meets:
https://patchwork.ffmpeg.org/patch/12507/

 libavcodec/vp9.c    | 18 ++++++++++++++----
 libavcodec/vp9dec.h |  2 ++
 2 files changed, 16 insertions(+), 4 deletions(-)

Comments

Hendrik Leppkes June 11, 2019, 7:50 a.m.
On Tue, Jun 11, 2019 at 9:20 AM Linjie Fu <linjie.fu@intel.com> wrote:
>
> Fix the multi-thread HWAccel decode error for vp9.
>
> VP9 supports the resolution changing. In multi-thread mode, worker-threads
> destroy and free the first created VAAPIDecodeContext if resolution change
> happens and create new one. Other threads still request to destroy previous
> and demand for new context. This leads to decode failures and memory issue.
>
> Modify to call hwaccel_uninit/hwaccel_init by ff_thread_get_format only
> when the first-come thread detected the resolution changing.
>

s->gf_fmt, s->w and s->h are updated through
vp9_decode_update_thread_context, wouldn't they prevent
re-initialization already, as long as the size only changes once?
If not, why not? Perhaps thats a better avenue to fix it, then hacky
conditions like this one.

- Hendrik
Linjie Fu June 12, 2019, 2:06 a.m.
> -----Original Message-----

> From: ffmpeg-devel [mailto:ffmpeg-devel-bounces@ffmpeg.org] On Behalf

> Of Hendrik Leppkes

> Sent: Tuesday, June 11, 2019 15:50

> To: FFmpeg development discussions and patches <ffmpeg-

> devel@ffmpeg.org>

> Subject: Re: [FFmpeg-devel] [PATCH, RFC] lavc/vp9dec: fix the multi-thread

> HWAccel decode error

> 

> On Tue, Jun 11, 2019 at 9:20 AM Linjie Fu <linjie.fu@intel.com> wrote:

> >

> > Fix the multi-thread HWAccel decode error for vp9.

> >

> > VP9 supports the resolution changing. In multi-thread mode, worker-

> threads

> > destroy and free the first created VAAPIDecodeContext if resolution

> change

> > happens and create new one. Other threads still request to destroy

> previous

> > and demand for new context. This leads to decode failures and memory

> issue.

> >

> > Modify to call hwaccel_uninit/hwaccel_init by ff_thread_get_format only

> > when the first-come thread detected the resolution changing.

> >

> 

> s->gf_fmt, s->w and s->h are updated through

> vp9_decode_update_thread_context, wouldn't they prevent

> re-initialization already, as long as the size only changes once?

> If not, why not? Perhaps thats a better avenue to fix it, then hacky

> conditions like this one.

> 


Thanks Hendrik, need to reconsider this and seek for a better solution.

Patch hide | download patch | download mbox

diff --git a/libavcodec/vp9.c b/libavcodec/vp9.c
index acf3ffc..e2fad9d 100644
--- a/libavcodec/vp9.c
+++ b/libavcodec/vp9.c
@@ -179,6 +179,7 @@  static int update_size(AVCodecContext *avctx, int w, int h)
     uint8_t *p;
     int bytesperpixel = s->bytesperpixel, ret, cols, rows;
     int lflvl_len, i;
+    int dim_reset = 0;
 
     av_assert0(w > 0 && h > 0);
 
@@ -186,6 +187,10 @@  static int update_size(AVCodecContext *avctx, int w, int h)
         if ((ret = ff_set_dimensions(avctx, w, h)) < 0)
             return ret;
 
+        if (!avctx->internal->hwaccel_priv_data ||
+            s->context == avctx->internal->hwaccel_priv_data)
+            dim_reset = 1;
+
         switch (s->pix_fmt) {
         case AV_PIX_FMT_YUV420P:
         case AV_PIX_FMT_YUV420P10:
@@ -216,16 +221,21 @@  static int update_size(AVCodecContext *avctx, int w, int h)
         *fmtp++ = s->pix_fmt;
         *fmtp = AV_PIX_FMT_NONE;
 
-        ret = ff_thread_get_format(avctx, pix_fmts);
-        if (ret < 0)
-            return ret;
+        if (dim_reset) {
+            ret = ff_thread_get_format(avctx, pix_fmts);
+            if (ret < 0)
+                return ret;
+
+            avctx->pix_fmt = ret;
+        }
 
-        avctx->pix_fmt = ret;
         s->gf_fmt  = s->pix_fmt;
         s->w = w;
         s->h = h;
     }
 
+    s->context = avctx->internal->hwaccel_priv_data;
+
     cols = (w + 7) >> 3;
     rows = (h + 7) >> 3;
 
diff --git a/libavcodec/vp9dec.h b/libavcodec/vp9dec.h
index 66573ed..bc3f36c 100644
--- a/libavcodec/vp9dec.h
+++ b/libavcodec/vp9dec.h
@@ -152,6 +152,8 @@  typedef struct VP9Context {
     int block_alloc_using_2pass;
     uint16_t mvscale[3][2];
     uint8_t mvstep[3][2];
+
+    void *context;
 } VP9Context;
 
 struct VP9TileData {