Fixing an overflow issue in NetEq::BackgroundNoise

webrtc/modules/audio_coding/neteq/background_noise.cc

Index: webrtc/modules/audio_coding/neteq/background_noise.cc
diff --git a/webrtc/modules/audio_coding/neteq/background_noise.cc b/webrtc/modules/audio_coding/neteq/background_noise.cc
index 9cfd6cb40ed7c25951ec118e78e431a6df56dafe..d80c9e7809ecee03c7fc74e7917620bb6bb696d8 100644
--- a/webrtc/modules/audio_coding/neteq/background_noise.cc
+++ b/webrtc/modules/audio_coding/neteq/background_noise.cc
@@ -14,6 +14,7 @@
 #include <string.h>  // memcpy
 
 #include <algorithm>  // min, max
+#include <limits>
 
 #include "webrtc/common_audio/signal_processing/include/signal_processing_library.h"
 #include "webrtc/modules/audio_coding/neteq/audio_multi_vector.h"
@@ -101,9 +102,15 @@ void BackgroundNoise::Update(const AudioMultiVector& input,
       // Check spectral flatness.
       // Comparing the residual variance with the input signal variance tells
       // if the spectrum is flat or not.
-      // If 20 * residual_energy >= sample_energy << 6, the spectrum is flat
+      // If 5 * residual_energy >= sample_energy << 4, the spectrum is flat
       // enough.  Also ensure that the energy is non-zero.
-      if ((residual_energy * 20 >= (sample_energy << 6)) &&
+      // If residual_energy is too large to fit in an int32_t after multiplying
+      // with 5, then simply skip the comparison, and consider the spectrum not
+      // flat enough.
+      constexpr int32_t kMaxEnergyToCompare =
+          std::numeric_limits<int32_t>::max() / 5;
+      if ((residual_energy <= kMaxEnergyToCompare) &&
+          (residual_energy * 5 >= (sample_energy << 4)) &&
           (sample_energy > 0)) {

[kwiberg-webrtc, 2017/02/28 10:31:35]

Hmm. Wouldn't it make more sense to have

  (residual_energy >= kMaxEnergyToCompare) || ...

since the original code enters the if block if residual_energy is *high* enough?

Alternatively, consider doing

  if (sample_energy > 0 && int64_t{5} * residual_energy >= 16 * sample_energy)

(Or use int64_t{16} if the rhs could overflow as well.)

Godbolt experiment here: https://godbolt.org/g/0DvNYi
Note that testing against kMaxEnergyToCompare seems to introduce a branch on all
of x86-64 clang, x86-64 gcc, arm64 gcc, and arm32 gcc. Doing 64-bit arithmetic
seems to result in a bunch more instructions on arm32 (all of them cheap
arithmetic things, though), but looks very efficient on the 64-bit platforms.

[hlundin-webrtc, 2017/02/28 12:15:32]

I went for the int64_t{x} suggestion. Thanks!

         // Spectrum is flat enough; save filter parameters.
         // |temp_signal| + |kVecLen| - |kMaxLpcOrder| points at the first of the