Laurent Aimar <[email protected]> writes:

> On Thu, Dec 15, 2011 at 05:02:55PM -0800, Ronald S. Bultje wrote:
>> Hi,
>> 
>> On Thu, Dec 15, 2011 at 4:31 PM, Laurent Aimar <[email protected]> wrote:
>> 
>>     On Thu, Dec 15, 2011 at 04:24:05PM -0800, Ronald S. Bultje wrote:
>>     > On Thu, Dec 15, 2011 at 4:22 PM, Laurent Aimar <[email protected]> 
>> wrote:
>>     >
>>     >     On Thu, Dec 15, 2011 at 12:30:38PM -0800, Ronald S. Bultje wrote:
>>     >     > +#if !UNCHECKED_BITSTREAM_READER
>>     >     > +    s->size_in_bits_plus1 = bit_size + 1;
>>     >     > +#endif
>>     >      One extra bit will ensure that get_bit_count() will return
>>     >     < 0 value, but it does not ensure that get_bits/show_bits
>>     >     like functions or macros return 0 on overread, and so it is
>>     >     incompatible with get_vlc* functions (you will have an
>>     >     infinite loop, at least it was like that when I last check
>>     >     this solution).
>>     >
>>     >
>>     > Why not?
>>     Please, read the thread
>>      "[libav-devel] [PATCH] Checked get_bits.h functions to prevent overread"
>>     as it contains some explanations and warnings about making a suitable
>>     safe get_bits implementation (for existing usage of it).
>> 
>>     For the get_vlc* case, the functions don't check for overread as
>>     it expects the buffer to be padded with enough 0 bits
>>     (FF_INPUT_BUFFER_PADDING_SIZE bytes), and those zeros will ensure
>>     that get_vlc() will abort on reading an invalid VLC code.
>> 
>> 
>> The buffer is padded with 32 bytes of zeroes and index never goes more than 1
>> bit beyond. You're telling me get_vlc() will read 32 bytes in a single
>> iteration?
>  No. But your implementation when reaching the end may not return 0 if
> you read/show more than 1 bit. You need to let the index goes beyong
> the max by at least the number of bits the cache can hold (in the case
> where size_in_bits is a multiple of 8, otherwise probably a bit more
> but it may depend on the actual implementation of the reader) to allow
> reading into the padding which will then allow returning 0.

You are not making any sense at all.  The bitstream readers never return
bits *before* the current position.  There is also no possibility of an
infinite loop, for the simple reason that get_vlc() does not contain a
loop.

It is probably a good idea to extend the cap to a position beyond the
last data byte, so callers don't need to make sure any extra bits in the
last byte are zeroed.  Adding 8 instead of 1 will do this.

-- 
Måns Rullgård
[email protected]
_______________________________________________
libav-devel mailing list
[email protected]
https://lists.libav.org/mailman/listinfo/libav-devel

Reply via email to