[FFmpeg-cvslog] celp_math: cleanup ff_dot_product()

[Vitor Sessak]

On Sat, Oct 1, 2011 at 6:10 PM, Michael Niedermayer <michaelni at gmx.at> wrote:
> On Sat, Oct 01, 2011 at 10:36:04AM +0200, Vitor Sessak wrote:
>> On Thu, Sep 29, 2011 at 10:09 PM, Michael Niedermayer <git at videolan.org> wrote:
>> > ffmpeg | branch: master | Michael Niedermayer <michaelni at gmx.at> | Thu Sep 29 21:21:26 2011 +0200| [11512367d3c6f67b093b54c532328763c9b815e2] | committer: Michael Niedermayer
>> >
>> > celp_math: cleanup ff_dot_product()
>> > based on code & idea by vitor
>> >
>> > Signed-off-by: Michael Niedermayer <michaelni at gmx.at>
>> >
>> >> http://git.videolan.org/gitweb.cgi/ffmpeg.git/?a=commit;h=11512367d3c6f67b093b54c532328763c9b815e2
>> > ---
>> >
>> >  libavcodec/celp_math.c |   12 ++++++------
>> >  libavcodec/celp_math.h |    3 +--
>> >  libavcodec/g723_1.c    |   18 +++++++++---------
>> >  3 files changed, 16 insertions(+), 17 deletions(-)
>> >
>> > diff --git a/libavcodec/celp_math.c b/libavcodec/celp_math.c
>> > index b78edd1..a00bb39 100644
>> > --- a/libavcodec/celp_math.c
>> > +++ b/libavcodec/celp_math.c
>> > @@ -197,14 +197,14 @@ int ff_log2(uint32_t value)
>> >     return (power_int << 15) + value;
>> >  }
>> >
>> > -int ff_dot_product(const int16_t *a, const int16_t *b, int length, int shift)
>> > +int ff_dot_product(const int16_t *a, const int16_t *b, int length)
>> >  {
>> > -    int i, sum = 0;
>> > +    int i;
>> > +    int64_t sum = 0;
>> > +
>> > +    for (i = 0; i < length; i++)
>> > +        sum += MUL16(a[i], b[i]);
>> >
>> > -    for (i = 0; i < length; i++) {
>> > -        int64_t prod = av_clipl_int32(MUL64(a[i], b[i]) << shift);
>> > -        sum = av_clipl_int32(sum + prod);
>> > -    }
>> >     return sum;
>> >  }
>>
>> Hmmm, are you sure you didn't meant to return a int64_t?
>>
>> Also, I'm afraid that after this change (and a few other cleanup) the
>> decoder is not bit-identical anymore to the reference decoder. Have
>> you tested?
>
> Ive tested with all the files i could find at the time of this commit,
> which admitably arent many and most are rather short.
>
> My idea was a bit toward using bugreports & git bisect to find out
> if this is safe.

Fine for me, but I still see no point of using a 64-bit accumulator
and truncating it to 32-bit before returning...

-Vitor