On 2017/12/14 8:51, Mark Thompson wrote: > 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.z...@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.z...@intel.com> >> Signed-off-by: Jun Zhao <jun.z...@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(...);"? Will double-check this part, Tks. > >> +} >> + >> 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);"? Yes, this change more pithy > >> + } >> + 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, Thanks for the reviewed and tested, will re-submit after clean the code. > > - Mark > _______________________________________________ > ffmpeg-devel mailing list > ffmpeg-devel@ffmpeg.org > http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
_______________________________________________ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel