[FFmpeg-devel] [PATCH] libavcodec/vp8dec: fix the multi-thread HWAccel decode error

Wang, Shaofei shaofei.wang at intel.com
Wed Mar 6 10:25:00 EET 2019


> -----Original Message-----
> From: ffmpeg-devel [mailto:ffmpeg-devel-bounces at ffmpeg.org] On Behalf Of
> Carl Eugen Hoyos
> Sent: Wednesday, March 6, 2019 3:49 PM
> To: FFmpeg development discussions and patches <ffmpeg-devel at ffmpeg.org>
> Subject: Re: [FFmpeg-devel] [PATCH] libavcodec/vp8dec: fix the multi-thread
> HWAccel decode error
> 
> 2018-08-09 9:09 GMT+02:00, Jun Zhao <mypopydev at gmail.com>:
> > the root cause is update_dimentions call get_pixel_format will trigger
> > the hwaccel_uninit/hwaccel_init , in current context, there are 3
> > situations in the update_dimentions():
> > 1. First time calling. No matter single thread or multithread,
> >    get_pixel_format() should be called after dimentions were
> >    set;
> > 2. Dimention changed at the runtime. Dimention need to be
> >    updated when macroblocks_base is already allocated,
> >    get_pixel_format() should be called to recreate new frames
> >    according to updated dimention;
> > 3. Multithread first time calling. After decoder init, the
> >    other threads will call update_dimentions() at first time
> >    to allocate macroblocks_base and set dimentions.
> >    But get_pixel_format() is shouldn't be called due to low
> >    level frames and context are already created.
> > In this fix, we only call update_dimentions as need.
> >
> > Signed-off-by: Wang, Shaofei <shaofei.wang at intel.com>
> > Reviewed-by: Jun, Zhao <jun.zhao at intel.com>
> > ---
> >  libavcodec/vp8.c |    7 +++++--
> >  1 files changed, 5 insertions(+), 2 deletions(-)
> >
> > diff --git a/libavcodec/vp8.c b/libavcodec/vp8.c index
> > 3adfeac..18d1ada 100644
> > --- a/libavcodec/vp8.c
> > +++ b/libavcodec/vp8.c
> > @@ -187,7 +187,7 @@ static av_always_inline  int
> > update_dimensions(VP8Context *s, int width, int height, int is_vp7)  {
> >      AVCodecContext *avctx = s->avctx;
> > -    int i, ret;
> > +    int i, ret, dim_reset = 0;
> >
> >      if (width  != s->avctx->width || ((width+15)/16 != s->mb_width ||
> > (height+15)/16 != s->mb_height) && s->macroblocks_base ||
> >          height != s->avctx->height) { @@ -196,9 +196,12 @@ int
> > update_dimensions(VP8Context *s, int width, int height, int is_vp7)
> >          ret = ff_set_dimensions(s->avctx, width, height);
> >          if (ret < 0)
> >              return ret;
> > +
> > +        dim_reset = (s->macroblocks_base != NULL);
> >      }
> >
> > -    if (!s->actually_webp && !is_vp7) {
> > +    if ((s->pix_fmt == AV_PIX_FMT_NONE || dim_reset) &&
> > +         !s->actually_webp && !is_vp7) {
> 
> Why is the new variable dim_reset needed?
> Wouldn't the patch be simpler if you used s->macroblocks_base here?
Since dim_reset was set in the "if" segment, it equal to
(width != s->avctx->width || ((width+15)/16 != s->mb_width ||
(height+15)/16 != s->mb_height) || height != s->avctx->height) && s->macroblocks_base

> 
> Carl Eugen
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel at ffmpeg.org
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel


More information about the ffmpeg-devel mailing list