[FFmpeg-devel] [PATCH/RFC] Remove triplication of compute_lpc_coefs() function

[Vitor Sessak]

Hi

Michael Niedermayer wrote:
> On Thu, Aug 28, 2008 at 02:28:50PM +0200, Vitor Sessak wrote:
>> Hi all.
>>
>> I've finally found a more or less clean way of doing $subj. I also want to 
>> know if it is ok to remove the following useless loop:
>>
>>> for(i=0; i<max_order; i++) lpc_tmp[i] = 0;
> 
> all useless loops can be removed ...

Gone.

> 
> 
> Some comments ...
> Is the double based code really a problem speedwise? IIRC someone (mans?)
> said it was very slow on some ARM, but that considered flac float vs.
> double which included the autocorrelation stuff, not just
> compute_lpc_coefs() float vs. double IIRC.

Even if compute_lpc_coefs() is not speed critical, to convert all 
input/output buffers to double/float before passing to the function is 
ugly and slow.

> It should be clarified how much time is actually spend in this code when
> encoding flac.
> 
> Also iam still curious if all variables must be double to maintain the good
> compression with flac ...

The following patch makes the flac encoder use a float version of 
compute_lpc_coefs() but keeps doubles for the intermediate variables. In 
my tests I've not seem a worsening of the encoded size, but I'd be glad 
if someone could test it independently.

> 
> [...]
> 
>> +    for(i=0; i<max_order; i++) {
>> +        if (normalize) {
>> +            r = -autoc[i+1];
>> +            for(j=0; j<i; j++) {
>> +                r -= lpc_tmp[j] * autoc[i-j];
>> +            }
>> +            r /= err;
>> +        } else
>> +            r = -autoc[i];
>> +
>> +        if (ref)
>> +            ref[i] = fabs(r);
>> +
>> +        err *= 1.0 - (r * r);
>> +
>> +        i2 = (i >> 1);
>> +        lpc_tmp[i] = r;
>> +        for(j=0; j<i2; j++) {
>> +            tmp = lpc_tmp[j];
>> +            lpc_tmp[j] += r * lpc_tmp[i-1-j];
>> +            lpc_tmp[i-1-j] += r * tmp;
>> +        }
>> +        if(i & 1) {
>> +            lpc_tmp[j] += lpc_tmp[j] * r;
>> +        }
>> +
>> +        if (lpc)
>> +            for(j=0; j<=i; j++) {
>> +                lpc[i][j] = -lpc_tmp[j];
>> +            }
> 
> that - can be avoided by fliping a few signs above

I'm trying to keep it the closest as possible to the original function. 
Would you prefer if I diff it against the original lpc.c?

-Vitor
-------------- next part --------------
A non-text attachment was scrubbed...
Name: lpc_float.c
Type: text/x-csrc
Size: 2477 bytes
Desc: not available
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20080829/16e91258/attachment.c>