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); 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? +#endif } --Alex
_______________________________________________ libav-devel mailing list [email protected] https://lists.libav.org/mailman/listinfo/libav-devel
