Message ID | 27a56d91-6d6d-dd12-794c-28d7fda99107@gmail.com |
---|---|
State | Superseded |
Headers | show |
ping ?
On 2017/11/30 7:53, Jun Zhao wrote:
> V2: fix the V1 lead to webp crash issue.
what is V1 and V2? On Tue, Dec 5, 2017 at 11:03 AM, Jun Zhao <mypopydev@gmail.com> wrote: > ping ? > > On 2017/11/30 7:53, Jun Zhao wrote: > > V2: fix the V1 lead to webp crash issue. > > _______________________________________________ > ffmpeg-devel mailing list > ffmpeg-devel@ffmpeg.org > http://ffmpeg.org/mailman/listinfo/ffmpeg-devel >
Re-ping, any comments? On 2017/12/5 11:03, Jun Zhao wrote: > ping ? > > On 2017/11/30 7:53, Jun Zhao wrote: >> V2: fix the V1 lead to webp crash issue.
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@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@intel.com> > Signed-off-by: Jun Zhao <jun.zhao@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
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.zhao@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@intel.com> >> Signed-off-by: Jun Zhao <jun.zhao@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
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; + av_log(s->avctx, AV_LOG_ERROR, + "Can not support the format. \n"); + } + + return fmt; +} + 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; + } + 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;