[Ffmpeg-cvslog] r5507 - trunk/libavcodec/flac.c

[Loren Merritt]

On Tue, 20 Jun 2006, Justin Ruggles wrote:
> lu_zero wrote:
>> -    for (i = pred_order; i < s->blocksize; i++)
>> -    {
>> -        sum = 0;
>> -        for (j = 0; j < pred_order; j++)
>> -            sum += coeffs[j] * s->decoded[channel][i-j-1];
>> -        s->decoded[channel][i] += sum >> qlevel;
>> +    if (s->bps > 16) {
>> +        int64_t sum;
>> +        for (i = pred_order; i < s->blocksize; i++)
>> +        {
>> +            sum = 0;
>> +            for (j = 0; j < pred_order; j++)
>> +                sum += (int64_t)coeffs[j] * s->decoded[channel][i-j-1];
>> +            s->decoded[channel][i] += sum >> qlevel;
>> +        }
>> +    } else {
>> +        int sum;
>> +        for (i = pred_order; i < s->blocksize; i++)
>> +        {
>> +            sum = 0;
>> +            for (j = 0; j < pred_order; j++)
>> +                sum += coeffs[j] * s->decoded[channel][i-j-1];
>> +            s->decoded[channel][i] += sum >> qlevel;
>> +        }
>
> sorry...I do realize that I am speaking up a bit late, but I have a
> question about this change.  I don't see the reason to have 2 versions
> of this.  I know that having a 32-bit sum for 16-bit audio is not likely
> to overflow, but it's not a guarantee, and with lossless audio it's
> critical to be 100% sure.  My suggestion would be to always use the
> 64-bit version.  This would put the overflow limit out of the range of
> theoretical possibility.

Overflow is not an error. For 16bit audio, if qlevel<=16, then bit #32 of 
the sum will never be used, no matter whether it's stored in a 64bit 
variable or just discarded.
OK, so maybe the check should be if(s->bps + qlevel > 32)

--Loren Merritt