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

[Vitor Sessak]

Michael Niedermayer wrote:
> On Mon, Sep 01, 2008 at 07:03:23AM +0200, Vitor Sessak wrote:
>> Michael Niedermayer wrote:
>>> On Sun, Aug 31, 2008 at 05:08:26AM +0200, Vitor Sessak wrote:
>>>> 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. 
>> [...]
>>
>>> for(i=0; i<max_order; i++) {
>>>     LPC_type r = -autoc[i+1];
>>>     LPC_type *lpc_last= lpc;
>>>     lpc += lpc_stride;
>>>     if (normalize) {
>>>         for(j=0; j<i; j++)
>>>             r -= lpc_last[j] * autoc[i-j];
>>>         r /= err;
>>>         err *= 1.0 - (r * r);
>>>     }
>>>     if (ref)
>>>         ref[i] = fabs(r);
>>>     lpc[i] = r;
>>>     for(j=0; j < (i+1)>>1; j++) {
>>>         LPC_type f = lpc_last[    j];
>>>         LPC_type b = lpc_last[i-1-j];
>>>         lpc[    j] = f + r * b;
>>>         lpc[i-1-j] = b + r * f;
>>>     }
>>>     if (fail && err < 0)
>>>         return -1;
>>> }
>> I like it, done.
>>
>>> [...]
>>>> Index: libavcodec/lpc.h
>>>> ===================================================================
>>>> --- libavcodec/lpc.h	(revision 15050)
>>>> +++ libavcodec/lpc.h	(working copy)
>>>> @@ -45,4 +45,66 @@
>>>>                        int32_t coefs[][MAX_LPC_ORDER], int *shift, int 
>>>> use_lpc,
>>>>                        int omethod, int max_shift, int zero_shift);
>>>>  +
>>>> +#ifdef LPC_USE_DOUBLE
>>>> +#define LPC_type double
>>>> +#else
>>>> +#define LPC_type float
>>>> +#endif
>>>> +
>>>> +/**
>>>> + * Levinson-Durbin recursion.
>>>> + * Produces LPC coefficients from autocorrelation data.
>>>> + */
>>>> +static inline int compute_lpc_coefs(const LPC_type *autoc,
>>>> +                                    int max_order,
>>>> +                                    LPC_type lpc[][MAX_LPC_ORDER],
>>>> +                                    LPC_type *ref,
>>>> +                                    LPC_type lpc_tmp[MAX_LPC_ORDER],
>>>> +                                    int fail, int normalize)
>>>> +{
>>>> +    int i, j;
>>>> +    LPC_type err = autoc[0];
>>>> +
>>>> +    if (fail && (autoc[max_order] == 0 || autoc[0] <= 0))
>>>> +        return -1;
>>>> +
>>>> +    for(i=0; i<max_order; i++) {
>>>> +        LPC_type r;
>>>> +        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];
>>> This can be factored out of the loop when autoc+1 is passed as argument
>>> instead of autoc for !normalize
>> I don't like it because I'll need to check autoc[-1] in the beginning...
>>
>>>> +
>>>> +        if (ref)
>>>> +            ref[i] = fabs(r);
>>>> +
>>>> +        err *= 1.0 - (r * r);
>>> This line can be under if(normalize) above
>> done.
>>
>> New version attached.
> [...]
>> Index: libavcodec/lpc.h
>> ===================================================================
>> --- libavcodec/lpc.h	(revision 15050)
>> +++ libavcodec/lpc.h	(working copy)
>> @@ -45,4 +45,62 @@
>>                        int32_t coefs[][MAX_LPC_ORDER], int *shift, int use_lpc,
>>                        int omethod, int max_shift, int zero_shift);
>>  
>> +
>> +#ifdef LPC_USE_DOUBLE
>> +#define LPC_type double
>> +#else
>> +#define LPC_type float
>> +#endif
>> +
>> +/**
>> + * Levinson-Durbin recursion.
>> + * Produces LPC coefficients from autocorrelation data.
>> + */
>> +static inline int compute_lpc_coefs(const LPC_type *autoc, int max_order,
>> +                                    LPC_type *lpc, LPC_type *ref,
>> +                                    int lpc_stride, int fail, int normalize)
>> +{
>> +    int i, j;
>> +    LPC_type err;
>> +    LPC_type *lpc_last = lpc;
>> +
>> +    if (normalize)
>> +        err = *autoc++;
>> +
>> +    if (fail && (autoc[max_order - 1] == 0 || err <= 0))
>> +        return -1;
>> +
>> +    for(i=0; i<max_order; i++) {
>> +        LPC_type r = -autoc[i];
>> +
>> +        if (normalize) {
>> +            for(j=0; j<i; j++)
>> +                r -= lpc_last[j] * autoc[i-j-1];
>> +
>> +            r /= err;
>> +            err *= 1.0 - (r * r);
>> +        }
>> +
>> +        if (ref)
>> +            ref[i] = fabs(r);
>> +
>> +        lpc[i] = r;
>> +
>> +        for(j=0; j < (i+1)>>1; j++) {
>> +            LPC_type f = lpc_last[    j];
>> +            LPC_type b = lpc_last[i-1-j];
>> +            lpc[    j] = f + r * b;
>> +            lpc[i-1-j] = b + r * f;
>> +        }
> 
> if iam not missing someting than the ref values get already exported in lpc[]
> ignoring the fabs() thus ref might not be needed ...
> 
> And I think iam fine with the patch after that

Applied without ref[].

-Vitor