[FFmpeg-devel] [PATCH] do not hardcode filter order in ff_acelp_lspd2lpc()

[Vitor Sessak]

Michael Niedermayer wrote:
> On Sat, Nov 07, 2009 at 06:09:07PM +0100, Vitor Sessak wrote:
>> Michael Niedermayer wrote:
>>> On Sat, Nov 07, 2009 at 12:03:39AM +0100, Vitor Sessak wrote:
>>>> Michael Niedermayer wrote:
>>>>> On Thu, Nov 05, 2009 at 05:36:36AM +0100, Vitor Sessak wrote:
>>>>>> Reynaldo H. Verdejo Pinochet wrote:
>>>>>>> Hello Vitor
>>>>>>> Vitor Sessak wrote:
>>>>>>>> $subj. It is useful for SIPR but also it makes sense to make shared
>>>>>>>> functions as general as possible.
>>>>>>> QCELP part is ok but I'd make lp_half_order
>>>>>>> a single-byte sized type.
>>>>>> Fine for me (I suppose comparing uint8_t with int is not slower in any 
>>>>>> arch we support). New version attached.
>>>>>>
>>>>>> -Vitor
>>>>>>  lsp.c      |   12 ++++++------
>>>>>>  lsp.h      |    2 +-
>>>>>>  qcelpdec.c |    2 +-
>>>>>>  3 files changed, 8 insertions(+), 8 deletions(-)
>>>>>> 4c16615dbbd30c5ab006c8da8b879ea577097b39  lspd2lpc_2.diff
>>>>>> Index: libavcodec/lsp.c
>>>>>> ===================================================================
>>>>>> --- libavcodec/lsp.c	(revision 20450)
>>>>>> +++ libavcodec/lsp.c	(working copy)
>>>>>> @@ -155,20 +155,20 @@
>>>>>>      }
>>>>>>  }
>>>>>>  -void ff_acelp_lspd2lpc(const double *lsp, float *lpc)
>>>>>> +void ff_acelp_lspd2lpc(const double *lsp, float *lpc, uint8_t 
>>>>>> lp_half_order)
>>>>>>  {
>>>>>> -    double pa[6], qa[6];
>>>>>> +    double pa[lp_half_order+1], qa[lp_half_order+1];
>>>>>>      int   i;
>>>>>>  -    lsp2polyf(lsp,     pa, 5);
>>>>>> -    lsp2polyf(lsp + 1, qa, 5);
>>>>>> +    lsp2polyf(lsp,     pa, lp_half_order);
>>>>>> +    lsp2polyf(lsp + 1, qa, lp_half_order);
>>>>>>  -    for (i=4; i>=0; i--)
>>>>>> +    for (i=lp_half_order-1; i>=0; i--)
>>>>>>      {
>>>>>>          double paf = pa[i+1] + pa[i];
>>>>>>          double qaf = qa[i+1] - qa[i];
>>>>>>           lpc[i  ] = 0.5*(paf+qaf);
>>>>>> -        lpc[9-i] = 0.5*(paf-qaf);
>>>>>> +        lpc[(lp_half_order << 1) - 1 -i] = 0.5*(paf-qaf);
>>>>>>      }
>>>>> i would
>>>>> lpc2= lpc + (lp_half_order << 1) - 1;
>>>>> and
>>>>> lpc2[-i] = ...
>>>>> that way lp_half_order and i also become redundant and one can be droped
>>>>> about int8 vs. int, its possible that using int8 as index into an array
>>>>> like lpc[int8_variable] could be slower in some archs (x86 being one)
>>>> Since pa[] is an array not a pointer I cannot do the 
>>>> while(lp_half_order--) { (...) pa++; ga++; lpc++; lpc2--;} trick not to 
>>>> use the var "i" (and the other option would use lpc[int8_var]). In this 
>>>> new version I've also made the loop count forward to make it more 
>>>> readable.
>>> you could change lp_half_order back to int if reynaldo doesnt mind.
>>> i think that would avoid the problem?
>> Yes, new patch attached.
>>
>> -Vitor
> 
>>  lsp.c      |   21 ++++++++++-----------
>>  lsp.h      |    2 +-
>>  qcelpdec.c |    2 +-
>>  3 files changed, 12 insertions(+), 13 deletions(-)
>> f232b1334e175b5427aa5642576cfb3c1adff534  lspd2lpc_4.diff
> 
> iam ok with the things iam maintainer for

Applied with an extra comment based on a suggestion of Reynaldo.

-Vitor