Luca Barbato <[email protected]> writes:

> On 02/12/11 21:20, Måns Rullgård wrote:
>> Luca Barbato<[email protected]>  writes:
>>
>>> On 26/11/11 18:07, Måns Rullgård wrote:
>>>> Kostya Shishkov<[email protected]>   writes:
>>>>
>>>>> On Sat, Nov 26, 2011 at 04:52:09PM +0000, Mans Rullgard wrote:
>>>>>> Signed-off-by: Mans Rullgard<[email protected]>
>>>>>> ---
>>>>>>    libavcodec/svq1dec.c |    3 ++-
>>>>>>    1 files changed, 2 insertions(+), 1 deletions(-)
>>>>>>
>>>>>> diff --git a/libavcodec/svq1dec.c b/libavcodec/svq1dec.c
>>>>>> index 8569615..7eb6e60 100644
>>>>>> --- a/libavcodec/svq1dec.c
>>>>>> +++ b/libavcodec/svq1dec.c
>>>>>> @@ -195,7 +195,8 @@ static const uint8_t string_table[256] = {
>>>>>>
>>>>>>    #define SVQ1_CALC_CODEBOOK_ENTRIES(cbook)\
>>>>>>          codebook = (const uint32_t *) cbook[level];\
>>>>>> -      bit_cache = get_bits (bitbuf, 4*stages);\
>>>>>> +      if (stages>   0)\
>>>>>> +        bit_cache = get_bits (bitbuf, 4*stages);\
>>>>>>          /* calculate codebook entries for this vector */\
>>>>>>          for (j=0; j<   stages; j++) {\
>>>>>>            entries[j] = (((bit_cache>>   (4*(stages - j - 1)))&   0xF) + 
>>>>>> 16*j)<<   (level + 1);\
>>>>>> --
>>>>>
>>>>> maybe set it to 0 for zero bits?
>>>>
>>>> That would add overhead in cases where it's impossible for it to be
>>>> zero, which should be everywhere.
>>>
>>> Assuming it's true, patch ok.
>>
>> That what's true?
>
> That's is impossible, thus the patch is ok. ^^

I'll elaborate a little on the problem.  Calling get_bits(0) results in
an undefined shift operation.  Since reading zero bits is not a normal
thing to do, checking for this in get_bits() would add overhead for all
calls, even when the length cannot possibly be zero, which is typically
the case.  Thus the few places that might end up calling get_bits(0)
need to check the value outside the call.  This is one of those cases.
With a little luck, the compiler will merge the comparison with the one
from the for loop that follows.

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

Reply via email to