Andreas Rheinhardt:
> Anton Khirnov:
>> +/* Unwind the cache so a refill_32 can fill it again. */
>> +static inline void bitstream_unwind(BitstreamContext *bc)
>> +{
>> + int unwind = 4;
>> + int unwind_bits = unwind * 8;
>
> I'm surprised that you used signed types here.
>
>> +
>> + if (bc->bits_left < unwind_bits)
>> + return;
>> +
>> + bc->bits >>= unwind_bits;
>> + bc->bits <<= unwind_bits;
>
> The above won't work in LE. Best to call skip_remaining here. And you
> need to templatize this function in 3/4.
Calling skip_remaining is wrong either. Both the above (for BE) as well
as skip_remaining would skip the oldest 32 bits in the cache, but we
need to skip the newest 32 bits in the cache. So the following should do it:
bc->bits_left -= unwind_bits;
bc->ptr -= unwind;
#ifdef BITSTREAM_READER_LE
bc->bits &= ((UINT64_C(1) << bc->bits_left) - 1);
#else
bc->bits &= ~(UINT64_T_MAX >> bc->bits_left);
#endif
(Given that bc->bits_left can be 0 one can't simply shift by 64 -
bits_left. I also don't know whether there should be any check before
decrementing ptr.)
>
>> + bc->bits_left -= unwind_bits;
>> + bc->ptr -= unwind;
>> +}
>> +
>> +/* Unget up to 32 bits. */
>> +static inline void bitstream_unget(BitstreamContext *bc, uint64_t value,
>> + size_t amount)
>
> size_t is the natural type for the bytesize of objects, but not for
> bitsizes. A plane unsigned would be more natural here.
>
>> +{
>> + size_t cache_size = sizeof(bc->bits) * 8;
>> +
>> + if (bc->bits_left + amount > cache_size)
>> + bitstream_unwind(bc);
>> +
>> + bc->bits = (bc->bits >> amount) | (value << (cache_size -
>> amount));
>
> This is big-endian only, too.
>
>> + bc->bits_left += amount;
>> +}
>> +
_______________________________________________
ffmpeg-devel mailing list
[email protected]
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
[email protected] with subject "unsubscribe".
