[FFmpeg-devel] [PATCH] Common ACELP code & G.729 [6/7] - G.729 postfilter

[Vladimir Voroshilov]

2008/5/8 Michael Niedermayer <michaelni at gmx.at>:
> On Fri, May 02, 2008 at 06:49:43PM +0700, Vladimir Voroshilov wrote:
>> Patch contains G.729 postfilter.
>> It was separated due to large size to help reviewing.
>> G.729 can produce audible speech even without this postfilter.
>>
>> --
>> Regards,
>> Vladimir Voroshilov mailto:voroshil at gmail.com
>> JID: voroshil at gmail.com, voroshil at jabber.ru
>> ICQ: 95587719
>
>> diff --git a/libavcodec/g729postfilter.c b/libavcodec/g729postfilter.c
>> new file mode 100644
>> index 0000000..b09d463
>> --- /dev/null
>> +++ b/libavcodec/g729postfilter.c
>> @@ -0,0 +1,704 @@
>> +#include <inttypes.h>
>> +#include <limits.h>
>> +
>> +#include "avcodec.h"
>> +#include "g729.h"
>> +#include "acelp_pitch_lag.h"
>> +#include "g729postfilter.h"
>> +#include "acelp_math.h"
>> +#include "acelp_filters.h"
>> +
>> +#define FRAC_BITS 15
>> +#include "mathops.h"
>> +
>
>> +/**
>> + * formant_pp_factor_num_pow[i] = FORMANT_PP_FACTOR_NUM^i
>> + */
>> +static const int16_t formant_pp_factor_num_pow[11]=
>> +{
>> +  /* (0.15) */
>> +  32768, 18022, 9912, 5451, 2998, 1649, 907, 499, 274, 151, 83
>     ^^^^^
> doesnt fit in int16_t, it does fir in uint16_t though

Reduced by 1

>
>
> [...]
>> +static void g729_residual(int16_t* filter_coeffs, const int16_t* speech, int16_t* residual, int subframe_size, int16_t* res_filter_data)
>> +{
>> +    int i, n, sum;
>> +
>> +    /*
>> +      4.2.1, Equation 79 Residual signal calculation
>> +      ( filtering through A(z/FORMANT_PP_FACTOR_NUM) , one half of short-term filter)
>> +    */
>
>> +    for(n=0; n<subframe_size; n++)
>> +    {
>> +        sum = res_filter_data[10+n] = speech[n];
>
> this is effectively a memcpy() and i do not like memcpy() ...

Filter is rewritten in similar way to another filters in acelp_filter.c
This added several small memcpy, though.

>> +        sum <<= 12;
>> +        for(i=0; i<10; i++)
>> +            sum += filter_coeffs[i] * res_filter_data[10+n-i-1];
>> +
>> +        residual[n] = (sum + 0x800) >> 12;
>
> sum= x<<12
> sum += blah
> y= (sum+z) >> 12;
>
> can be changed to
>
> sum= z
> sum += blah
> y= (sum >> 12) + x;
>
> which does 1 shift less

done.

> [...]
>> +    if (shift>0)
>> +        for(i=0; i<subframe_size + RES_PREV_DATA_SIZE; i++)
>> +         sig_scaled[i] = residual[i] >> shift;
>
> tabs

removed.

> [...]
>> +        /*
>> +          Recompute delayed signal with an interpolation filter of length 129
>> +        */
>> +        for(n=0; n<subframe_size; n++)
>> +        {
>> +            sum = 0;
>> +            for(i=0;i<LONG_INT_FILT_LEN; i++)
>> +                sum += ff_g729_interp_filt_long[best_delay_frac-1][i] *
>> +                       sig_scaled[n - (best_delay_int + 1 - delayed_signal_offset) + (LONG_INT_FILT_LEN/2 - i) + RES_PREV_DATA_SIZE];
>> +            residual_filt[n] = (sum + 0x4000) >> 15;
>> +        }
>
> duplicate, there already was some interpolation code somewhere else in g729

fixed. see also changes in acelp_vectors.


>> +static int16_t g729_get_tilt_comp(int16_t *lp_gn, const int16_t *lp_gd, int16_t* speech, int subframe_size)
>> +{
>> +    int rh1,rh0; // Q12
>> +    int sum, temp;
>> +    int i, n;
>> +    int gain_term; //gain term for short-term postfilter
>> +
>> +    lp_gn[10] = 4096; //1.0 in Q12
>> +
>> +    /* Apply 1/A(z/FORMANT_PP_FACTOR_DEN) filter to hf */
>
>> +    for(n=0; n<22; n++)
>> +    {
>> +        sum = lp_gn[n+11] << 12;
>> +        for(i=0; i<10; i++)
>> +            sum -= lp_gd[i+1] * lp_gn[n+11-i-1];
>> +        sum = av_clip(sum, -0x08000000, 0x07ffffff);
>> +
>> +        lp_gn[n+11] = (sum + 0x800) >> 12;
>> +    }
>
> This filter looks duplicated as well

fixed.

>
>> +    /* Now lp_gn (starting with 10) contains impulse response of A(z/FORMANT_PP_FACTOR_NUM)/A(z/FORMANT_PP_FACTOR_DEN) filter */
>> +
>
>> +    /* 4.2.3, Equation 87, calcuate rh(0)  */
>> +    rh0 = sum_of_squares(lp_gn + 10, 20, 0, 0) << 1;   // Q24 -> Q9
>
>> +    temp = 30 - av_log2(rh0);
>> +    if(temp > 16)
>> +        rh0 <<= temp - 16;
>> +    else
>> +        rh0 >>= 16 - temp;
>> +
> [...]
>> +    if(temp > 16)
>> +        rh1 <<= temp - 16;
>> +    else
>> +        rh1 >>= 16 - temp;
>
> looks useless, this just reduces precission

This shift guaranties that later "(rh1<<15)/rh0" will
not cause int overflow.

>
>
> [...]
>> +            if(exp_after - exp_before - 1 > 0)
>> +                gain <<= exp_after - exp_before - 1;
>> +            else
>> +                gain >>= exp_before - exp_after + 1;
>
> This stuff is duplicated all over the place and should be in a fuction

Fixed.


> Anyway the whole postfilter looks like it needs some serious cleanup.

Sure.
Postfilter is most complicated part of entire G.729 decoder.
And good cleanup is not made for it yet.



-- 
Regards,
Vladimir Voroshilov mailto:voroshil at gmail.com
JID: voroshil at gmail.com, voroshil at jabber.ru
ICQ: 95587719
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 06_g729post_49.diff
Type: text/x-diff
Size: 27205 bytes
Desc: not available
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20080511/f22fdf92/attachment.diff>