[FFmpeg-devel] [PATCH 01/10] avcodec/dca: remove Rice code length limit

foo86 foobaz86 at gmail.com
Fri May 20 15:09:25 CEST 2016

On Fri, May 20, 2016 at 02:35:53PM +0200, Christophe Gisquet wrote:
> 2016-05-13 11:48 GMT+02:00 foo86 <foobaz86 at gmail.com>:
> > -    unsigned int v = get_unary(gb, 1, 128);
> > +    unsigned int v = get_unary(gb, 1, get_bits_left(gb));
> Not that the patch is not ok, but I have a few uneducated questions:
> 1) Given the get_bits_long(gb, k) afterwards, won't that code cause
> overreads for corrupted bitstreams?

This will cause overread, but that should not be a problem for checked
bitstream reader.

> 2) I haven't checked the calling code, but consequently, wouldn't it
> be better to first check that at least k+1 bits are available?

I don't think this is necessarry. Fixed length overread is safe; adding
explicit check will make code less readable IMO (and possibly slower).

> 3) 128 is already fairly large; is the new code for valid bitstreams
> (in the sense of specs and actually generated) or for corrupted
> bitstreams? I don't know where the parsing is validated afterwards
> (e.g. if there have been overreads or invalid values parsed)

This is for valid bitstreams. There is no indication of limit on maximum
Rice code length in the XLL spec, hence existing constant is not
strictly "valid" (but it always worked in practice with existing
encoders). Reference decoder also loops indefinitely until it sees stop
bit here.

Parsing is validated at the end of chs_parse_band_data(), there is an
explicit check whether overread has occured (and if it has, entire
segment is zeroed out).

More information about the ffmpeg-devel mailing list