On Sat, Jul 8, 2017 at 7:09 PM, foo86 <fooba...@gmail.com> wrote:
> On Sat, Jul 08, 2017 at 11:12:06AM +0200, Paul B Mahol wrote:
>> [...]
>
>> static inline void skip_bits(GetBitContext *s, int n)
>> {
>> +#ifdef CACHED_BITSTREAM_READER
>> + if (n <= s->bits_left)
>> + skip_remaining(s, n);
>> + else {
>> + n -= s->bits_left;
>> + skip_remaining(s, s->bits_left);
> This causes undefined behavior if s->bits_left == 64.
>
>> + if (n >= 64) {
>> + unsigned skip = n;
>> +
>> + n -= skip;
>> + s->index += skip;
>> + }
> This block looks strange.
>
>> + refill_32(s);
>> + if (n)
>> + skip_remaining(s, n);
>> + }
>> +#else
>> OPEN_READER(re, s);
>> LAST_SKIP_BITS(re, s, n);
>> CLOSE_READER(re, s);
>> +#endif
>> +}
>> +
>> +static inline void skip_bits_long(GetBitContext *s, int n)
>> +{
>> +#ifdef CACHED_BITSTREAM_READER
>> + skip_bits(s, n);
>> +#else
>> +#if UNCHECKED_BITSTREAM_READER
>> + s->index += n;
>> +#else
>> + s->index += av_clip(n, -s->index, s->size_in_bits_plus8 - s->index);
> Uncached bitstream reader allows seeking back by passing negative n
> here. If cached bitstream reader disallows this, there should be a
> comment saying so (and possibly an assert).
This seems like an undocumented and possibly insecure/invalid use of
the API, maybe we should just generally discourage such use.
Why would you need to skip backwards anyway? Usually code uses
show_bits, or creates a copy of the reader so it can revert to the
original if needed.
- Hendrik
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
