On Tue, Apr 03, 2018 at 01:38:12PM +0200, Paul B Mahol wrote: [...] > > -static inline void skip_bits_long(GetBitContext *s, int n) > +static inline void refill_32(GetBitContext *s) > { > -#if UNCHECKED_BITSTREAM_READER > - s->index += n; > +#ifdef CACHED_BITSTREAM_READER > +#if !UNCHECKED_BITSTREAM_READER > + if (s->index >> 3 >= s->buffer_end - s->buffer) > + return; > +#endif > + > +#ifdef BITSTREAM_READER_LE > + s->cache = (uint64_t)AV_RL32(s->buffer + (s->index >> 3)) << > s->bits_left | s->cache; > #else > - s->index += av_clip(n, -s->index, s->size_in_bits_plus8 - s->index); > + s->cache = s->cache | (uint64_t)AV_RB32(s->buffer + (s->index >> > 3)) << (32 - s->bits_left); > +#endif > + s->index += 32; > + s->bits_left += 32; > +#endif > +}
please split the movement of code into a seperate patch, this is unreadable > + > +static inline void refill_64(GetBitContext *s) > +{ > +#ifdef CACHED_BITSTREAM_READER > +#if !UNCHECKED_BITSTREAM_READER > + if (s->index >> 3 >= s->buffer_end - s->buffer) > + return; > +#endif > + > +#ifdef BITSTREAM_READER_LE > + s->cache = AV_RL64(s->buffer + (s->index >> 3)); > +#else > + s->cache = AV_RB64(s->buffer + (s->index >> 3)); > +#endif > + s->index += 64; > + s->bits_left = 64; > +#endif > +} all uses of refill_64 are under CACHED_BITSTREAM_READER, so there should not be a need for an empty function and the ifdef can be then merged with the next, simplifying the code > + > +#ifdef CACHED_BITSTREAM_READER > +static inline uint64_t get_val(GetBitContext *s, unsigned n, int is_le) > +{ > + uint64_t ret; > + av_assert2(n>0 && n<=63); > + if (is_le) { > + ret = s->cache & ((UINT64_C(1) << n) - 1); > + s->cache >>= n; > + } else { > + ret = s->cache >> (64 - n); > + s->cache <<= n; > + } > + s->bits_left -= n; > + return ret; > +} > +#endif > + > +#ifdef CACHED_BITSTREAM_READER these 2 ifdef CACHED_BITSTREAM_READER can be merged > +static inline unsigned show_val(const GetBitContext *s, unsigned n) > +{ > +#ifdef BITSTREAM_READER_LE > + return s->cache & ((UINT64_C(1) << n) - 1); > +#else > + return s->cache >> (64 - n); > +#endif > +} > +#endif > + > +/** > + * Show 1-25 bits. > + */ > +static inline unsigned int show_bits(GetBitContext *s, int n) > +{ > + register int tmp; > +#ifdef CACHED_BITSTREAM_READER > + if (n > s->bits_left) > + refill_32(s); > + > + tmp = show_val(s, n); > +#else > + OPEN_READER_NOSIZE(re, s); > + av_assert2(n>0 && n<=25); > + UPDATE_CACHE(re, s); > + tmp = SHOW_UBITS(re, s, n); > #endif > + return tmp; > } This is more moved code in which it is impossible to see what is changed [...] > > -static inline int get_sbits(GetBitContext *s, int n) > +/** > + * Read 1-25 bits. > + */ > +static inline unsigned int get_bits(GetBitContext *s, int n) also unreadable and confusing, the code moves make these matchup incorrectly > { > register int tmp; > +#ifdef CACHED_BITSTREAM_READER > + > + av_assert2(n>0 && n<=32); > + if (n > s->bits_left) { > + refill_32(s); > + if (s->bits_left < 32) > + s->bits_left = n; > + } > + > +#ifdef BITSTREAM_READER_LE > + tmp = get_val(s, n, 1); > +#else > + tmp = get_val(s, n, 0); > +#endif > +#else > OPEN_READER(re, s); > av_assert2(n>0 && n<=25); > UPDATE_CACHE(re, s); > - tmp = SHOW_SBITS(re, s, n); > + tmp = SHOW_UBITS(re, s, n); > LAST_SKIP_BITS(re, s, n); > CLOSE_READER(re, s); > +#endif > return tmp; > } > > -/** > - * Read 1-25 bits. > - */ > -static inline unsigned int get_bits(GetBitContext *s, int n) > +static inline int get_sbits(GetBitContext *s, int n) > { > register int tmp; > +#ifdef CACHED_BITSTREAM_READER > + av_assert2(n>0 && n<=25); > + tmp = sign_extend(get_bits(s, n), n); > +#else > OPEN_READER(re, s); > av_assert2(n>0 && n<=25); > UPDATE_CACHE(re, s); > - tmp = SHOW_UBITS(re, s, n); > + tmp = SHOW_SBITS(re, s, n); > LAST_SKIP_BITS(re, s, n); > CLOSE_READER(re, s); > +#endif > return tmp; > } more mismatched functions that are diffed against each other [...] -- Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB Never trust a computer, one day, it may think you are the virus. -- Compn
signature.asc
Description: PGP signature
_______________________________________________ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel