Hi, 2011/12/12 Måns Rullgård <[email protected]>
> Alex Converse <[email protected]> writes: > > > On Mon, Dec 12, 2011 at 8:55 AM, Ronald S. Bultje <[email protected] > >wrote: > > > >> Hi, > >> > >> On Dec 12, 2011 8:50 AM, "Laurent Aimar" <[email protected]> wrote: > >> > > >> > On Mon, Dec 12, 2011 at 08:35:16AM -0800, Ronald S. Bultje wrote: > >> > > Hi, > >> > > > >> > > On Mon, Dec 12, 2011 at 7:54 AM, Laurent Aimar <[email protected] > > > >> wrote: > >> > > > >> > > I think that with your patch, get_bits_count() will never be > >> negative and > >> > > this > >> > > will creates issues with some decoders. > >> > > > >> > > > >> > > We should fix those decoders. get_bits_left() < 0 is a bug and we > >> should > >> > > document that as undefined behaviour, IMO. > >> > In itself it is not a bug. It is prefectly fine for get_bits_left() > to > >> > return < 0 without creating any issue if the user of the get bits > >> function > >> > ensure that the 'overread' does not exceed the mandatory padding > there is > >> > at the end of each buffer. > >> > > >> > Now, it can of course be decided to make get_bits_left() returning < > 0 > >> > a non valid use case, but it's a change from what I think was > previously > >> > understood (at least that's my impression reading various decoder > codes). > >> > It will probably need a lot of code review before the change can be > done > >> > safely. > >> > >> I agree, but I do feel this change has lowest effect on performance > while > >> still covering all of get_bits. > >> > >> Want to help review? > >> > > > > static inline void skip_bits_long(GetBitContext *s, int n){ > > +#if UNCHECKED_BITSTREAM_READER > > s->index += n; > > +#else > > + s->index = FFMIN(s->size_in_bits, s->index + n); > > +#endif > > } > > > > If we make this FFMIN(s->size_in_bits + 1, s->index + n) can't we get the > > best of both worlds for just one additional add? > > That still doesn't protect against overflow from a very large n. > You must check for n <= s->size_in_bits - s->index. > Yes I'll fix that in my next round - I'm a bit slow trying to catch up on email and stuff. Ronald
_______________________________________________ libav-devel mailing list [email protected] https://lists.libav.org/mailman/listinfo/libav-devel
