[FFmpeg-devel] [PATCH 1/2] avcodec/adxenc: Avoid undefined left shift of negative numbers

[Andreas Rheinhardt]

Replace "((a << shift) + b) >> shift" by "a + (b >> shift)". This avoids
a left shift which also happens to trigger undefined behaviour in case "a"
is negative. This affected the FATE-tests acodec-adpcm-adx and
acodec-adpcm-adx-trellis; it also fixes ticket #8008.

Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt at gmail.com>
---
The equivalence of the old code and the new code in the first two
changes is clear from the type of wav[i] (int16_t) and the fact that
COEFF_BITS is 12 (i.e. no high bits are lost when left shifting); for
the last change the equivalence depends upon scale being representable
in an uint16_t. I don't know whether this is indeed so (the direct
estimate gives an upper bound of (2^19 + 2^15 + 1) / 7), but it better
should be so because scale is written via AV_WB16. Maybe it should be
clipped if it is outside of this range?

 libavcodec/adxenc.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/libavcodec/adxenc.c b/libavcodec/adxenc.c
index a781151b44..93b902b0e1 100644
--- a/libavcodec/adxenc.c
+++ b/libavcodec/adxenc.c
@@ -48,7 +48,7 @@ static void adx_encode(ADXContext *c, uint8_t *adx, const int16_t *wav,
     s2 = prev->s2;
     for (i = 0, j = 0; j < 32; i += channels, j++) {
         s0 = wav[i];
-        d = ((s0 << COEFF_BITS) - c->coeff[0] * s1 - c->coeff[1] * s2) >> COEFF_BITS;
+        d = s0 + ((-c->coeff[0] * s1 - c->coeff[1] * s2) >> COEFF_BITS);
         if (max < d)
             max = d;
         if (min > d)
@@ -79,13 +79,13 @@ static void adx_encode(ADXContext *c, uint8_t *adx, const int16_t *wav,
     s1 = prev->s1;
     s2 = prev->s2;
     for (i = 0, j = 0; j < 32; i += channels, j++) {
-        d = ((wav[i] << COEFF_BITS) - c->coeff[0] * s1 - c->coeff[1] * s2) >> COEFF_BITS;
+        d = wav[i] + ((-c->coeff[0] * s1 - c->coeff[1] * s2) >> COEFF_BITS);
 
         d = av_clip_intp2(ROUNDED_DIV(d, scale), 3);
 
         put_sbits(&pb, 4, d);
 
-        s0 = ((d << COEFF_BITS) * scale + c->coeff[0] * s1 + c->coeff[1] * s2) >> COEFF_BITS;
+        s0 = d * scale + ((c->coeff[0] * s1 + c->coeff[1] * s2) >> COEFF_BITS);
         s2 = s1;
         s1 = s0;
     }
-- 
2.20.1