[FFmpeg-devel] [PATCH] Common fixed-point ACELP routines (1/3) - math

[Michael Niedermayer]

On Fri, Apr 25, 2008 at 02:20:46PM +0700, Vladimir Voroshilov wrote:
> 
> 
> On Fri, Apr 25, 2008 at 12:19 PM, Vladimir Voroshilov <voroshil at gmail.com> wrote:
> > Michael Niedermayer wrote:
> >  
> >  
> >  > following is a tiny bit more accurate and doesnt need the 3rd table:
> >  
> >  [...]
> >  
> > 
> >  > Note, yes the output is scaled different again
> >  
> >  Updated.
> >  
> > 
> >  > > +/**
> >  > > + * \brief multiplies 32-bit integer by another 16-bit and divides result by 2^15
> >  > > + * \param var_q24 32-bit integer
> >  > > + * \param var_15 16-bit integer
> >  > > + *
> >  > > + * \return result of (var_q24 * var_q15 >> 15) with clipping to [INT_MIN; INT_MAX] range
> >  > > + */
> >  > > +static inline int mul_32_16(int var_q24, int16_t var_q15)
> >  > > +{
> >  > > +    return (((int64_t)var_q24 * (int64_t)var_q15) >> 15);
> >  > > +}
> >  >
> >  > now this looks like it belongs to libavcodec/mathops.h
> >  
> >  separate patch is attached.
> >  
> > 
> >  > > +/**
> >  > > + * \brief Calculates sum of array elements multiplications
> >  > > + * \param speech array with input data
> >  > > + * \param cycles number elements to proceed
> >  > > + * \param offset offset for calculation sum of s[i]*s[i+offset]
> >  > > + * \param shift right shift by this value will be done before multiplication
> >  > > + *
> >  > > + * \return sum of multiplications
> >  > > + *
> >  > > + * \note array must be at least length+offset long!
> >  > > + */
> >  > > +static int sum_of_squares(const int16_t* speech, int cycles, int offset, int shift)
> >  > > +{
> >  > > +    const int16_t* speech_end;
> >  > > +    int sum = 0;
> >  > > +
> >  > > +    for(speech_end=speech+cycles; speech<speech_end; speech++)
> >  > > +       sum += (speech[0] * speech[offset]) >> (shift << 1);
> >  >
> >  > IMHO the shift << 1 should be done outside this function
> >  
> >  Fixed.
> >  
> > 
> >  
> 
> acelp_math_26.diff in previous mail contains unrelated code.
> Please ignore it.
> This is right one.
> 
> P.S. i did not attach math_opts_27.diff since it is not changed.

[...]

> +/**
> + * \brief Calculates sum of array elements multiplications
> + * \param speech array with input data
> + * \param cycles number elements to proceed
> + * \param offset offset for calculation sum of s[i]*s[i+offset]
> + * \param shift right shift by this value will be done before multiplication
> + *
> + * \return sum of multiplications
> + *
> + * \note array must be at least length+offset long!
                                   ^^^^^^
its called cycles in the function, and yes i agree length is a better name


> + */
> +static int sum_of_squares(const int16_t* speech, int cycles, int offset, int shift)
> +{
> +    const int16_t* speech_end;
> +    int sum = 0;
> +

> +    shift <<= 1;

I would prefer if this is done outside of the function, doing it inside
means only even shift values can be used.

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

Asymptotically faster algorithms should always be preferred if you have
asymptotical amounts of data
-------------- 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/20080425/fbe69305/attachment.pgp>