[FFmpeg-devel] [PATCH, v4] lavc/vp9: fix reference frame dimensions check for SINGLE_REFERENCE mode

[Fu, Linjie]

Hi Carl,
> From: ffmpeg-devel <ffmpeg-devel-bounces at ffmpeg.org> On Behalf Of
> Carl Eugen Hoyos
> Sent: Thursday, March 19, 2020 21:04
> To: FFmpeg development discussions and patches <ffmpeg-
> devel at 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 at gmail.com>:
> >
> > Hi,
> >
> >> On Tue, Mar 17, 2020 at 10:59 AM Linjie Fu <linjie.fu at 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 at 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