[FFmpeg-devel] [PATCH] Common ACELP code & G.729 [1/7] - filters

[Vladimir Voroshilov]

On Wed, May 7, 2008 at 3:42 AM, Michael Niedermayer <michaelni at gmx.at> wrote:
>
> On Tue, May 06, 2008 at 11:24:38PM +0700, Vladimir Voroshilov wrote:
>  > On Mon, May 5, 2008 at 3:42 AM, Michael Niedermayer <michaelni at gmx.at> wrote:
>  > > On Sun, May 04, 2008 at 01:19:10PM +0700, Vladimir Voroshilov wrote:
>  > >  > On Sat, May 3, 2008 at 7:23 PM, Michael Niedermayer <michaelni at gmx.at> wrote:
>  > >  > > On Sat, May 03, 2008 at 03:24:54PM +0700, Vladimir Voroshilov wrote:
>  > >  > >  > Michael Niedermayer wrote:
>  > >  > >  [...]
>  > >  > >  > +void ff_acelp_convolve_circ(
>  > >  > >  > +        int16_t* fc_out,
>  > >  > >  > +        const int16_t* fc_in,
>  > >  > >  > +        const int16_t* filter,
>  > >  > >  > +        int subframe_size)
>  > >  > >  > +{
>  > >  > >  > +    int i, k;
>  > >  > >  > +
>  > >  > >  > +    memset(fc_out, 0, subframe_size * sizeof(int16_t));
>  > >  > >
>  > >  > > > +
>  > >  > >  > +    /* Since there are few pulses over all subframe (i.e. almost all
>  > >  > >
>  > >  > > > +       fc_in[i] are zero, in case of G.729D it is only two non-zero
>  > >  > >  > +       samples of total 40), it is faster to swap two loops and process
>  > >  > >  > +       non-zero samples only. This will reduce number of multiplications
>  > >  > >
>  > >  > > > +       from 40*40 to 2*40 for G.729D */
>  > >  > >
>  > >  > >  doesnt ff_acelp_fc_enchance_harmonics() increase the number of non 0
>  > >  > >  elements above 2 ?
>  > >  >
>  > >  > Perhaps i misspelled sentence.
>  > >  > I meant that using swapped loops with checking for non-zero will
>  > >  > require 2*40 multiplications,
>  > >
>  > >  The sentance is fine.
>  > >  What i meant is that ff_acelp_fc_enchance_harmonics() can increase the number
>  > >  of non zero samples above 2. Or do i miss somehing that prevents this?
>  >
>  > This is exactly what this filter intended to do.
>  > It get buffer with few pulses and smooth them over all subframe.
>  > In reference code it is caller "pitch_shrp" ("pitch sharpening" i guess).
>
>  What iam trying to say is, if ff_acelp_fc_enchance_harmonics() increases the
>  number of non zero samples above 2 then the comment is no longer correct.
>  As its no longer exactly 2 for g729d

I'm afraid you still don't understand what I wanted to say.
1. At each call "in" buffer for g729d contains exactly two no-zero values.
2. During process these two pulses are smoothed over all subframe and
written to "output" (!) buffer. "in" buffer remains the same.
3. When used formula from doxygen comments, filter will make 40*40
multiplications (regardless of values in "in" buffer).
4. When two loops are swapped (comparing to formula in doxygen
comment) and check for non-zero is added, filter will make only 2*40
multiplications (since input buffer is never touched).

This is why i asked for phrase correctness (i meant not english, but
phrase meaning).

>  > +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);                     /* (14.13) = (13.13) * (1.13) */
>  > +        hpf_f[0] += MULL(hpf_f[2], -7667);                     /* (13.13) = (13.13) * (0.13) */
>  > +        hpf_f[0] += 7699 * (hpf_z[0] - 2*hpf_z[1] + hpf_z[2]); /* (14.13) =  (0.13) * (14.0) */
>
> > +
>  > +        /* Multiplication by 2 with rounding can cause short type
>  > +           overflow, thus clipping is required. */
>  > +
>  > +        out[i] = av_clip_int16((hpf_f[0] + 0x800) >> 12);      /* (15.0) = 2 * (13.13) = (14.13) */
>  > +
>  > +        memmove(hpf_f + 1, hpf_f, 2 * sizeof(hpf_f[0]));
>  > +    }
>
>
>  t =  MULL(hpf_f[0], 15836);                     /* (14.13) = (13.13) * (1.13) */
>    + MULL(hpf_f[1], -7667);                     /* (13.13) = (13.13) * (0.13) */
>    + 7699 * (in[i] - 2*in[i-1] + in[i-2]);      /* (14.13) =  (0.13) * (14.0) */
>
>  out[i] = av_clip_int16((t + 0x800) >> 12);      /* (15.0) = 2 * (13.13) = (14.13) */
>
>  hpf_f[1]= hpf_f[0];
>  hpf_f[0]= t;

This will require either two samples before start in "in" buffer, copied from
the previous subframe or
duplicating code by moving two first iterations outside the loop.
E.g, something like:

t =  MULL(hpf_f[0], 15836);                     /* (14.13) = (13.13) * (1.13) */
   + MULL(hpf_f[1], -7667);                     /* (13.13) = (13.13) * (0.13) */
   + 7699 * (in[0] - 2*hpf_z[0] + hpf_z[1]);      /* (14.13) =  (0.13)
* (14.0) */

 out[0] = av_clip_int16((t + 0x800) >> 12);      /* (15.0) = 2 *
(13.13) = (14.13) */

  hpf_f[1]= hpf_f[0];
  hpf_f[0]= t;

t =  MULL(hpf_f[0], 15836);                     /* (14.13) = (13.13) * (1.13) */
   + MULL(hpf_f[1], -7667);                     /* (13.13) = (13.13) * (0.13) */
   + 7699 * (in[1] - 2*in[0] + hpf_z[0]);      /* (14.13) =  (0.13) * (14.0) */

 out[1] = av_clip_int16((t + 0x800) >> 12);      /* (15.0) = 2 *
(13.13) = (14.13) */

  hpf_f[1]= hpf_f[0];
  hpf_f[0]= t;

for(i=2; i<subframe_size; i++)
{
  t =  MULL(hpf_f[0], 15836);                     /* (14.13) = (13.13)
* (1.13) */
    + MULL(hpf_f[1], -7667);                     /* (13.13) = (13.13)
* (0.13) */
    + 7699 * (in[i] - 2*in[i-1] + in[i-2]);      /* (14.13) =  (0.13)
* (14.0) */

  out[i] = av_clip_int16((t + 0x800) >> 12);      /* (15.0) = 2 *
(13.13) = (14.13) */

  hpf_f[1]= hpf_f[0];
  hpf_f[0]= t;
}

hpf_z[0] = in[subframe_size-1];
hpf_z[1] = in[subframe_size-2];



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