[FFmpeg-devel] [PATCH] QCELP decoder

[Diego Biurrun]

On Thu, Oct 16, 2008 at 03:38:03PM -0700, Kenan Gillet wrote:
> On Oct 15, 2008, at 3:09 PM, Michael Niedermayer wrote:
> 
> > cOn Tue, Oct 14, 2008 at 06:59:34PM -0700, Kenan Gillet wrote:
> >>
> >> --- libavcodec/qcelp.h	(revision 0)
> >> +++ libavcodec/qcelp.h	(revision 0)
> >> +static float compute_subframe_energy(const float *vector, const  
> >> int subframeno) {
> >> +    int   i;
> >> +    float energy = 0;
> >> +
> >> +    vector += 40 * subframeno;
> >> +
> >> +    for (i = 0; i < 40; i++)
> >> +        energy += vector[i] * vector[i];
> >> +
> >> +    return energy;
> >> +}
> >
> > IMHO its cleaner not to add subframeno inside the function.
> > a simple dot_product(vector, len) should be more readable
> > also such a function has a much better chance to be shareable
> > between codec, and especially when things like that are optimized its
> > better if it isnt needed to be redone for each len and other obscure  
> > variant
> 
> done, I svn cp libavcodec/acelp_math.c into libavcodec/qcelp_math.c
> and made it ff_dot_product.
> or is there a better place for that ?

If the file is used for both acelp and qcelp, it should not be called
qcelp, but celp instead.

> Should it be in a different patch ?

Smaller patches get reviewed and applied faster.

Diego