[Ffmpeg-cvslog] r5507 - trunk/libavcodec/flac.c
Loren Merritt
lorenm
Wed Jun 21 05:07:50 CEST 2006
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
More information about the ffmpeg-cvslog
mailing list