On Fri, Jul 14, 2017 at 05:12:25PM +0200, Hendrik Leppkes wrote:
> On Fri, Jul 14, 2017 at 4:08 PM, foo86 <fooba...@gmail.com> wrote:
> > On Thu, Jul 13, 2017 at 12:27:03PM +0200, Paul B Mahol wrote:
> >> +static inline unsigned int get_bits(GetBitContext *s, int n)
> >> {
> >> +#ifdef CACHED_BITSTREAM_READER
> >> + register int tmp = 0;
> >> +#ifdef BITSTREAM_READER_LE
> >> + uint64_t left = 0;
> >> +#endif
> >> +
> >> + av_assert2(n>0 && n<=32);
> >> + if (n > s->bits_left) {
> >> + n -= s->bits_left;
> >> +#ifdef BITSTREAM_READER_LE
> >> + left = s->bits_left;
> >> +#endif
> >> + tmp = get_val(s, s->bits_left);
> > This triggers an assert in get_val() if s->bits_left == 0.
> >
> >> + refill_32(s);
> >> + }
> >> +
> >> +#ifdef BITSTREAM_READER_LE
> >> + tmp = get_val(s, n) << left | tmp;
> >> +#else
> >> + tmp = get_val(s, n) | tmp << n;
> > This causes undefined behavior if n > 30.
>
> get_bits is only valid until n = 25 in the "non-cached" case, so its
> not a problem to impose the same limitation on the cached reader.
> In fact, if they are to share the exact same API, it should probably
> follow that they also share the same constraints, so that we can do
> proper performance comparisons between the two, instead of having to
> re-write the using code.
Cached bitstream reader currently uses get_bits() to implement
get_bits_long(), which means cached get_bits() must support reading
values up to 32 bits.
I agree however that cached/uncached bistream readers should have the
same API contraints. That means cached get_bits_long() should probably
have a separate implementation.
>
> - Hendrik
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
