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

[Michael Niedermayer]

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

[...]
-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

I count him braver who overcomes his desires than him who conquers his
enemies for the hardest victory is over self. -- Aristotle
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 189 bytes
Desc: Digital signature
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20080903/e7056c9b/attachment.pgp>