[FFmpeg-devel] [PATCH] Common ACELP code & G.729 [2/7] - pitch lag decoding

[Vladimir Voroshilov]

2008/6/27 Michael Niedermayer <michaelni at gmx.at>:
> On Fri, Jun 27, 2008 at 08:30:56PM +0700, Vladimir Voroshilov wrote:
>> 2008/6/27 Michael Niedermayer <michaelni at gmx.at>:
>> > On Fri, Jun 27, 2008 at 06:30:00AM +0700, Vladimir Voroshilov wrote:
>> >> 2008/6/27 Michael Niedermayer <michaelni at gmx.at>:
>> >> > On Fri, Jun 27, 2008 at 12:00:01AM +0700, Vladimir Voroshilov wrote:
>> >>
>> >> [...]
>> >>
>> >> >> +
>> >> >> +    for(i=0; i<ma_pred_order; i++)
>> >> >> +        energy += quant_energy[i] * ma_prediction_coeff[i];
>> >> >> +
>> >> >> +    innov_energy = sum_of_squares(fc_v, subframe_size, 0, 0);
>> >> >> +#ifdef G729_BITEXACT
>> >> >
>> >> >> +    energy +=  MULL(ff_log2(innov_energy), -24660) << 10;
>> >> >
>> >> > s/24660/6165/
>> >> > and i wonder if a normal * would do with the correctly simplified constant?
>> >>
>> >> Breaks bitexactness.
>> >
>> > you mean (x*2)>>1 != x then you have a overflow in your code
>>
>> No, i meant ((x*2)>>2)<<1 != (x>>2)<<2
>> You didn't take in account MULL macro.
>> The reason is rounding, not overflow.
>
> you should not use MULL!
> if you do use it, you would have to use it correctly, its not ok to
> multiply all constants by 4 because you forgot a #define at the top of
> your code. But as said you should not use MULL at all, use
> *6165 or *6165LL

I've rechecked code again.
And you was right :)
There is 32bit overflow too. 6165LL solves it, but
I have question about rounding. What is better:
(a >> 13)<<10
or
(a>>3) & ~0x3ff
?
Such code is required for bitexactness.


-- 
Regards,
Vladimir Voroshilov mailto:voroshil at gmail.com
JID: voroshil at gmail.com, voroshil at jabber.ru
ICQ: 95587719