[FFmpeg-devel] Review request - ra288.{c,h} ra144.{c,h}

Vitor Sessak vitor1001
Wed Aug 6 06:34:58 CEST 2008


Justin Ruggles wrote:
> Justin Ruggles wrote:
>> Vitor Sessak wrote:
>>> Michael Niedermayer wrote:
>>>> On Tue, Jul 29, 2008 at 08:20:45PM +0200, Vitor Sessak wrote:
>>>>> Hi,
>>>>>
>>>>> Those four files never passed a review. I've just finished cleaning them 
>>>>> up, so if anyone wants to review them (Michael already said he will), 
>>>>> now is time.
>>>> heres 288
>>>>
>>>> [...]
>>>>> static void decode(RA288Context *ractx, float gain, int cb_coef)
>>> [...]
>>>
>>>>> /**
>>>>>  * Converts autocorrelation coefficients to LPC coefficients using the
>>>>>  * Levinson-Durbin algorithm. See blocks 37 and 50 of the G.728 specification.
>>>>>  *
>>>>>  * @return 0 if success, -1 if fail
>>>>>  */
>>>>> static int eval_lpc_coeffs(const float *in, float *tgt, int n)
>>>>> {
>>>>>     int i, j;
>>>>>     double f0, f1, f2;
>>>>>
>>>>>     if (in[n] == 0)
>>>>>         return -1;
>>>>>
>>>>>     if ((f0 = *in) <= 0)
>>>>>         return -1;
>>>>>
>>>>>     in--; // To avoid a -1 subtraction in the inner loop
>>>>>
>>>>>     for (i=1; i <= n; i++) {
>>>>>         f1 = in[i+1];
>>>>>
>>>>>         for (j=0; j < i - 1; j++)
>>>>>             f1 += in[i-j]*tgt[j];
>>>>>
>>>>>         tgt[i-1] = f2 = -f1/f0;
>>>>>         for (j=0; j < i >> 1; j++) {
>>>>>             float temp = tgt[j] + tgt[i-j-2]*f2;
>>>>>             tgt[i-j-2] += tgt[j]*f2;
>>>>>             tgt[j] = temp;
>>>>>         }
>>>>>         if ((f0 += f1*f2) < 0)
>>>>>             return -1;
>>>>>     }
>>>>>
>>>>>     return 0;
>>>>> }
>>>> duplicate of compute_lpc_coefs() ?
>>> Yes, the two functions are practically identical, but 
>>> compute_lpc_coefs() use doubles and ra288 uses floats.
>>>
>>> Justin: does FLAC really need double precision or that was just an 
>>> arbitrary choice?
>> I know I ran some compression comparison tests at one point, and using
>> floats vs. doubles did negatively affect compression.  How much, I don't
>> remember.  I'll test again and post the results.  If it's not too bad,
>> I'm ok with changing to floats.
> 
> test of 5 concatenated samples.
> total playing time: 26m47s
> 
> system: athlon64 x2 6000+, linux, gcc 4.1.2
> 
> compression level 5:
> double  -- 6.224s -- 171183921
> no sse2 -- 6.600s -- 171183931
> float   -- 6.452s -- 171839999 = 0.4% larger
> 
> compression level 8:
> double  -- 13.385s -- 167634457
> no sse2 -- 13.981s -- 167634492
> float   -- 13.857s -- 168444919 = 0.4% larger
> 
> 
> I'm guessing that Loren's optimized functions could be adapted to use
> floats to give better results.

Thanks for digging into this. I'm not sure if 2% faster for a 0.4% 
larger file is preferred (even more so if it cannot be fixed by 
increasing the compression level). Also, this trade-off only make sense 
supposing that we get the same speed gain after adapting the optimized 
functions for float (what I can't do myself, I don't know asm).

I'm attaching a patch to factorize the code (with macros, keeping both 
the float and double versions). I've tried to do it in the least hackish 
possible way, but suggestions are welcome. It gives binary identical 
output for both ra288 decoding and flac encoding.

-Vitor
-------------- next part --------------
A non-text attachment was scrubbed...
Name: levinson.diff
Type: text/x-diff
Size: 10859 bytes
Desc: not available
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20080806/e71020be/attachment.diff>



More information about the ffmpeg-devel mailing list