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

[Diego Biurrun]

On Sat, Apr 26, 2008 at 11:11:56PM +0700, Vladimir Voroshilov wrote:
> 
> P.S. Please anybody check English spelling too.
> 
> --- /dev/null
> +++ b/libavcodec/acelp_filters.c
> @@ -0,0 +1,265 @@
> +/**
> + *
> + *   G.729 specification says:
> + *     b30 is based on Hamming windowed sinc functions, truncated at +/-29 and

What's sinc?

> +void ff_acelp_interpolate_excitation(int16_t* ac_v, int pitch_delay_6x, int subframe_size)

Please break all those long lines.

> +    // TODO: clarify why used such expression (hint: -1/3 , 0 ,1/3 order in interpol_filter)

clarify why such an expression is used

> +        // Checking for overflow

Do not use ing-forms everywhere.

> +    // Processing first 10 samples using filter_data as past data

ditto

> +    /*
> +      Processing remaining samples using previously computed
> +      samples in the same (output) buffer as past data
> +    */

ditto

Also, why do you spread this comment out over 4 lines instead of 2?

> +    // Saving data for using in next subframe

ditto

> +            /*
> +              Multiplication by 2 can cause short type overflow, thus
> +              additional clipping is required.
> +            */

see above

> --- /dev/null
> +++ b/libavcodec/acelp_filters.h
> @@ -0,0 +1,122 @@
> +
> +#ifndef FFMPEG_ACELP_FILT_H
> +#define FFMPEG_ACELP_FILT_H

Use the filename as multiple inclusion guard.

> + * \param subframe_size length of subframe

subframe length

> + * Routine assumes following fractions order (X - integer delay):

The routine assumes the following order of fractions

> + * Second parameter should contain 6*int+frac+2

The second

> + * Routine can be used for 1/3 precision too, by passing 2*pitch_delay as second parameter

The routine

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

"convolve" is not an English word.  I have no idea what you are trying
to say here.

> + * \param filter impulse response of phase filter to apply

umm, ?

> + * \param stop_on_overflow   1 - return immediately if overflow ocures

occurs

> + * \note output buffer must contains 10 samples of past

contain

> + * \param subframe_size length of subframe

subframe length

> + * \param update_filter_data 1 - update past filter data arrays
> + *                           0 - don't update

do not

> + * \return 1 if overflow occured, 0 - otherwise

occurred

> + * \note filter_data should be at least subframe_size+10 size

s/size//

> + * \param speech [in/out] reconstructed speech signal for applying filter to

?

> + * \param length size of input data

input data size 

Diego