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/webrtc@7025 4adac7df-926f-26a2-2b94-8c16560cd09d
diff --git a/modules/audio_coding/neteq/mock/mock_expand.h b/modules/audio_coding/neteq/mock/mock_expand.h
new file mode 100644
index 0000000..45e3239
--- /dev/null
+++ b/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_
diff --git a/modules/audio_coding/neteq/normal.cc b/modules/audio_coding/neteq/normal.cc
index bfde179..46d03fb 100644
--- a/modules/audio_coding/neteq/normal.cc
+++ b/modules/audio_coding/neteq/normal.cc
@@ -37,6 +37,11 @@
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 @@
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) &&
diff --git a/modules/audio_coding/neteq/normal_unittest.cc b/modules/audio_coding/neteq/normal_unittest.cc
index c855865..309dad3 100644
--- a/modules/audio_coding/neteq/normal_unittest.cc
+++ b/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 @@
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
diff --git a/modules/modules.gyp b/modules/modules.gyp
index b302bdb..7254fee 100644
--- a/modules/modules.gyp
+++ b/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',
