From ee0fb187a583b0c66b2c9fc8571411dca510ce7b Mon Sep 17 00:00:00 2001 From: "henrik.lundin@webrtc.org" Date: Tue, 2 Sep 2014 13:22:11 +0000 Subject: [PATCH] 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 --- .../audio_coding/neteq/mock/mock_expand.h | 58 ++++++++++++++ webrtc/modules/audio_coding/neteq/normal.cc | 11 ++- .../audio_coding/neteq/normal_unittest.cc | 80 +++++++++++++++++++ webrtc/modules/modules.gyp | 1 + 4 files changed, 149 insertions(+), 1 deletion(-) create mode 100644 webrtc/modules/audio_coding/neteq/mock/mock_expand.h diff --git a/webrtc/modules/audio_coding/neteq/mock/mock_expand.h b/webrtc/modules/audio_coding/neteq/mock/mock_expand.h new file mode 100644 index 000000000..45e3239f6 --- /dev/null +++ b/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_ diff --git a/webrtc/modules/audio_coding/neteq/normal.cc b/webrtc/modules/audio_coding/neteq/normal.cc index bfde179bd..46d03fb8c 100644 --- a/webrtc/modules/audio_coding/neteq/normal.cc +++ b/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) && diff --git a/webrtc/modules/audio_coding/neteq/normal_unittest.cc b/webrtc/modules/audio_coding/neteq/normal_unittest.cc index c855865cf..309dad327 100644 --- a/webrtc/modules/audio_coding/neteq/normal_unittest.cc +++ b/webrtc/modules/audio_coding/neteq/normal_unittest.cc @@ -15,11 +15,17 @@ #include #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 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 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/webrtc/modules/modules.gyp b/webrtc/modules/modules.gyp index b302bdbb9..7254feed4 100644 --- a/webrtc/modules/modules.gyp +++ b/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',