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

[Xiang, Haihao]

On Thu, 2018-08-09 at 15:09 +0800, Jun Zhao wrote:
> the root cause is update_dimentions call get_pixel_format will

Typo? s/dimentions/dimensions. And I see the same typo a few times in this log.

> trigger the hwaccel_uninit/hwaccel_init , in current context,

What is the decode error? Could you document the error a little or a link to the
issue?

> 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,

According to the code, ff_set_dimensions is called to overwrite previously setup
dimensions when height is changed no matter  
s->macroblocks_base is allocated or not. I think the HW frames and context
should be re-created as well for this case 

But I am curious why height and width are treated differently in this condition:

    if (width  != s->avctx->width || ((width+15)/16 != s->mb_width ||
(height+15)/16 != s->mb_height) && s->macroblocks_base ||
        height != s->avctx->height) {
       ...
       ff_set_dimensions(...);
}

The condition for updating dimensions is simple in VP9:

   if (!(s->pix_fmt == s->gf_fmt && w == s->w && h == s->h)) {
       ff_set_dimensions(...);
       ...
   }

>    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) {
>          s->pix_fmt = get_pixel_format(s);
>          if (s->pix_fmt < 0)
>              return AVERROR(EINVAL);