[FFmpeg-devel] [PATCH V2] lavc/vp8: Fix HWAccel VP8 decoder can't support resolution change.
Mark Thompson
sw at jkqxz.net
Thu Dec 14 02:51:19 EET 2017
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
More information about the ffmpeg-devel
mailing list