[FFmpeg-devel] [PATCH] Move pitch vector interpolation filter to acelp_filters.

[Vladimir Voroshilov]

2008/5/19 Vladimir Voroshilov <voroshil at gmail.com>:
> 2008/5/18 Michael Niedermayer <michaelni at gmx.at>:
>
>
> > On Sun, May 18, 2008 at 12:23:32AM +0700, Vladimir Voroshilov wrote:
>  >
>  > > Attached patch does $subj.
>  >  >
>  >  > Filter was extended recently and now receives
>  >  > filter coefficients and filter length as parameters.
>  >  >
>  >  > Since this makes the filter more common, Michael suggested to move it
>  >  > along with used filter tables from acelp_vectors to acelp_filter.
>  >  [...]
>  >  > +const int16_t ff_acelp_interp_filter[66] =
>  >  > +{ /* (0.15) */
>  >  > +  29443, 28346, 25207, 20449, 14701,  8693,
>  >  > +   3143, -1352, -4402, -5865, -5850, -4673,
>  >  > +  -2783,  -672,  1211,  2536,  3130,  2991,
>  >  > +   2259,  1170,     0, -1001, -1652, -1868,
>  >  > +  -1666, -1147,  -464,   218,   756,  1060,
>  >  > +   1099,   904,   550,   135,  -245,  -514,
>  >  > +   -634,  -602,  -451,  -231,     0,   191,
>  >  > +    308,   340,   296,   198,    78,   -36,
>  >  > +   -120,  -163,  -165,  -132,   -79,   -19,
>  >  > +     34,    73,    91,    89,    70,    38,
>  >  > +      0,     0,     0,     0,     0,     0,
>  >  > +};
>  >  > +
>  >  > +const int16_t ff_g729_interp_filt_short[(ANALYZED_FRAC_DELAYS+1)*(SHORT_INT_FILT_LEN+1)] =
>  >  > +{
>  >  > +      0, 31650, 28469, 23705, 18050, 12266,  7041,  2873,
>  >  > +      0, -1597, -2147, -1992, -1492,  -933,  -484,  -188,
>  >  > +      0,     0,     0,     0,     0,     0,     0,     0,
>  >  > +};
>  >  > +
>  >  > +const int16_t ff_g729_interp_filt_long[(ANALYZED_FRAC_DELAYS+1)*(LONG_INT_FILT_LEN+1)] =
>  >  > +{
>  >  > +   0, 31915, 29436, 25569, 20676, 15206,  9639,  4439,
>  >  > +   0, -3390, -5579, -6549, -6414, -5392, -3773, -1874,
>  >  > +   0,  1595,  2727,  3303,  3319,  2850,  2030,  1023,
>  >  > +   0,  -887, -1527, -1860, -1876, -1614, -1150,  -579,
>  >  > +   0,   501,   859,  1041,  1044,   892,   631,   315,
>  >  > +   0,  -266,  -453,  -543,  -538,  -455,  -317,  -156,
>  >  > +   0,   130,   218,   258,   253,   212,   147,    72,
>  >  > +   0,   -59,  -101,  -122,  -123,  -106,   -77,   -40,
>  >  > +   0,     0,     0,     0,     0,     0,     0,     0,
>  >  > +};
>  >
>  >  "ff_g729*" tables should be in g729 related files so they are
>  >  not included in the binary if g729 is disabled but some other ACELP codec
>  >  is compiled in
>  >
>  >
>  >  > +
>  >  > +void ff_acelp_interpolate_pitch_vector(
>  >  > +        int16_t* out,
>  >  > +        const int16_t* in,
>  >  > +        const int16_t* filter_coeffs,
>  >  > +        int precision,
>  >  > +        int pitch_delay_int,
>  >  > +        int pitch_delay_frac,
>  >  > +        int filter_length,
>  >  > +        int subframe_size)
>  >  > +{
>  >  > +    int n, i;
>  >  > +
>  >  > +    /* pitch_delay_frac [0; 5]
>  >  > +       pitch_delay_int  [PITCH_LAG_MIN; PITCH_LAG_MAX] */
>  >  > +    for(n=0; n<subframe_size; n++)
>  >  > +    {
>  >  > +        int idx = precision;
>  >  > +        /* 3.7.1 of G.729, Equation 40 */
>  >  > +        int v = in[n - pitch_delay_int] * filter_coeffs[FFABS(pitch_delay_frac)];
>  >  > +        for(i=1; i<filter_length+1; i++)
>  >  > +        {
>  >  > +
>  >  > +            /* The reference G.729 and AMR fixed point code performs clipping after
>  >  > +               each of the two following accumulations.
>  >  > +               Since clipping affects only the synthetic OVERFLOW test without
>  >  > +               causing an int type overflow, it was moved outside the loop. */
>  >  > +
>  >  > +            /*  R(x):=ac_v[-k+x]
>  >  > +                v += R(n-i)*ff_acelp_interp_filter(t+6i)
>  >  > +                v += R(n+i+1)*ff_acelp_interp_filter(6-t+6i) */
>  >  > +
>  >  > +            v += in[n - pitch_delay_int - i] * filter_coeffs[idx - pitch_delay_frac];
>  >  > +            v += in[n - pitch_delay_int + i] * filter_coeffs[idx + pitch_delay_frac];
>  >  > +            idx += precision;
>  >  > +        }
>  >  > +        out[n] = av_clip_int16((v + 0x4000) >> 15);
>  >  > +    }
>  >  > +}
>  >
>  >  This code is written for a filter with an odd number of coeffs (1,3,5,...) but
>  >  all but the first filter for which it is used have an even number thus they
>  >  end up needing these 0 coeffs.
>  >  The code should be changed so it expects even number of coeff filters and the
>  >  unneeded 0 elements should be removed from the tables
>  >
>  >  anyway, the obvious implementation is:
>  >
>  >  for(n=0; n<subframe_size; n++){
>  >     int idx = precision>>1;
>  >     int v = 0x4000;
>  >     for(i=1; i<filter_length+1; i++){
>  >         v += in[n - pitch_delay_int - i] * filter_coeffs[idx - frac];
>  >         v += in[n - pitch_delay_int + i] * filter_coeffs[idx + frac];
>  >         idx += precision;
>  >     }
>  >     out[n] = av_clip_int16(v >> 15);
>
>  First, I can't see here equivalent  (due to i>0) for
>
>     int v = in[n - pitch_delay_int] * filter_coeffs[FFABS(pitch_delay_frac)];
>
>  Second, your code 'as is' gives me PSNR 10 and i'm afraid it is totally wrong:
>  you have shifted filter represented in filter_coeffs by precision/2
>  as respect signal. This will obviously produce different result.

Sorry, i overreacted with 'totally wrong'.
I'm still investingating problem, but already found several
inconsistences between doxygen comment to the filter and it's usage
(like passing [-5; 0] value
in pitch_frac_delay).

Code is not looks like yours yet, but is going to that direction.

-- 
Regards,
Vladimir Voroshilov mailto:voroshil at gmail.com
JID: voroshil at gmail.com, voroshil at jabber.ru
ICQ: 95587719