[FFmpeg-devel] [PATCH V2] lavc/vp8: Fix HWAccel VP8 decoder can't support resolution change.

[Mark Thompson]

On 29/11/17 23:53, Jun Zhao wrote:
> V2: fix the V1 lead to webp crash issue.
> 
> From b943c2814789288d09b4032fa6cdfbc3fd672a2b Mon Sep 17 00:00:00 2001
> From: Jun Zhao <jun.zhao at intel.com>
> Date: Wed, 29 Nov 2017 10:22:03 +0800
> Subject: [PATCH V2] lavc/vp8: Fix HWAccel VP8 decoder can't support resolution
>  change.
> 
> Use the following command to reproduce this issue:
> make fate-vp8-size-change HWACCEL="vaapi -vaapi_device \
> /dev/dri/renderD128 -hwaccel_output_format yuv420p"
> SAMPLES=../fate-suite/.
> 
> At the same time, reconstruct the public logic as a function.
> 
> Signed-off-by: Yun Zhou <yunx.z.zhou at intel.com>
> Signed-off-by: Jun Zhao <jun.zhao at intel.com>
> ---
>  libavcodec/vp8.c | 46 ++++++++++++++++++++++++++++++++++------------
>  1 file changed, 34 insertions(+), 12 deletions(-)
> 
> diff --git a/libavcodec/vp8.c b/libavcodec/vp8.c
> index 471c0bb89e..d5cb7be7b3 100644
> --- a/libavcodec/vp8.c
> +++ b/libavcodec/vp8.c
> @@ -167,6 +167,30 @@ static VP8Frame *vp8_find_free_buffer(VP8Context *s)
>      return frame;
>  }
>  
> +static enum AVPixelFormat get_pixel_format(VP8Context *s)
> +{
> +    enum AVPixelFormat fmt;
> +    enum AVPixelFormat pix_fmts[] = {
> +#if CONFIG_VP8_VAAPI_HWACCEL
> +        AV_PIX_FMT_VAAPI,
> +#endif
> +#if CONFIG_VP8_NVDEC_HWACCEL
> +        AV_PIX_FMT_CUDA,
> +#endif
> +        AV_PIX_FMT_YUV420P,
> +        AV_PIX_FMT_NONE,
> +    };
> +
> +    fmt = ff_get_format(s->avctx, pix_fmts);
> +    if (fmt < 0) {
> +        fmt = AV_PIX_FMT_NONE;

ff_get_format() already returns either a format in the list or AV_PIX_FMT_NONE.

> +        av_log(s->avctx, AV_LOG_ERROR,
> +               "Can not support the format. \n");

This error message is meaningless.

I don't think an error message is appropriate here, anyway - either the user explicitly chose to fail (and already knows it) or something went wrong in ff_get_format() (which already prints a more useful error there).

> +    }
> +
> +    return fmt;

So I think just "return ff_get_format(...);"?

> +}
> +
>  static av_always_inline
>  int update_dimensions(VP8Context *s, int width, int height, int is_vp7)
>  {
> @@ -182,6 +206,15 @@ int update_dimensions(VP8Context *s, int width, int height, int is_vp7)
>              return ret;
>      }
>  
> +    if (!s->actually_webp && !is_vp7) {
> +        s->pix_fmt = get_pixel_format(s);
> +        if (s->pix_fmt < 0) {
> +            ret = AVERROR(EINVAL);
> +            return ret;

Just "return AVERROR(EINVAL);"?

> +        }
> +        avctx->pix_fmt = s->pix_fmt;
> +    }
> +
>      s->mb_width  = (s->avctx->coded_width  + 15) / 16;
>      s->mb_height = (s->avctx->coded_height + 15) / 16;
>  
> @@ -2598,18 +2631,7 @@ int vp78_decode_frame(AVCodecContext *avctx, void *data, int *got_frame,
>      if (s->actually_webp) {
>          // avctx->pix_fmt already set in caller.
>      } else if (!is_vp7 && s->pix_fmt == AV_PIX_FMT_NONE) {
> -        enum AVPixelFormat pix_fmts[] = {
> -#if CONFIG_VP8_VAAPI_HWACCEL
> -            AV_PIX_FMT_VAAPI,
> -#endif
> -#if CONFIG_VP8_NVDEC_HWACCEL
> -            AV_PIX_FMT_CUDA,
> -#endif
> -            AV_PIX_FMT_YUV420P,
> -            AV_PIX_FMT_NONE,
> -        };
> -
> -        s->pix_fmt = ff_get_format(s->avctx, pix_fmts);
> +        s->pix_fmt = get_pixel_format(s);
>          if (s->pix_fmt < 0) {
>              ret = AVERROR(EINVAL);
>              goto err;
> -- 
> 2.14.1
> 

Tested with VAAPI, logic LGTM.

Thanks,

- Mark