[FFmpeg-devel] [PATCH] Common ACELP routines (2/3) - filters

[Michael Niedermayer]

On Tue, Apr 22, 2008 at 02:18:11AM +0700, Vladimir Voroshilov wrote:
> On Tue, Apr 22, 2008 at 2:08 AM, Diego Biurrun <diego at biurrun.de> wrote:
> > On Tue, Apr 22, 2008 at 01:21:16AM +0700, Vladimir Voroshilov wrote:
> >  > This patch contains various ACELP filter-rrelated routines like LP synthesis,
> >  > weighted, excitation interpolation, fixed vector phase dispertion.
> >  >
> >  > --- /dev/null
> >  > +++ b/libavcodec/acelp_filt.c
> >
> >  acelp_filter.c would be a better filename IMO, characters are cheap
> >  these days :)
> 
> renamed to acelp_filters
[...]
> +const int16_t ff_g729_interp_filter[11][3] =
> +{ /* Q15 */
> +  { 29443,   25207,   14701},
> +  {  3143,   -4402,   -5850},
> +  { -2783,    1211,    3130},
> +  {  2259,       0,   -1652},
> +  { -1666,    -464,     756},
> +  {  1099,     550,    -245},
> +  {  -634,    -451,       0},
> +  {   308,     296,      78},
> +  {  -120,    -165,     -79},
> +  {    34,      91,      70},
> +  {     0,       0,       0},
> +};
> +
> +/**
> + *   b60[n]:= b(60)[n]
> + */
> +static const int16_t b60[11][6] =
> +{
> +  { 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},
> +};

The second table contains the first. Thus the first can be droped.


> +
> +/**
> + * \brief Decoding of the adaptive-codebook vector (4.1.3)
> + * \param pitch_delay_3x pitch delay with 1/3 precision, mutiplied by 3
> + * \param ac_v [out] (Q0) buffer to store decoded vector into
> + * \param subframe_size length of subframe
> + */
> +void ff_acelp_interpolate_excitation(int pitch_delay_3x, int16_t* ac_v, int subframe_size)
> +{
> +    int n, i;
> +    int v;
> +    // TODO: clarify why used such expression (hint: -1/3 , 0 ,1/3 order in interpol_filter)
> +    int pitch_delay_frac = 1 - (pitch_delay_3x%3);
> +    int pitch_delay_int = pitch_delay_3x / 3;
> +
> +    //Make sure that pitch_delay_frac will be always positive
> +    if(pitch_delay_frac < 0)
> +    {
> +        pitch_delay_frac += 3;
> +        pitch_delay_int++;
> +    }
> +
> +    //pitch_delay_frac [0, 1, 2]
> +    //pitch_delay_int  [PITCH_LAG_MIN-1; PITCH_LAG_MAX]
> +    for(n=0; n<subframe_size; n++)
> +    {

> +        /* 3.7.1, Equation 40 */

as this file is not g729 specific, the is ambiguous, g729 should be
mentioned as well.


> +        v=0;
> +        for(i=0; i<10; i++)
> +        {

> +            /*  R(x):=ac_v[-k+x] */
> +            v += ac_v[n - pitch_delay_int - i    ] * ff_g729_interp_filter[i][    pitch_delay_frac];
> +            v = av_clip(v, -0x40000000, 0x3fffffff); //v += R(n-i)*ff_g729_interp_filter(t+3i)
> +            v += ac_v[n - pitch_delay_int + i + 1] * ff_g729_interp_filter[i][3 - pitch_delay_frac];
> +            v = av_clip(v, -0x40000000, 0x3fffffff); //v += R(n+i+1)*ff_g729_interp_filter(3-t+3i)

The cliping is incorrect for generic code. Also i doubt g729 really needs
it. What happens without that cliping or at least with it at the end, just
before storing in ac_v?


> +        }
> +        ac_v[n] = (v + 0x4000) >> 15;
> +    }
> +}
> +

> +/**
> + * \brief Circularly convolve fixed fector with a phase dispersion impulse response filter

what is a fector?


> + * \param fc_in source vector
> + * \param filter impulse response of phase filter to apply
> + * \param fc_out vector with filter applied
> + *
> + * \note fc_in and fc_out should not overlap!
> + */

Also all doxy belongs in the header not the .c file.


[...]
> +/**
> + * \brief LP synthesis filter
> + * \param filter_coeffs (Q12) filter coefficients
> + * \param in (Q0) input signal
> + * \param out [out] (Q0) output (filtered) signal
> + * \param filter_data [in/out] (Q0) filter data array (previous synthesis data)
> + * \param subframe_size length of subframe
> + * \param update_filter_data 1 - update past filter data arrays
> + *                           0 - don't update
> + *
> + * \return 1 if overflow occured, 0 - otherwise
> + *
> + * \note filter_data should be at least subframe_size+10 size
> + * Routine applies 1/A(z) filter to given speech data
> + */
> +int ff_acelp_lp_synthesis_filter(
> +        const int16_t* filter_coeffs,
> +	const int16_t *in,
> +	int16_t *out,
> +	int16_t *filter_data,
> +	int subframe_size,
> +	int update_filter_data)

tabs


> +{
> +    int i,n;
> +    int sum;
> +
> +    for(n=0; n<subframe_size; n++)
> +    {
> +        int overflow=0;
> +
> +        sum = in[n] << 12;
> +        for(i=0; i<10; i++)
> +            sum -= filter_coeffs[i+1] * filter_data[10+n-i-1];
> +
> +        sum = (sum + 0x800) >> 12;
> +
> +        if(sum > SHRT_MAX)

SHRT_MAX is definitly not correct because theres no short anywhere


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

Frequently ignored awnser#1 FFmpeg bugs should be sent to our bugtracker. User
questions about the command line tools should be sent to the ffmpeg-user ML.
And questions about how to use libav* should be sent to the libav-user ML.
-------------- 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/20080422/ec40993a/attachment.pgp>