Divide-by-zero problem in NetEq's Normal::Process fixed

Adding a couple of tests that tries to trigger a certain divide-by-zero
issue. The tests triggered the issue, but this CL also includes a fix
for this.

BUG=3761
R=tina.legrand@webrtc.org

Review URL: https://webrtc-codereview.appspot.com/22269004

git-svn-id: http://webrtc.googlecode.com/svn/trunk@7025 4adac7df-926f-26a2-2b94-8c16560cd09d

webrtc/modules/audio_coding/neteq/mock/mock_expand.h

@@ -0,0 +1,58 @@
+/*
+ *  Copyright (c) 2014 The WebRTC project authors. All Rights Reserved.
+ *
+ *  Use of this source code is governed by a BSD-style license
+ *  that can be found in the LICENSE file in the root of the source
+ *  tree. An additional intellectual property rights grant can be found
+ *  in the file PATENTS.  All contributing project authors may
+ *  be found in the AUTHORS file in the root of the source tree.
+ */
+
+#ifndef WEBRTC_MODULES_AUDIO_CODING_NETEQ_MOCK_MOCK_EXPAND_H_
+#define WEBRTC_MODULES_AUDIO_CODING_NETEQ_MOCK_MOCK_EXPAND_H_
+
+#include "webrtc/modules/audio_coding/neteq/expand.h"
+
+#include "testing/gmock/include/gmock/gmock.h"
+
+namespace webrtc {
+
+class MockExpand : public Expand {
+ public:
+  MockExpand(BackgroundNoise* background_noise,
+             SyncBuffer* sync_buffer,
+             RandomVector* random_vector,
+             int fs,
+             size_t num_channels)
+      : Expand(background_noise, sync_buffer, random_vector, fs, num_channels) {
+  }
+  virtual ~MockExpand() { Die(); }
+  MOCK_METHOD0(Die, void());
+  MOCK_METHOD0(Reset,
+      void());
+  MOCK_METHOD1(Process,
+      int(AudioMultiVector* output));
+  MOCK_METHOD0(SetParametersForNormalAfterExpand,
+      void());
+  MOCK_METHOD0(SetParametersForMergeAfterExpand,
+      void());
+  MOCK_CONST_METHOD0(overlap_length,
+      size_t());
+};
+
+}  // namespace webrtc
+
+namespace webrtc {
+
+class MockExpandFactory : public ExpandFactory {
+ public:
+  MOCK_CONST_METHOD5(Create,
+                     Expand*(BackgroundNoise* background_noise,
+                             SyncBuffer* sync_buffer,
+                             RandomVector* random_vector,
+                             int fs,
+                             size_t num_channels));
+};
+
+}  // namespace webrtc
+#endif  // WEBRTC_MODULES_AUDIO_CODING_NETEQ_MOCK_MOCK_EXPAND_H_

webrtc/modules/audio_coding/neteq/normal.cc

@@ -37,6 +37,11 @@ int Normal::Process(const int16_t* input,
 
   assert(output->Empty());
   // Output should be empty at this point.
+  if (length % output->Channels() != 0) {
+    // The length does not match the number of channels.
+    output->Clear();
+    return 0;
+  }
   output->PushBackInterleaved(input, length);
   int16_t* signal = &(*output)[0][0];
 
@@ -78,7 +83,11 @@ int Normal::Process(const int16_t* input,
       scaling = std::max(scaling, 0);  // |scaling| should always be >= 0.
       int32_t energy = WebRtcSpl_DotProductWithScale(signal, signal,
                                                      energy_length, scaling);
-      energy = energy / (energy_length >> scaling);
+      if ((energy_length >> scaling) > 0) {
+        energy = energy / (energy_length >> scaling);
+      } else {
+        energy = 0;
+      }
 
       int mute_factor;
       if ((energy != 0) &&

webrtc/modules/audio_coding/neteq/normal_unittest.cc

@@ -15,11 +15,17 @@
 #include <vector>
 
 #include "gtest/gtest.h"
+#include "webrtc/common_audio/signal_processing/include/signal_processing_library.h"
+#include "webrtc/modules/audio_coding/neteq/audio_multi_vector.h"
 #include "webrtc/modules/audio_coding/neteq/background_noise.h"
 #include "webrtc/modules/audio_coding/neteq/expand.h"
 #include "webrtc/modules/audio_coding/neteq/mock/mock_decoder_database.h"
+#include "webrtc/modules/audio_coding/neteq/mock/mock_expand.h"
 #include "webrtc/modules/audio_coding/neteq/random_vector.h"
 #include "webrtc/modules/audio_coding/neteq/sync_buffer.h"
+#include "webrtc/system_wrappers/interface/scoped_ptr.h"
+
+using ::testing::_;
 
 namespace webrtc {
 
@@ -35,6 +41,80 @@ TEST(Normal, CreateAndDestroy) {
   EXPECT_CALL(db, Die());  // Called when |db| goes out of scope.
 }
 
+TEST(Normal, AvoidDivideByZero) {
+  WebRtcSpl_Init();
+  MockDecoderDatabase db;
+  int fs = 8000;
+  size_t channels = 1;
+  BackgroundNoise bgn(channels);
+  SyncBuffer sync_buffer(1, 1000);
+  RandomVector random_vector;
+  MockExpand expand(&bgn, &sync_buffer, &random_vector, fs, channels);
+  Normal normal(fs, &db, bgn, &expand);
+
+  int16_t input[1000] = {0};
+  scoped_ptr<int16_t[]> mute_factor_array(new int16_t[channels]);
+  for (size_t i = 0; i < channels; ++i) {
+    mute_factor_array[i] = 16384;
+  }
+  AudioMultiVector output(channels);
+
+  // Zero input length.
+  EXPECT_EQ(
+      0,
+      normal.Process(input, 0, kModeExpand, mute_factor_array.get(), &output));
+  EXPECT_EQ(0u, output.Size());
+
+  // Try to make energy_length >> scaling = 0;
+  EXPECT_CALL(expand, SetParametersForNormalAfterExpand());
+  EXPECT_CALL(expand, Process(_));
+  EXPECT_CALL(expand, Reset());
+  // If input_size_samples < 64, then energy_length in Normal::Process() will
+  // be equal to input_size_samples. Since the input is all zeros, decoded_max
+  // will be zero, and scaling will be >= 6. Thus, energy_length >> scaling = 0,
+  // and using this as a denominator would lead to problems.
+  int input_size_samples = 63;
+  EXPECT_EQ(input_size_samples,
+            normal.Process(input,
+                           input_size_samples,
+                           kModeExpand,
+                           mute_factor_array.get(),
+                           &output));
+
+  EXPECT_CALL(db, Die());      // Called when |db| goes out of scope.
+  EXPECT_CALL(expand, Die());  // Called when |expand| goes out of scope.
+}
+
+TEST(Normal, InputLengthAndChannelsDoNotMatch) {
+  WebRtcSpl_Init();
+  MockDecoderDatabase db;
+  int fs = 8000;
+  size_t channels = 2;
+  BackgroundNoise bgn(channels);
+  SyncBuffer sync_buffer(channels, 1000);
+  RandomVector random_vector;
+  MockExpand expand(&bgn, &sync_buffer, &random_vector, fs, channels);
+  Normal normal(fs, &db, bgn, &expand);
+
+  int16_t input[1000] = {0};
+  scoped_ptr<int16_t[]> mute_factor_array(new int16_t[channels]);
+  for (size_t i = 0; i < channels; ++i) {
+    mute_factor_array[i] = 16384;
+  }
+  AudioMultiVector output(channels);
+
+  // Let the number of samples be one sample less than 80 samples per channel.
+  size_t input_len = 80 * channels - 1;
+  EXPECT_EQ(
+      0,
+      normal.Process(
+          input, input_len, kModeExpand, mute_factor_array.get(), &output));
+  EXPECT_EQ(0u, output.Size());
+
+  EXPECT_CALL(db, Die());      // Called when |db| goes out of scope.
+  EXPECT_CALL(expand, Die());  // Called when |expand| goes out of scope.
+}
+
 // TODO(hlundin): Write more tests.
 
 }  // namespace webrtc

webrtc/modules/modules.gyp

@@ -150,6 +150,7 @@
             'audio_coding/neteq/mock/mock_delay_peak_detector.h',
             'audio_coding/neteq/mock/mock_dtmf_buffer.h',
             'audio_coding/neteq/mock/mock_dtmf_tone_generator.h',
+            'audio_coding/neteq/mock/mock_expand.h',
             'audio_coding/neteq/mock/mock_external_decoder_pcm16b.h',
             'audio_coding/neteq/mock/mock_packet_buffer.h',
             'audio_coding/neteq/mock/mock_payload_splitter.h',