diff --git a/webrtc/common_audio/signal_processing/include/spl_inl.h b/webrtc/common_audio/signal_processing/include/spl_inl.h index d3cc6dee6c..3c0a81cf34 100644 --- a/webrtc/common_audio/signal_processing/include/spl_inl.h +++ b/webrtc/common_audio/signal_processing/include/spl_inl.h @@ -35,42 +35,32 @@ static __inline int16_t WebRtcSpl_SatW32ToW16(int32_t value32) { return out16; } -static __inline int32_t WebRtcSpl_AddSatW32(int32_t l_var1, int32_t l_var2) { - int32_t l_sum; +static __inline int32_t WebRtcSpl_AddSatW32(int32_t a, int32_t b) { + // Do the addition in unsigned numbers, since signed overflow is undefined + // behavior. + const int32_t sum = (int32_t)((uint32_t)a + (uint32_t)b); - // Perform long addition - l_sum = l_var1 + l_var2; - - if (l_var1 < 0) { // Check for underflow. - if ((l_var2 < 0) && (l_sum >= 0)) { - l_sum = (int32_t)0x80000000; - } - } else { // Check for overflow. - if ((l_var2 > 0) && (l_sum < 0)) { - l_sum = (int32_t)0x7FFFFFFF; - } + // a + b can't overflow if a and b have different signs. If they have the + // same sign, a + b also has the same sign iff it didn't overflow. + if ((a < 0) == (b < 0) && (a < 0) != (sum < 0)) { + // The direction of the overflow is obvious from the sign of a + b. + return sum < 0 ? INT32_MAX : INT32_MIN; } - - return l_sum; + return sum; } -static __inline int32_t WebRtcSpl_SubSatW32(int32_t l_var1, int32_t l_var2) { - int32_t l_diff; +static __inline int32_t WebRtcSpl_SubSatW32(int32_t a, int32_t b) { + // Do the subtraction in unsigned numbers, since signed overflow is undefined + // behavior. + const int32_t diff = (int32_t)((uint32_t)a - (uint32_t)b); - // Perform subtraction. - l_diff = l_var1 - l_var2; - - if (l_var1 < 0) { // Check for underflow. - if ((l_var2 > 0) && (l_diff > 0)) { - l_diff = (int32_t)0x80000000; - } - } else { // Check for overflow. - if ((l_var2 < 0) && (l_diff < 0)) { - l_diff = (int32_t)0x7FFFFFFF; - } + // a - b can't overflow if a and b have the same sign. If they have different + // signs, a - b has the same sign as a iff it didn't overflow. + if ((a < 0) != (b < 0) && (a < 0) != (diff < 0)) { + // The direction of the overflow is obvious from the sign of a - b. + return diff < 0 ? INT32_MAX : INT32_MIN; } - - return l_diff; + return diff; } static __inline int16_t WebRtcSpl_AddSatW16(int16_t a, int16_t b) { diff --git a/webrtc/common_audio/signal_processing/signal_processing_unittest.cc b/webrtc/common_audio/signal_processing/signal_processing_unittest.cc index 108f459c89..0dc0d37878 100644 --- a/webrtc/common_audio/signal_processing/signal_processing_unittest.cc +++ b/webrtc/common_audio/signal_processing/signal_processing_unittest.cc @@ -8,6 +8,9 @@ * be found in the AUTHORS file in the root of the source tree. */ +#include +#include + #include "testing/gtest/include/gtest/gtest.h" #include "webrtc/common_audio/signal_processing/include/signal_processing_library.h" @@ -118,26 +121,25 @@ TEST_F(SplTest, InlineTest) { EXPECT_EQ(104, WebRtcSpl_AddSatW16(a16, b16)); EXPECT_EQ(138, WebRtcSpl_SubSatW16(a16, b16)); +} - EXPECT_EQ(109410, WebRtcSpl_AddSatW32(a32, b32)); - EXPECT_EQ(112832, WebRtcSpl_SubSatW32(a32, b32)); - - a32 = 0x80000000; - b32 = 0x80000000; - // Cast to signed int to avoid compiler complaint on gtest.h. - EXPECT_EQ(static_cast(0x80000000), WebRtcSpl_AddSatW32(a32, b32)); - a32 = 0x7fffffff; - b32 = 0x7fffffff; - EXPECT_EQ(0x7fffffff, WebRtcSpl_AddSatW32(a32, b32)); - a32 = 0; - b32 = 0x80000000; - EXPECT_EQ(0x7fffffff, WebRtcSpl_SubSatW32(a32, b32)); - a32 = 0x7fffffff; - b32 = 0x80000000; - EXPECT_EQ(0x7fffffff, WebRtcSpl_SubSatW32(a32, b32)); - a32 = 0x80000000; - b32 = 0x7fffffff; - EXPECT_EQ(static_cast(0x80000000), WebRtcSpl_SubSatW32(a32, b32)); +TEST_F(SplTest, AddSubSatW32) { + static constexpr int32_t kAddSubArgs[] = { + INT32_MIN, INT32_MIN + 1, -3, -2, -1, 0, 1, -1, 2, + 3, INT32_MAX - 1, INT32_MAX}; + for (int32_t a : kAddSubArgs) { + for (int32_t b : kAddSubArgs) { + const int64_t sum = std::max( + INT32_MIN, std::min(INT32_MAX, static_cast(a) + b)); + const int64_t diff = std::max( + INT32_MIN, std::min(INT32_MAX, static_cast(a) - b)); + std::ostringstream ss; + ss << a << " +/- " << b << ": sum " << sum << ", diff " << diff; + SCOPED_TRACE(ss.str()); + EXPECT_EQ(sum, WebRtcSpl_AddSatW32(a, b)); + EXPECT_EQ(diff, WebRtcSpl_SubSatW32(a, b)); + } + } } TEST_F(SplTest, MathOperationsTest) {