[FFmpeg-devel] [PATCH] Common ACELP routines (3/3) - LPC decoding routines

[Robert Swain]

On 23 Apr 2008, at 20:53, Vladimir Voroshilov wrote:
> Michael Niedermayer wrote:
>> On Tue, Apr 22, 2008 at 01:21:24AM +0700, Vladimir Voroshilov wrote:
>>> +void ff_acelp_reorder_lsf(int16_t* lsfq, int16_t  
>>> lsfq_min_distance, int16_t lsfq_min, int16_t lsfq_max)
>>> +{
>>> +    int i;
>>> +
>>> +    lsfq[0] = FFMAX(lsfq[0], lsfq_min); //Is warning required ?
>>> +
>>> +    for(i=0;i<9; i++)
>>> +        lsfq[i+1] = FFMAX(lsfq[i+1], lsfq[i] + lsfq_min_distance);
>>
>> simplification stolen from soc/amrnbdec.c:
>>
>> for(i=0; i<10; i++){
>>    lsf[i] = FFMAX(lsf[i], lsf_min);
>>    lsf_min = lsf[i] + min_distance;
>> }
>>
>> also id make lsfq_min_distance and lsfq_min/max int
>>
>> ahh, and reorder_lsf() in soc/amrnbdec.c is buggy (uninitalized var)
>
> It also assumes that LSFs already ordered.
> G.729 explicitly sorts them.
> Should i put sorting code inside routine?

You may have spotted a bug in my implementation. I don't recall. I  
think I remember when working on this in the fixed point decoder that  
I thought it was strange and assumed the LSFs were ordered when read  
from the tables but it's a very hazy memory.

>>> +
>>> +    lsfq[9] = FFMIN(lsfq[9], lsfq_max);//Is warning required ?
>>> +}
>>> +
>>
>>> +/**
>>> + * \brief Convert LSF to LSP
>>> + * \param lsf (Q13) LSF coefficients (0 <= lsf < PI)
>>
>> ff_acelp_cos() behaves like PI=0x8000 or so IIRC so above is not  
>> really
>> correct
>
> Fixed by dividing on PI
> There is also some unclear part in AMR: spec says about cos(PI*lsf)
> while floating point code uses cos(lsf).
> Robert, please recheck this part or fix me if i'm wrong

/**
  * Convert an lsf vector into an lsp vector
  *
  * @param lsf               input lsf vector
  * @param lsp               output lsp vector
  *
  * @return void
  */

static void lsf2lsp(float *lsf, float *lsp) {
     int i;

     for(i=0; i<LP_FILTER_ORDER; i++) {
         lsp[i] = cos(lsf[i]*FREQ_LSP_FAC); // FREQ_LSP_FAC = 2*M_PI/ 
8000.0
     }
}

Take note of the comment. Is that what you meant?

>>> + * \param lsp [out] (Q15) LSP coefficients (-1 <= lsp < 1)
>>> + *
>>> + * \remark It is safe to pass the same array in lsf and lsp  
>>> parameters
>>> + */
>>> +void ff_acelp_lsf2lsp(const int16_t *lsf, int16_t *lsp, int16_t  
>>> factor)
>>
>> i slightly prefer that the written to argument is left (lsp, lsf,  
>> factor)
>> same for the other functions (this of course is minor nitpicking ...)
>
> ok. Remaining fixes of ordering will be made while touching code.

I prefer them ordered with inputs on the left and outputs on the  
right, but it's a matter of taste and wouldn't matter too much if they  
were randomly ordered.

Rob