[FFmpeg-cvslog] celp_math: cleanup ff_dot_product()

[Michael Niedermayer]

On Sat, Oct 01, 2011 at 08:13:48PM +0200, Vitor Sessak wrote:
> 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...

changed locally to int64_t

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

It is what and why we do it that matters, not just one of them.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 198 bytes
Desc: Digital signature
URL: <http://ffmpeg.org/pipermail/ffmpeg-cvslog/attachments/20111001/6da087d9/attachment.asc>