On Mon, Jul 24, 2017 at 01:52:20AM +0200, Carl Eugen Hoyos wrote:
> 2017-07-24 0:33 GMT+02:00 Tyler Jones <tdjones...@gmail.com>:
> > Vorbis I specification requires that the version number as well as the
> > window and transform types in the setup header be equal to 0.
> >
> > Signed-off-by: Tyler Jones <tdjones...@gmail.com>
> > ---
> > libavcodec/vorbisdec.c | 18 +++++++++++++++---
> > 1 file changed, 15 insertions(+), 3 deletions(-)
> >
> > diff --git a/libavcodec/vorbisdec.c b/libavcodec/vorbisdec.c
> > index 2a4f482031..f9c3848c4e 100644
> > --- a/libavcodec/vorbisdec.c
> > +++ b/libavcodec/vorbisdec.c
> > @@ -898,8 +898,16 @@ static int vorbis_parse_setup_hdr_modes(vorbis_context
> > *vc)
> > vorbis_mode *mode_setup = &vc->modes[i];
> >
> > mode_setup->blockflag = get_bits1(gb);
> > - mode_setup->windowtype = get_bits(gb, 16); //FIXME check
> > - mode_setup->transformtype = get_bits(gb, 16); //FIXME check
> > + mode_setup->windowtype = get_bits(gb, 16);
> > + if (mode_setup->windowtype) {
> > + av_log(vc->avctx, AV_LOG_ERROR, "Invalid window type,
> > must equal 0.\n");
> > + return AVERROR_INVALIDDATA;
>
> Does this fix anything?
>
> By default, FFmpeg decoders should not (and, more so, should not
> suddenly start to) reject files that can be decoded without any
> effort.
> Or are such files already unplayable, the error message was
> just missing?
>
> You can reject such files for strict conformance mode.
>
> Carl Eugen
I'll defer to your judgement, but this is how the specifications defines it:
(4.2.4 -- Modes)
verify ranges; zero is the only legal value in Vorbis I for
[vorbis_mode_windowtype]
and [vorbis_mode_transformtype]. [vorbis_mode_mapping] must not be greater
than the
highest number mapping in use. Any illegal values render the stream
undecodable.
These values are unused in the decoder and otherwise ignored in the
specification.
What is even the value of storing these values beyond a temporary value?
I believed that it is important to check these values so that an encoder cannot
produce
a stream that comes out of sync and end up failing a later test anyways.
In the interest of consistency, there are several identical cases where values
have the potential to make the stream 'undecodable' even if they have no impact
on the behavior of the decoder. In all of these other cases, the decoding quits
immediately. Should these be reverted to only log an error message and not
return error values?
Thanks,
Tyler Jones
signature.asc
Description: PGP signature
_______________________________________________ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
