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

[Vladimir Voroshilov]

On Thu, Apr 24, 2008 at 9:57 PM, Michael Niedermayer <michaelni at gmx.at> wrote:
>
> On Thu, Apr 24, 2008 at 09:32:53PM +0700, Vladimir Voroshilov wrote:
>  > On Thu, Apr 24, 2008 at 3:33 AM, Michael Niedermayer <michaelni at gmx.at> wrote:
>  > > On Thu, Apr 24, 2008 at 02:41:54AM +0700, Vladimir Voroshilov wrote:
>  > >  [...]
>  > >  > > > +/**
>  > >  > > > + * \brief Calculates sum of array elements absolute values
>  > >  > > > + * \param speech array with input data
>  > >  > > > + * \param cycles number elements to proceed
>  > >  > > > + * \param shift right shift by this value will be done before addition
>  > >  > > > + *
>  > >  > > > + * \return sum of absolute values
>  > >  > > > + */
>  > >  > > > +static int sum_of_absolute(const int16_t* speech, int cycles, int shift)
>  > >  > > > +{
>  > >  > > > +    int n;
>  > >  > > > +    int sum = 0;
>  > >  > > > +
>  > >  > > > +    for(n=0; n<cycles; n++)
>  > >  > > > +       sum += FFABS(speech[n] >> shift);
>  > >  > > > +
>  > >  > > > +    return sum;
>  > >  > > > +}
>  > >  > >
>  > >  > > The place where the shift is done is not optimal for precission.
>  > >  >
>  > >  > I've just realized that i'm not using shift at all.
>  > >  > Should i keep routine as is, remove shift or remove all routine?
>  > >  > Currently moved outside loop.
>  > >
>  > >  if shift is always 0 remove shift.
>  > >  and maybe yes, remove the whole routine its so simple that iam not sure
>  > >  if this makes sense here, maybe if its speed critical it could be put in
>  > >  dsputil but i guess it could as well stay in g729.c for now
>  >
>  > I've inlined that code.
>
>
> [...]
>
>  > +int ff_log2(int value)
>
>  value should be unsigned.

Fixed.

>  > +{
>  > +    uint32_t result;
>  > +    uint8_t  power_int;
>  > +    uint8_t  frac_x0;
>  > +    uint16_t frac_dx;
>  > +
>  > +    assert(value > 0);
>  > +
>  > +    // Stripping zeros from beginning
>  > +    power_int = av_log2(value);
>  > +    result = value << (31 - power_int);
>
>  value could be used here, which would mean 1 variable less

Fixed.

>  > +/**
>  > + * \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_24_15(int var_q24, int16_t var_q15)
>  > +{
>  > +    int64_t tmp = (((int64_t)var_q24 * (int64_t)var_q15) >> 15);
>  > +
>  > +    if(tmp < -0x7fffffff)
>  > +        return -0x7fffffff;
>  > +    else if (tmp > 0x7fffffff)
>  > +        return 0x7fffffff;
>  > +    else
>  > +        return tmp;
>  > +}
>  > +
>
>  does amr/qcelp also require such cliped multiply?
>  Also the word clip should occur in the name, maybe cliped_mul_24_15()

LOL. it is unnecessary in G.729 too :)
Renamed to mul_32_16 to reflect parameters size.

>  > +/**
>  > + * \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)
>
> > +{
>  > +    int n;
>  > +    int sum = 0;
>  > +
>  > +    for(n=0; n<cycles; n++)
>
> > +       sum += (speech[n] >> shift) * (speech[n + offset] >> shift);
>
>   >> shift should be done after the *
>  and the n can be moved out of the inner loop:
>
>  for(speech_end= speech + cycles; speech < speech_end; speech++)
>     sum += (speech[0] * speech[offset]) >> shift;
>
>
>
>  > +
>  > +    return av_clip(sum, -0x40000000, 0x3fffffff);
>  > +}
>
>  if such clip is needed it should be done outside the common code

Unnecessary. Removed.



-- 
Regards,
Vladimir Voroshilov mailto:voroshil at gmail.com
JID: voroshil at gmail.com, voroshil at jabber.ru
ICQ: 95587719
-------------- next part --------------
An embedded and charset-unspecified text was scrubbed...
Name: acelp_math_24.diff
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20080424/da98d0d8/attachment.asc>