Message ID | 1584456838-7151-1-git-send-email-linjie.fu@intel.com |
---|---|
State | Accepted |
Commit | 9473268cfb580dccd3f1f3be338cd22661ef791e |
Headers | show |
Series | [FFmpeg-devel,v4] lavc/vp9: fix reference frame dimensions check for SINGLE_REFERENCE mode | expand |
Context | Check | Description |
---|---|---|
andriy/ffmpeg-patchwork | success | Make fate finished |
Hi, On Tue, Mar 17, 2020 at 10:59 AM Linjie Fu <linjie.fu@intel.com> wrote: > With the description in frame size with refs semantics (SPEC 7.2.5), > it is a requirement of bitstream conformance that for at least one > reference frame has the valid dimensions. > > Modify the check to make sure the decoder works well in SINGLE_REFERENCE > mode that not all reference frames have valid dimensions. > > Check and error out if invalid reference frame is used in inter_recon. > > One of the failure case is a 480x272 inter frame (SINGLE_REFERENCE mode) > with following reference pool: > > 0. 960x544 LAST valid > 1. 1920x1088 GOLDEN invalid, but not used in single reference mode > 2. 1920x1088 ALTREF invalid, but not used in single reference mode > 3~7 ... Unused > > Identical logic in libvpx: > < > https://github.com/webmproject/libvpx/blob/master/vp9/decoder/vp9_decodeframe.c#L736 > > > > Signed-off-by: Linjie Fu <linjie.fu@intel.com> > --- > [v3]: replace assert with check/return, tested in both multi frame/slice > mode > [v4]: clear error_info to make decoding still work for other frames in > this stream > > libavcodec/vp9.c | 20 ++++++++++++++++++-- > libavcodec/vp9dec.h | 5 +++++ > libavcodec/vp9recon.c | 10 ++++++++++ > 3 files changed, 33 insertions(+), 2 deletions(-) LGTM, thanks for the revisions. (We have been discussing this on IRC.) Ronald
> Am 19.03.2020 um 13:10 schrieb Ronald S. Bultje <rsbultje@gmail.com>: > > Hi, > >> On Tue, Mar 17, 2020 at 10:59 AM Linjie Fu <linjie.fu@intel.com> wrote: >> >> With the description in frame size with refs semantics (SPEC 7.2.5), >> it is a requirement of bitstream conformance that for at least one >> reference frame has the valid dimensions. >> >> Modify the check to make sure the decoder works well in SINGLE_REFERENCE >> mode that not all reference frames have valid dimensions. >> >> Check and error out if invalid reference frame is used in inter_recon. >> >> One of the failure case is a 480x272 inter frame (SINGLE_REFERENCE mode) >> with following reference pool: >> >> 0. 960x544 LAST valid >> 1. 1920x1088 GOLDEN invalid, but not used in single reference mode >> 2. 1920x1088 ALTREF invalid, but not used in single reference mode >> 3~7 ... Unused >> >> Identical logic in libvpx: >> < >> https://github.com/webmproject/libvpx/blob/master/vp9/decoder/vp9_decodeframe.c#L736 >>> >> >> Signed-off-by: Linjie Fu <linjie.fu@intel.com> >> --- >> [v3]: replace assert with check/return, tested in both multi frame/slice >> mode >> [v4]: clear error_info to make decoding still work for other frames in >> this stream >> >> libavcodec/vp9.c | 20 ++++++++++++++++++-- >> libavcodec/vp9dec.h | 5 +++++ >> libavcodec/vp9recon.c | 10 ++++++++++ >> 3 files changed, 33 insertions(+), 2 deletions(-) > > > LGTM, thanks for the revisions. (We have been discussing this on IRC.) Why does this error out even if explode is not set? Carl Eugen
Hi Carl, > From: ffmpeg-devel <ffmpeg-devel-bounces@ffmpeg.org> On Behalf Of > Carl Eugen Hoyos > Sent: Thursday, March 19, 2020 21:04 > To: FFmpeg development discussions and patches <ffmpeg- > devel@ffmpeg.org> > Subject: Re: [FFmpeg-devel] [PATCH, v4] lavc/vp9: fix reference frame > dimensions check for SINGLE_REFERENCE mode > > > > > Am 19.03.2020 um 13:10 schrieb Ronald S. Bultje <rsbultje@gmail.com>: > > > > Hi, > > > >> On Tue, Mar 17, 2020 at 10:59 AM Linjie Fu <linjie.fu@intel.com> wrote: > >> > >> With the description in frame size with refs semantics (SPEC 7.2.5), > >> it is a requirement of bitstream conformance that for at least one > >> reference frame has the valid dimensions. > >> > >> Modify the check to make sure the decoder works well in > SINGLE_REFERENCE > >> mode that not all reference frames have valid dimensions. > >> > >> Check and error out if invalid reference frame is used in inter_recon. > >> > >> One of the failure case is a 480x272 inter frame (SINGLE_REFERENCE > mode) > >> with following reference pool: > >> > >> 0. 960x544 LAST valid > >> 1. 1920x1088 GOLDEN invalid, but not used in single reference mode > >> 2. 1920x1088 ALTREF invalid, but not used in single reference mode > >> 3~7 ... Unused > >> > >> Identical logic in libvpx: > >> < > >> > https://github.com/webmproject/libvpx/blob/master/vp9/decoder/vp9_de > codeframe.c#L736 > >>> > >> > >> Signed-off-by: Linjie Fu <linjie.fu@intel.com> > >> --- > >> [v3]: replace assert with check/return, tested in both multi frame/slice > >> mode > >> [v4]: clear error_info to make decoding still work for other frames in > >> this stream > >> > >> libavcodec/vp9.c | 20 ++++++++++++++++++-- > >> libavcodec/vp9dec.h | 5 +++++ > >> libavcodec/vp9recon.c | 10 ++++++++++ > >> 3 files changed, 33 insertions(+), 2 deletions(-) > > > > > > LGTM, thanks for the revisions. (We have been discussing this on IRC.) > > Why does this error out even if explode is not set? "explode" seems to mean that aborting the whole decode procedure on minor error detection. This fix [v4] intended to report errors and exit decode_frame() for the exact frame witch has invalid mvscale factors used, and didn't mean to break the decoding for subsequent frames if they could be decoded correctly. It's not an aborting for the whole decoding hence I think this behavior may not match the description of explode. Thanks, - Linjie
Am Do., 19. März 2020 um 14:45 Uhr schrieb Fu, Linjie <linjie.fu@intel.com>: > This fix [v4] intended to report errors and exit decode_frame() for the exact frame witch has > invalid mvscale factors used, and didn't mean to break the decoding for subsequent frames if > they could be decoded correctly. How does the output look visually before this patch and how does it look with this patch? Carl Eugen
> From: ffmpeg-devel <ffmpeg-devel-bounces@ffmpeg.org> On Behalf Of > Carl Eugen Hoyos > Sent: Thursday, March 19, 2020 22:13 > To: FFmpeg development discussions and patches <ffmpeg- > devel@ffmpeg.org> > Subject: Re: [FFmpeg-devel] [PATCH, v4] lavc/vp9: fix reference frame > dimensions check for SINGLE_REFERENCE mode > > Am Do., 19. März 2020 um 14:45 Uhr schrieb Fu, Linjie <linjie.fu@intel.com>: > > > This fix [v4] intended to report errors and exit decode_frame() for the > exact frame witch has > > invalid mvscale factors used, and didn't mean to break the decoding for > subsequent frames if > > they could be decoded correctly. > > How does the output look visually before this patch and how does it > look with this patch? > $ ffmpeg -i g2_decoder_case_15865.ivf -f null - * 1. Before this patch: Reported "Invalid ref frame dimensions" in the decode_frame_header() and abort the decoding. (only 8 frame decode) ==== [vp9 @ 0x55e6496b3050] Invalid ref frame dimensions 1920x1088 for frame size 480x272 [vp9 @ 0x55e6496baab0] Not all references are available [vp9 @ 0x55e6496c2510] Not all references are available [vp9 @ 0x55e6496c5620] Not all references are available Output #0, null, to 'pipe:': Metadata: encoder : Lavf58.42.100 Stream #0:0: Video: wrapped_avframe, yuv420p, 1920x1088, q=2-31, 200 kb/s, 25 fps, 25 tbn, 25 tbc Metadata: encoder : Lavc58.76.100 wrapped_avframe Error while decoding stream #0:0: Invalid data found when processing input Last message repeated 3 times frame= 8 fps=7.7 q=-0.0 Lsize=N/A time=00:00:00.48 bitrate=N/A speed=0.464x ==== * 2. With this patch: All frames (12) decoded correctly (visually) with a warning in the header check. ==== Input #0, ivf, from 'g2_decoder_case_15865.ivf': Duration: 00:00:00.01, start: 0.000000, bitrate: 100442 kb/s Stream #0:0: Video: vp9 (Profile 0) (VP90 / 0x30395056), yuv420p(tv), 1920x1088, 25 fps, 25 tbr, 1k tbn, 1k tbc Stream mapping: Stream #0:0 -> #0:0 (vp9 (native) -> wrapped_avframe (native)) Press [q] to stop, [?] for help [vp9 @ 0x56072d8df080] Invalid ref frame dimensions 1920x1088 for frame size 480x272 Last message repeated 1 times Output #0, null, to 'pipe:': Metadata: encoder : Lavf58.41.100 Stream #0:0: Video: wrapped_avframe, yuv420p, 1920x1088, q=2-31, 200 kb/s, 25 fps, 25 tbn, 25 tbc Metadata: encoder : Lavc58.75.100 wrapped_avframe frame= 12 fps=0.0 q=-0.0 Lsize=N/A time=00:00:00.48 bitrate=N/A speed=1.77x ==== If talking about the error out, it didn't happen in this bitstream since the invalid refs is not used. It would take effect in the situations that invalid ref is unexpected used in either single/comp reference during inter recon. And identical messages would be reported that invalid data found when processing input, and the outputted frames is less than expected. - Linjie
Am Do., 19. März 2020 um 16:12 Uhr schrieb Fu, Linjie <linjie.fu@intel.com>: > > > From: ffmpeg-devel <ffmpeg-devel-bounces@ffmpeg.org> On Behalf Of > > Carl Eugen Hoyos > > Sent: Thursday, March 19, 2020 22:13 > > To: FFmpeg development discussions and patches <ffmpeg- > > devel@ffmpeg.org> > > Subject: Re: [FFmpeg-devel] [PATCH, v4] lavc/vp9: fix reference frame > > dimensions check for SINGLE_REFERENCE mode > > > > Am Do., 19. März 2020 um 14:45 Uhr schrieb Fu, Linjie <linjie.fu@intel.com>: > > > > > This fix [v4] intended to report errors and exit decode_frame() for the > > exact frame witch has > > > invalid mvscale factors used, and didn't mean to break the decoding for > > subsequent frames if > > > they could be decoded correctly. > > > > How does the output look visually before this patch and how does it > > look with this patch? > > > $ ffmpeg -i g2_decoder_case_15865.ivf -f null - > > * 1. Before this patch: > Reported "Invalid ref frame dimensions" in the decode_frame_header() and abort the decoding. > (only 8 frame decode) > > ==== > [vp9 @ 0x55e6496b3050] Invalid ref frame dimensions 1920x1088 for frame size 480x272 > [vp9 @ 0x55e6496baab0] Not all references are available > [vp9 @ 0x55e6496c2510] Not all references are available > [vp9 @ 0x55e6496c5620] Not all references are available > Output #0, null, to 'pipe:': > Metadata: > encoder : Lavf58.42.100 > Stream #0:0: Video: wrapped_avframe, yuv420p, 1920x1088, q=2-31, 200 kb/s, 25 fps, 25 tbn, 25 tbc > Metadata: > encoder : Lavc58.76.100 wrapped_avframe > Error while decoding stream #0:0: Invalid data found when processing input > Last message repeated 3 times > frame= 8 fps=7.7 q=-0.0 Lsize=N/A time=00:00:00.48 bitrate=N/A speed=0.464x > ==== > > * 2. With this patch: > All frames (12) decoded correctly (visually) with a warning in the header check. > > ==== > Input #0, ivf, from 'g2_decoder_case_15865.ivf': > Duration: 00:00:00.01, start: 0.000000, bitrate: 100442 kb/s > Stream #0:0: Video: vp9 (Profile 0) (VP90 / 0x30395056), yuv420p(tv), 1920x1088, 25 fps, 25 tbr, 1k tbn, 1k tbc > Stream mapping: > Stream #0:0 -> #0:0 (vp9 (native) -> wrapped_avframe (native)) > Press [q] to stop, [?] for help > [vp9 @ 0x56072d8df080] Invalid ref frame dimensions 1920x1088 for frame size 480x272 > Last message repeated 1 times > Output #0, null, to 'pipe:': > Metadata: > encoder : Lavf58.41.100 > Stream #0:0: Video: wrapped_avframe, yuv420p, 1920x1088, q=2-31, 200 kb/s, 25 fps, 25 tbn, 25 tbc > Metadata: > encoder : Lavc58.75.100 wrapped_avframe > frame= 12 fps=0.0 q=-0.0 Lsize=N/A time=00:00:00.48 bitrate=N/A speed=1.77x Then I wonder if the commit message can be improved but that is just nit-picking. Thank you for your patience! Carl Eugen
> From: ffmpeg-devel <ffmpeg-devel-bounces@ffmpeg.org> On Behalf Of > Carl Eugen Hoyos > Sent: Friday, March 20, 2020 00:47 > To: FFmpeg development discussions and patches <ffmpeg- > devel@ffmpeg.org> > Subject: Re: [FFmpeg-devel] [PATCH, v4] lavc/vp9: fix reference frame > dimensions check for SINGLE_REFERENCE mode > > Am Do., 19. März 2020 um 16:12 Uhr schrieb Fu, Linjie <linjie.fu@intel.com>: > > > > > From: ffmpeg-devel <ffmpeg-devel-bounces@ffmpeg.org> On Behalf Of > > > Carl Eugen Hoyos > > > Sent: Thursday, March 19, 2020 22:13 > > > To: FFmpeg development discussions and patches <ffmpeg- > > > devel@ffmpeg.org> > > > Subject: Re: [FFmpeg-devel] [PATCH, v4] lavc/vp9: fix reference frame > > > dimensions check for SINGLE_REFERENCE mode > > > > > > Am Do., 19. März 2020 um 14:45 Uhr schrieb Fu, Linjie > <linjie.fu@intel.com>: > > > > > > > This fix [v4] intended to report errors and exit decode_frame() for the > > > exact frame witch has > > > > invalid mvscale factors used, and didn't mean to break the decoding for > > > subsequent frames if > > > > they could be decoded correctly. > > > > > > How does the output look visually before this patch and how does it > > > look with this patch? > > > > > $ ffmpeg -i g2_decoder_case_15865.ivf -f null - > > > > * 1. Before this patch: > > Reported "Invalid ref frame dimensions" in the decode_frame_header() > and abort the decoding. > > (only 8 frame decode) > > > > ==== > > [vp9 @ 0x55e6496b3050] Invalid ref frame dimensions 1920x1088 for frame > size 480x272 > > [vp9 @ 0x55e6496baab0] Not all references are available > > [vp9 @ 0x55e6496c2510] Not all references are available > > [vp9 @ 0x55e6496c5620] Not all references are available > > Output #0, null, to 'pipe:': > > Metadata: > > encoder : Lavf58.42.100 > > Stream #0:0: Video: wrapped_avframe, yuv420p, 1920x1088, q=2-31, 200 > kb/s, 25 fps, 25 tbn, 25 tbc > > Metadata: > > encoder : Lavc58.76.100 wrapped_avframe > > Error while decoding stream #0:0: Invalid data found when processing input > > Last message repeated 3 times > > frame= 8 fps=7.7 q=-0.0 Lsize=N/A time=00:00:00.48 bitrate=N/A > speed=0.464x > > ==== > > > > * 2. With this patch: > > All frames (12) decoded correctly (visually) with a warning in the header > check. > > > > ==== > > Input #0, ivf, from 'g2_decoder_case_15865.ivf': > > Duration: 00:00:00.01, start: 0.000000, bitrate: 100442 kb/s > > Stream #0:0: Video: vp9 (Profile 0) (VP90 / 0x30395056), yuv420p(tv), > 1920x1088, 25 fps, 25 tbr, 1k tbn, 1k tbc > > Stream mapping: > > Stream #0:0 -> #0:0 (vp9 (native) -> wrapped_avframe (native)) > > Press [q] to stop, [?] for help > > [vp9 @ 0x56072d8df080] Invalid ref frame dimensions 1920x1088 for frame > size 480x272 > > Last message repeated 1 times > > Output #0, null, to 'pipe:': > > Metadata: > > encoder : Lavf58.41.100 > > Stream #0:0: Video: wrapped_avframe, yuv420p, 1920x1088, q=2-31, 200 > kb/s, 25 fps, 25 tbn, 25 tbc > > Metadata: > > encoder : Lavc58.75.100 wrapped_avframe > > frame= 12 fps=0.0 q=-0.0 Lsize=N/A time=00:00:00.48 bitrate=N/A > speed=1.77x > > Then I wonder if the commit message can be improved but that is just > nit-picking. > > Thank you for your patience! > With pleasure, thank you for the review. - Linjie
> From: Ronald S. Bultje <rsbultje@gmail.com> > Sent: Thursday, March 19, 2020 20:10 > To: FFmpeg development discussions and patches <ffmpeg-devel@ffmpeg.org> > Cc: Fu, Linjie <linjie.fu@intel.com> > Subject: Re: [FFmpeg-devel] [PATCH, v4] lavc/vp9: fix reference frame dimensions check for SINGLE_REFERENCE mode > > Hi, > > On Tue, Mar 17, 2020 at 10:59 AM Linjie Fu <linjie.fu@intel.com<mailto:linjie.fu@intel.com>> wrote: > With the description in frame size with refs semantics (SPEC 7.2.5), > it is a requirement of bitstream conformance that for at least one > reference frame has the valid dimensions. > > Modify the check to make sure the decoder works well in SINGLE_REFERENCE > mode that not all reference frames have valid dimensions. > > Check and error out if invalid reference frame is used in inter_recon. > > One of the failure case is a 480x272 inter frame (SINGLE_REFERENCE mode) > with following reference pool: > > 0. 960x544 LAST valid > 1. 1920x1088 GOLDEN invalid, but not used in single reference mode > 2. 1920x1088 ALTREF invalid, but not used in single reference mode > 3~7 ... Unused > > Identical logic in libvpx: > <https://github.com/webmproject/libvpx/blob/master/vp9/decoder/vp9_decodeframe.c#L736> > > Signed-off-by: Linjie Fu <linjie.fu@intel.com<mailto:linjie.fu@intel.com>> > --- > [v3]: replace assert with check/return, tested in both multi frame/slice mode > [v4]: clear error_info to make decoding still work for other frames in this stream > > libavcodec/vp9.c | 20 ++++++++++++++++++-- > libavcodec/vp9dec.h | 5 +++++ > libavcodec/vp9recon.c | 10 ++++++++++ > 3 files changed, 33 insertions(+), 2 deletions(-) > > LGTM, thanks for the revisions. (We have been discussing this on IRC.) Thanks for the review and valuable suggestions. - Linjie
> From: ffmpeg-devel <ffmpeg-devel-bounces@ffmpeg.org> On Behalf Of Fu, > Linjie > Sent: Friday, March 20, 2020 09:49 > To: Ronald S. Bultje <rsbultje@gmail.com>; FFmpeg development > discussions and patches <ffmpeg-devel@ffmpeg.org> > Subject: Re: [FFmpeg-devel] [PATCH, v4] lavc/vp9: fix reference frame > dimensions check for SINGLE_REFERENCE mode > > > From: Ronald S. Bultje <rsbultje@gmail.com> > > Sent: Thursday, March 19, 2020 20:10 > > To: FFmpeg development discussions and patches <ffmpeg- > devel@ffmpeg.org> > > Cc: Fu, Linjie <linjie.fu@intel.com> > > Subject: Re: [FFmpeg-devel] [PATCH, v4] lavc/vp9: fix reference frame > dimensions check for SINGLE_REFERENCE mode > > > > Hi, > > > > On Tue, Mar 17, 2020 at 10:59 AM Linjie Fu > <linjie.fu@intel.com<mailto:linjie.fu@intel.com>> wrote: > > With the description in frame size with refs semantics (SPEC 7.2.5), > > it is a requirement of bitstream conformance that for at least one > > reference frame has the valid dimensions. > > > > Modify the check to make sure the decoder works well in > SINGLE_REFERENCE > > mode that not all reference frames have valid dimensions. > > > > Check and error out if invalid reference frame is used in inter_recon. > > > > One of the failure case is a 480x272 inter frame (SINGLE_REFERENCE mode) > > with following reference pool: > > > > 0. 960x544 LAST valid > > 1. 1920x1088 GOLDEN invalid, but not used in single reference mode > > 2. 1920x1088 ALTREF invalid, but not used in single reference mode > > 3~7 ... Unused > > > > Identical logic in libvpx: > > > <https://github.com/webmproject/libvpx/blob/master/vp9/decoder/vp9_d > ecodeframe.c#L736> > > > > Signed-off-by: Linjie Fu <linjie.fu@intel.com<mailto:linjie.fu@intel.com>> > > --- > > [v3]: replace assert with check/return, tested in both multi frame/slice > mode > > [v4]: clear error_info to make decoding still work for other frames in this > stream > > > > libavcodec/vp9.c | 20 ++++++++++++++++++-- > > libavcodec/vp9dec.h | 5 +++++ > > libavcodec/vp9recon.c | 10 ++++++++++ > > 3 files changed, 33 insertions(+), 2 deletions(-) > > > > LGTM, thanks for the revisions. (We have been discussing this on IRC.) > > Thanks for the review and valuable suggestions. > Ping for merge, thx. - Linjie
Hi, On Wed, Apr 29, 2020 at 8:08 AM Fu, Linjie <linjie.fu@intel.com> wrote: > > From: ffmpeg-devel <ffmpeg-devel-bounces@ffmpeg.org> On Behalf Of Fu, > > Linjie > > Sent: Friday, March 20, 2020 09:49 > > To: Ronald S. Bultje <rsbultje@gmail.com>; FFmpeg development > > discussions and patches <ffmpeg-devel@ffmpeg.org> > > Subject: Re: [FFmpeg-devel] [PATCH, v4] lavc/vp9: fix reference frame > > dimensions check for SINGLE_REFERENCE mode > > > > > From: Ronald S. Bultje <rsbultje@gmail.com> > > > Sent: Thursday, March 19, 2020 20:10 > > > To: FFmpeg development discussions and patches <ffmpeg- > > devel@ffmpeg.org> > > > Cc: Fu, Linjie <linjie.fu@intel.com> > > > Subject: Re: [FFmpeg-devel] [PATCH, v4] lavc/vp9: fix reference frame > > dimensions check for SINGLE_REFERENCE mode > > > > > > Hi, > > > > > > On Tue, Mar 17, 2020 at 10:59 AM Linjie Fu > > <linjie.fu@intel.com<mailto:linjie.fu@intel.com>> wrote: > > > With the description in frame size with refs semantics (SPEC 7.2.5), > > > it is a requirement of bitstream conformance that for at least one > > > reference frame has the valid dimensions. > > > > > > Modify the check to make sure the decoder works well in > > SINGLE_REFERENCE > > > mode that not all reference frames have valid dimensions. > > > > > > Check and error out if invalid reference frame is used in inter_recon. > > > > > > One of the failure case is a 480x272 inter frame (SINGLE_REFERENCE > mode) > > > with following reference pool: > > > > > > 0. 960x544 LAST valid > > > 1. 1920x1088 GOLDEN invalid, but not used in single reference mode > > > 2. 1920x1088 ALTREF invalid, but not used in single reference mode > > > 3~7 ... Unused > > > > > > Identical logic in libvpx: > > > > > <https://github.com/webmproject/libvpx/blob/master/vp9/decoder/vp9_d > > ecodeframe.c#L736> > > > > > > Signed-off-by: Linjie Fu <linjie.fu@intel.com<mailto: > linjie.fu@intel.com>> > > > --- > > > [v3]: replace assert with check/return, tested in both multi > frame/slice > > mode > > > [v4]: clear error_info to make decoding still work for other frames in > this > > stream > > > > > > libavcodec/vp9.c | 20 ++++++++++++++++++-- > > > libavcodec/vp9dec.h | 5 +++++ > > > libavcodec/vp9recon.c | 10 ++++++++++ > > > 3 files changed, 33 insertions(+), 2 deletions(-) > > > > > > LGTM, thanks for the revisions. (We have been discussing this on IRC.) > > > > Thanks for the review and valuable suggestions. > > > Ping for merge, thx. Applied. Ronald
diff --git a/libavcodec/vp9.c b/libavcodec/vp9.c index 7aaae9b..2b79dfd 100644 --- a/libavcodec/vp9.c +++ b/libavcodec/vp9.c @@ -794,6 +794,7 @@ static int decode_frame_header(AVCodecContext *avctx, /* check reference frames */ if (!s->s.h.keyframe && !s->s.h.intraonly) { + int valid_ref_frame = 0; for (i = 0; i < 3; i++) { AVFrame *ref = s->s.refs[s->s.h.refidx[i]].f; int refw = ref->width, refh = ref->height; @@ -807,17 +808,25 @@ static int decode_frame_header(AVCodecContext *avctx, } else if (refw == w && refh == h) { s->mvscale[i][0] = s->mvscale[i][1] = 0; } else { + /* Check to make sure at least one of frames that */ + /* this frame references has valid dimensions */ if (w * 2 < refw || h * 2 < refh || w > 16 * refw || h > 16 * refh) { - av_log(avctx, AV_LOG_ERROR, + av_log(avctx, AV_LOG_WARNING, "Invalid ref frame dimensions %dx%d for frame size %dx%d\n", refw, refh, w, h); - return AVERROR_INVALIDDATA; + s->mvscale[i][0] = s->mvscale[i][1] = REF_INVALID_SCALE; + continue; } s->mvscale[i][0] = (refw << 14) / w; s->mvscale[i][1] = (refh << 14) / h; s->mvstep[i][0] = 16 * s->mvscale[i][0] >> 14; s->mvstep[i][1] = 16 * s->mvscale[i][1] >> 14; } + valid_ref_frame++; + } + if (!valid_ref_frame) { + av_log(avctx, AV_LOG_ERROR, "No valid reference frame is found, bitstream not supported\n"); + return AVERROR_INVALIDDATA; } } @@ -1614,6 +1623,7 @@ FF_ENABLE_DEPRECATION_WARNINGS s->td[i].eob = s->td[i].eob_base; s->td[i].uveob[0] = s->td[i].uveob_base[0]; s->td[i].uveob[1] = s->td[i].uveob_base[1]; + s->td[i].error_info = 0; } #if HAVE_THREADS @@ -1670,6 +1680,12 @@ FF_ENABLE_DEPRECATION_WARNINGS } while (s->pass++ == 1); ff_thread_report_progress(&s->s.frames[CUR_FRAME].tf, INT_MAX, 0); + if (s->td->error_info < 0) { + av_log(avctx, AV_LOG_ERROR, "Failed to decode tile data\n"); + s->td->error_info = 0; + return AVERROR_INVALIDDATA; + } + finish: // ref frame setup for (i = 0; i < 8; i++) { diff --git a/libavcodec/vp9dec.h b/libavcodec/vp9dec.h index 66573ed..f2585c3 100644 --- a/libavcodec/vp9dec.h +++ b/libavcodec/vp9dec.h @@ -36,6 +36,8 @@ #include "vp9dsp.h" #include "vp9shared.h" +#define REF_INVALID_SCALE 0xFFFF + enum MVJoint { MV_JOINT_ZERO, MV_JOINT_H, @@ -217,6 +219,9 @@ struct VP9TileData { struct { int x, y; } min_mv, max_mv; int16_t *block_base, *block, *uvblock_base[2], *uvblock[2]; uint8_t *eob_base, *uveob_base[2], *eob, *uveob[2]; + + // error message + int error_info; }; void ff_vp9_fill_mv(VP9TileData *td, VP56mv *mv, int mode, int sb); diff --git a/libavcodec/vp9recon.c b/libavcodec/vp9recon.c index 49bb04e..9a4e7c7 100644 --- a/libavcodec/vp9recon.c +++ b/libavcodec/vp9recon.c @@ -572,6 +572,16 @@ static av_always_inline void inter_recon(VP9TileData *td, int bytesperpixel) VP9Block *b = td->b; int row = td->row, col = td->col; + if (s->mvscale[b->ref[0]][0] == REF_INVALID_SCALE || + (b->comp && s->mvscale[b->ref[1]][0] == REF_INVALID_SCALE)) { + if (!s->td->error_info) { + s->td->error_info = AVERROR_INVALIDDATA; + av_log(NULL, AV_LOG_ERROR, "Bitstream not supported, " + "reference frame has invalid dimensions\n"); + } + return; + } + if (s->mvscale[b->ref[0]][0] || (b->comp && s->mvscale[b->ref[1]][0])) { if (bytesperpixel == 1) { inter_pred_scaled_8bpp(td);
With the description in frame size with refs semantics (SPEC 7.2.5), it is a requirement of bitstream conformance that for at least one reference frame has the valid dimensions. Modify the check to make sure the decoder works well in SINGLE_REFERENCE mode that not all reference frames have valid dimensions. Check and error out if invalid reference frame is used in inter_recon. One of the failure case is a 480x272 inter frame (SINGLE_REFERENCE mode) with following reference pool: 0. 960x544 LAST valid 1. 1920x1088 GOLDEN invalid, but not used in single reference mode 2. 1920x1088 ALTREF invalid, but not used in single reference mode 3~7 ... Unused Identical logic in libvpx: <https://github.com/webmproject/libvpx/blob/master/vp9/decoder/vp9_decodeframe.c#L736> Signed-off-by: Linjie Fu <linjie.fu@intel.com> --- [v3]: replace assert with check/return, tested in both multi frame/slice mode [v4]: clear error_info to make decoding still work for other frames in this stream libavcodec/vp9.c | 20 ++++++++++++++++++-- libavcodec/vp9dec.h | 5 +++++ libavcodec/vp9recon.c | 10 ++++++++++ 3 files changed, 33 insertions(+), 2 deletions(-)