[FFmpeg-devel] [PATCH] ac3enc: check snr_offset of 0 before failing in cbr_bit_allocation().

[Justin Ruggles]

On 03/06/2011 01:52 PM, M?ns Rullg?rd wrote:

> Justin Ruggles <justin.ruggles at gmail.com> writes:
> 
>> snr_offset does not always start at a multiple of 64, so some values below 64
>> could go untested without this change.
>> ---
>> New patch which uses 0 directly, then sets snr_offset only on success.
>>
>>  libavcodec/ac3enc.c |    7 +++++--
>>  1 files changed, 5 insertions(+), 2 deletions(-)
>>
>>
>> diff --git a/libavcodec/ac3enc.c b/libavcodec/ac3enc.c
>> index 676bb5e..2cebb81 100644
>> --- a/libavcodec/ac3enc.c
>> +++ b/libavcodec/ac3enc.c
>> @@ -959,8 +959,11 @@ static int cbr_bit_allocation(AC3EncodeContext *s)
>>             bit_alloc(s, snr_offset) > bits_left) {
>>          snr_offset -= 64;
>>      }
>> -    if (snr_offset < 0)
>> -        return AVERROR(EINVAL);
>> +    if (snr_offset < 0) {
>> +        if (bit_alloc(s, 0) > bits_left)
>> +            return AVERROR(EINVAL);
>> +        snr_offset = 0;
>> +    }
>>  
>>      FFSWAP(uint8_t *, s->bap_buffer, s->bap1_buffer);
>>      for (snr_incr = 64; snr_incr > 0; snr_incr >>= 2) {
> 
> Looks reasonable, with the caveat that I don't actually know the details
> of what this is doing.


Well, now I think I've come up with a slightly better solution.

The search is actually faster on average if the initial downward search
is incremented by 16 rather than 64.  Since the search starts at a
multiple of 16, that eliminates the case where snr_offset jumps from >0
to <0 and fails without checking positive values in-between (which is
what this patch was meant to fix).

I'll send a new patch shortly.

Thanks,
Justin