[FFmpeg-devel] avcodec/truespeech: Eliminate some left shifts

Submitted by Michael Niedermayer on Sept. 20, 2019, 6:24 p.m.

Details

Message ID 20190920182431.2906-1-michael@niedermayer.cc
State Accepted
Commit c7c0229bebbb2ea6eec6eddb75774eca3e33d075
Headers show

Commit Message

Michael Niedermayer Sept. 20, 2019, 6:24 p.m.
This avoids some invalid shifts

Signed-off-by: Michael Niedermayer <michael@niedermayer.cc>
---
 libavcodec/truespeech.c | 7 +++----
 1 file changed, 3 insertions(+), 4 deletions(-)

Comments

Andreas Rheinhardt Sept. 22, 2019, 9:02 a.m.
Michael Niedermayer:
> This avoids some invalid shifts
> 
> Signed-off-by: Michael Niedermayer <michael@niedermayer.cc>
> ---
>  libavcodec/truespeech.c | 7 +++----
>  1 file changed, 3 insertions(+), 4 deletions(-)
> 
> diff --git a/libavcodec/truespeech.c b/libavcodec/truespeech.c
> index d4ddfcbf9c..54352851b3 100644
> --- a/libavcodec/truespeech.c
> +++ b/libavcodec/truespeech.c
> @@ -132,8 +132,7 @@ static void truespeech_correlate_filter(TSContext *dec)
>          if(i > 0){
>              memcpy(tmp, dec->cvector, i * sizeof(*tmp));
>              for(j = 0; j < i; j++)
> -                dec->cvector[j] = ((tmp[i - j - 1] * dec->vector[i]) +
> -                                   (dec->cvector[j] << 15) + 0x4000) >> 15;
> +                dec->cvector[j] += (tmp[i - j - 1] * dec->vector[i] + 0x4000) >> 15;
>          }
>          dec->cvector[i] = (8 - dec->vector[i]) >> 3;
>      }
> @@ -256,7 +255,7 @@ static void truespeech_synth(TSContext *dec, int16_t *out, int quart)
>          int sum = 0;
>          for(k = 0; k < 8; k++)
>              sum += ptr0[k] * ptr1[k];
> -        sum = (sum + (out[i] << 12) + 0x800) >> 12;
> +        sum = out[i] + ((sum + 0x800) >> 12);
>          out[i] = av_clip(sum, -0x7FFE, 0x7FFE);
>          for(k = 7; k > 0; k--)
>              ptr0[k] = ptr0[k - 1];
> @@ -274,7 +273,7 @@ static void truespeech_synth(TSContext *dec, int16_t *out, int quart)
>          for(k = 7; k > 0; k--)
>              ptr0[k] = ptr0[k - 1];
>          ptr0[0] = out[i];
> -        out[i] = ((out[i] << 12) - sum) >> 12;
> +        out[i] += (- sum) >> 12;
>      }
>  
>      for(i = 0; i < 8; i++)
> 
LGTM.

- Andreas

PS: My first patch for this (never sent to the ML) was exactly as
yours until I dropped this approach because (x << shift) >> shift is
not the identity, but may change the high bits. Then I switched to
using multiplications and checked that they don't overflow, because
one factor is always an int16_t. At this point I should have realized
that my first approach was viable, but somehow didn't.
Michael Niedermayer Sept. 24, 2019, 10:26 a.m.
On Sun, Sep 22, 2019 at 09:02:00AM +0000, Andreas Rheinhardt wrote:
> Michael Niedermayer:
> > This avoids some invalid shifts
> > 
> > Signed-off-by: Michael Niedermayer <michael@niedermayer.cc>
> > ---
> >  libavcodec/truespeech.c | 7 +++----
> >  1 file changed, 3 insertions(+), 4 deletions(-)
> > 
> > diff --git a/libavcodec/truespeech.c b/libavcodec/truespeech.c
> > index d4ddfcbf9c..54352851b3 100644
> > --- a/libavcodec/truespeech.c
> > +++ b/libavcodec/truespeech.c
> > @@ -132,8 +132,7 @@ static void truespeech_correlate_filter(TSContext *dec)
> >          if(i > 0){
> >              memcpy(tmp, dec->cvector, i * sizeof(*tmp));
> >              for(j = 0; j < i; j++)
> > -                dec->cvector[j] = ((tmp[i - j - 1] * dec->vector[i]) +
> > -                                   (dec->cvector[j] << 15) + 0x4000) >> 15;
> > +                dec->cvector[j] += (tmp[i - j - 1] * dec->vector[i] + 0x4000) >> 15;
> >          }
> >          dec->cvector[i] = (8 - dec->vector[i]) >> 3;
> >      }
> > @@ -256,7 +255,7 @@ static void truespeech_synth(TSContext *dec, int16_t *out, int quart)
> >          int sum = 0;
> >          for(k = 0; k < 8; k++)
> >              sum += ptr0[k] * ptr1[k];
> > -        sum = (sum + (out[i] << 12) + 0x800) >> 12;
> > +        sum = out[i] + ((sum + 0x800) >> 12);
> >          out[i] = av_clip(sum, -0x7FFE, 0x7FFE);
> >          for(k = 7; k > 0; k--)
> >              ptr0[k] = ptr0[k - 1];
> > @@ -274,7 +273,7 @@ static void truespeech_synth(TSContext *dec, int16_t *out, int quart)
> >          for(k = 7; k > 0; k--)
> >              ptr0[k] = ptr0[k - 1];
> >          ptr0[0] = out[i];
> > -        out[i] = ((out[i] << 12) - sum) >> 12;
> > +        out[i] += (- sum) >> 12;
> >      }
> >  
> >      for(i = 0; i < 8; i++)
> > 
> LGTM.

will apply

thx

[...]

Patch hide | download patch | download mbox

diff --git a/libavcodec/truespeech.c b/libavcodec/truespeech.c
index d4ddfcbf9c..54352851b3 100644
--- a/libavcodec/truespeech.c
+++ b/libavcodec/truespeech.c
@@ -132,8 +132,7 @@  static void truespeech_correlate_filter(TSContext *dec)
         if(i > 0){
             memcpy(tmp, dec->cvector, i * sizeof(*tmp));
             for(j = 0; j < i; j++)
-                dec->cvector[j] = ((tmp[i - j - 1] * dec->vector[i]) +
-                                   (dec->cvector[j] << 15) + 0x4000) >> 15;
+                dec->cvector[j] += (tmp[i - j - 1] * dec->vector[i] + 0x4000) >> 15;
         }
         dec->cvector[i] = (8 - dec->vector[i]) >> 3;
     }
@@ -256,7 +255,7 @@  static void truespeech_synth(TSContext *dec, int16_t *out, int quart)
         int sum = 0;
         for(k = 0; k < 8; k++)
             sum += ptr0[k] * ptr1[k];
-        sum = (sum + (out[i] << 12) + 0x800) >> 12;
+        sum = out[i] + ((sum + 0x800) >> 12);
         out[i] = av_clip(sum, -0x7FFE, 0x7FFE);
         for(k = 7; k > 0; k--)
             ptr0[k] = ptr0[k - 1];
@@ -274,7 +273,7 @@  static void truespeech_synth(TSContext *dec, int16_t *out, int quart)
         for(k = 7; k > 0; k--)
             ptr0[k] = ptr0[k - 1];
         ptr0[0] = out[i];
-        out[i] = ((out[i] << 12) - sum) >> 12;
+        out[i] += (- sum) >> 12;
     }
 
     for(i = 0; i < 8; i++)