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

Michael Niedermayer michaelni
Sun Apr 27 20:35:06 CEST 2008


On Mon, Apr 28, 2008 at 01:02:32AM +0700, Vladimir Voroshilov wrote:
> On Sun, Apr 27, 2008 at 10:57 PM, Michael Niedermayer <michaelni at gmx.at> wrote:
> >
> > On Sun, Apr 27, 2008 at 08:39:15PM +0700, Vladimir Voroshilov wrote:
> 
> [...]
> 
> >  > Here is latest version.
> >
> > [...]
> >  > +void ff_acelp_interpolate_excitation(
> >  > +        int16_t* ac_v,
> >  > +        int pitch_delay_int,
> >  > +        int pitch_delay_frac,
> >
> > > +        int subframe_size)
> >  > +{
> >  > +    int n, i;
> >  > +    int v;
> >  > +
> >  > +    /* The lookup table contain values corresponding
> >  > +       to -2/6 -1/6 0 1/6 2/6 3/6 fraction order.
> >
> > > +       The filtering process uses a negative pitch lag offset, but
> >  > +       a negative offset should not be used in then table. To avoid
> >  > +       a negative offset in the table dimension corresponding to
> >  > +       fractional delay the following conversion applies:
> >  > +
> >
> >  trailing whitespace
> 
> Fixed.
> 

> >  > +       -pitch_delay = -(6*pitch_delay_int+pitch_delay_frac) =
> >  > +
> >  > +       =  -6*pitch_delay_int - pitch_delay_frac =
> >  > +
> >  > +         / -6*(pitch_delay_int)   - pitch_delay_frac,     pitch_delay_frac <  0
> >  > +       =<
> >  > +         \ -6*(pitch_delay_int+1) + (6-pitch_delay_frac), pitch_delay_frac >= 0
> >  > +    */
> >  > +
> >  > +    /* Compute negative value of fractional delay */
> >  > +    pitch_delay_frac = - pitch_delay_frac;
> >  > +
> >
> >  > +    /* Now make sure that pitch_delay_frac will always be positive */
> >
> > > +    if(pitch_delay_frac < 0)
> >  > +    {
> >  > +        pitch_delay_frac += 6;
> >  > +        pitch_delay_int++;
> >  > +    }
> >
> >  Can it really be >= 0 and <0 ?
> 
> Didn't understand you.
> According to current pitch delay decode routine pitch_delay_frac
> belongs to [-2; 3]
> -pitch_delay_frac belongs to [-3; 2]

In the latest code i have it is:

+                pitch_delay_3x = 3 * ctx->pitch_delay_int_prev + 1;
+                if(!i) 
+                    pitch_delay_1st_int = pitch_delay_3x / 3;
+        }
+        else if (!i)
+        {
+            // Decoding of the adaptive-codebook vector delay for first subframe (4.1.3)
+            pitch_delay_3x = parm->ac_index[i] + 59;
+            if(pitch_delay_3x > 255)
+                pitch_delay_3x = 3 * pitch_delay_3x - 512;
+            pitch_delay_1st_int = pitch_delay_3x / 3;
+        }
+        else
+        {
+            // Decoding of the adaptive-codebook vector delay for second subframe (4.1.3)
+            int pitch_delay_min_3k = 3 * av_clip(
+                    ctx->pitch_delay_int_prev-5,
+                    PITCH_LAG_MIN,
+                    PITCH_LAG_MAX-9);
+
+                pitch_delay_3x = parm->ac_index[i] + pitch_delay_min_3k - 1;
+        }
...
+        ff_acelp_interpolate_excitation(pitch_delay_3x, ctx->exc + i*ctx->subframe_size, ctx->subframe_size);

In this code pitch_delay_3x should be >0 thus the if will always be true and
can be simplified out! The same applies to the *(-1) before it.


> 
> >  > +void ff_acelp_weighted_filter(
> >  > +        int16_t *out,
> >  > +        const int16_t* in,
> >  > +        int16_t weight,
> >  > +        int filter_length)
> >  > +{
> >  > +    int weight_pow = 1 << 15;
> >  > +    int n;
> >  > +
> >  > +    for(n=0; n<filter_length; n++)
> >  > +    {
> >  > +        /* (0.15) * (3.12) and (3.27) -> (3.12) with rounding */
> >  > +        out[n] = (in[n] * weight_pow + 0x4000) >> 15;
> >  > +        /* (0.15) * (3.12) and (3.27) -> (3.12) with rounding */
> >  > +        weight_pow = (weight_pow * weight + 0x4000) >> 15;
> >  > +    }
> >  > +}
> >
> >  as weight is a constant weight_pow is as well and half of the computations
> >  are unneeded
> 
> Now routine accepts array of constants.
> 
> >  > +void ff_acelp_high_pass_filter(
> >  > +        int16_t* out,
> >
> > > +        int16_t* hpf_z,
> >  > +        int* hpf_f,
> >  > +        const int16_t* in,
> >
> > > +        int length)
> >  > +{
> >  > +    int i;
> >  > +
> >  > +    for(i=0; i<length; i++)
> >  > +    {
> >  > +        memmove(hpf_z + 1, hpf_z, 2 * sizeof(hpf_z[0]));
> >  > +        hpf_z[0] = in[i];
> >
> > > +
> >  > +        hpf_f[0] =  MULL(hpf_f[1], 15836);
> >  > +        hpf_f[0] += MULL(hpf_f[2], -7667);
> >  > +
> >  > +        hpf_f[0] += 7699 * (hpf_z[0] - 2*hpf_z[1] + hpf_z[2]);
> >  > +
> >  > +        /* Clippin is required to pass G.729 OVERFLOW test */
> >
> > > +        if(hpf_f[0] >= 0xfffffff)
> >  > +        {
> >  > +            out[i] = SHRT_MAX;
> >
> > > +            hpf_f[0] = 0x3fffffff;
> >  > +        }
> >  > +        else if (hpf_f[0] <= -0x10000000)
> >  > +        {
> >  > +            out[i] = SHRT_MIN;
> >
> > > +            hpf_f[0] = -0x40000000;
> >  > +        }
> >  > +        else
> >  > +        {
> >
> >  > +            hpf_f[0] <<= 2;
> >
> >  The shift is avoidable i think as already said
> 
> Changed and added comment about bitexactness.

I do think the bitexactness can be maintained.


> 
> There is also some not clear thing for me..
> ff_acelp_interpolate_excitation (according to math) is just  an low-pass filter.
> But it is intended to build adaptive-codebook (pitch) vector from
> previous speech data.
> I wonder should it be moved to something like acelp_vectors.c (along
> with fixed codebook (innovation) vector decoding, fixed vector
> updating and building excitation signal from pitch and innovation
> vectors) ?

Whichever way you prefer ...

[...]
> +void ff_acelp_high_pass_filter(
> +        int16_t* out,
> +        int16_t* hpf_z,
> +        int* hpf_f,
> +        const int16_t* in,
> +        int length)
> +{
> +    int i;
> +
> +    for(i=0; i<length; i++)
> +    {
> +        memmove(hpf_z + 1, hpf_z, 2 * sizeof(hpf_z[0]));
> +        hpf_z[0] = in[i];
> +
> +        /* Note: to make result bitexact with G.729 reference code,
> +           two least significant bits of each value returned by MULL
> +           should be cleared */
> +        hpf_f[0] =  MULL(hpf_f[1], 15836); // (15.14) = (14.14) * (1.13)
> +        hpf_f[0] += MULL(hpf_f[2], -7667); // (14.14) = (14.14) * (0.13)
> +
> +        hpf_f[0] += 30796 * (hpf_z[0] - 2*hpf_z[1] + hpf_z[2]); // (16.14) = (3.13) * (14.0)
> +

> +        /* Clipping is required to pass G.729 OVERFLOW test */
> +        if(hpf_f[0] >= 0x3fffffff)
> +        {
> +            out[i] = SHRT_MAX;
> +            hpf_f[0] = 0x3fffffff;
> +        }
> +        else if (hpf_f[0] <= -0x40000000)
> +        {
> +            out[i] = SHRT_MIN;
> +            hpf_f[0] = -0x40000000;
> +        }
> +        else
> +        {
> +            /* Multiplication by 2 with rounding can cause short type overflow, thus
> +               additional clipping is required. */
> +            out[i] = av_clip_int16((hpf_f[0] + 0x2000) >> 14); /* (15.0) = 2 * (14.14) */
> +        }

The cliping looks duplicated


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

Old school: Use the lowest level language in which you can solve the problem
            conveniently.
New school: Use the highest level language in which the latest supercomputer
            can solve the problem without the user falling asleep waiting.
-------------- 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/20080427/6de8d6d1/attachment.pgp>



More information about the ffmpeg-devel mailing list