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

[Michael Niedermayer]

On Thu, Apr 24, 2008 at 02:15:51AM +0700, Vladimir Voroshilov wrote:
> Michael Niedermayer wrote: 
[...]
> > > +        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?
> 
> Removing those line breaks OVERFLOW test (regardless of clipping outside loop),
> significantly reduces PSNR from bitexact's 99,99 to 18,54 without outside
> clipping and to 26,01 with it.

What is the OVERFLOW test anyway? Is this a normal valid bitstream generated
from a pcm wave? Or some sythetic overflow excercise which cannot be generated
by any input wave?
(If you dont know you can test it by encoding the wave from overflow to see if
there are still any overflows happening from the resulting bitstream)


[...]
> +void ff_acelp_convolve_circ(const int16_t* fc_in, const int16_t* filter, int16_t* fc_out, int subframe_size)
> +{
> +    int i, k;
> +
> +    memset(fc_out, 0, subframe_size * sizeof(int16_t));
> +
> +    for(i=0; i<subframe_size; i++)
> +    {
> +        if(fc_in[i])
> +        {
> +            for(k=0; k<i; k++)
> +                fc_out[k] += (fc_in[i] * filter[subframe_size + k - i]) >> 15;
> +
> +            for(k=i; k<subframe_size; k++)
> +                fc_out[k] += (fc_in[i] * filter[k - i]) >> 15;
> +        }
> +    }
> +}

where is this used? i cant find it in g729 and neither this file


> +
> +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)
> +{
> +    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 > 0x7fff)
> +        {
> +            sum = 0x7fff;
> +            overflow = 1;
> +        }
> +        else if (sum < -0x8000)
> +        {
> +            sum =-0x8000;
> +            overflow = 1;
> +        }
> +
> +        if(overflow && !update_filter_data)
> +            return 1;

if(sum + 0x8000 > 0xFFFFU){
    if(!update_filter_data)
        return 1;
    sum= (sum>>31) ^ 32767;
}


> +        filter_data[10+n] = out[n] = sum;

This duplicated storeage is unacceptable.


> +    }
> +
> +    // Saving data for using in next subframe
> +    if(update_filter_data)
> +        memcpy(filter_data, filter_data + subframe_size, 10 * sizeof(int16_t));

This design requires the memcpy to be duplicated.

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

Complexity theory is the science of finding the exact solution to an
approximation. Benchmarking OTOH is finding an approximation of the exact
-------------- 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/20080423/d3b374f7/attachment.pgp>