From 0a2277448ec22b0c9af98d9ebc1acd757be8b89e Mon Sep 17 00:00:00 2001 From: "henrik.lundin@webrtc.org" Date: Thu, 24 Apr 2014 08:11:39 +0000 Subject: [PATCH] Fixing a bug in ACM2 where the output frame energy was incorrectly set The value of AudioFrame::energy_ must be set to -1 in order to have the energy calculated later on in the AudioConferenceMixer module. This was not the case in ACM2, where the value was set to 0 instead. This resulted in bad audio for multi-party calls (5 or more participants). Implemented a unit test to verify ACM output frame. BUG=3255 TBR=turaj@webrtc.org Review URL: https://webrtc-codereview.appspot.com/12369005 git-svn-id: http://webrtc.googlecode.com/svn/trunk@5969 4adac7df-926f-26a2-2b94-8c16560cd09d --- .../main/acm2/audio_coding_module_impl.cc | 4 +++- .../main/acm2/audio_coding_module_unittest.cc | 20 +++++++++++++++++-- 2 files changed, 21 insertions(+), 3 deletions(-) diff --git a/webrtc/modules/audio_coding/main/acm2/audio_coding_module_impl.cc b/webrtc/modules/audio_coding/main/acm2/audio_coding_module_impl.cc index 26a4d30b98..e8519609e9 100644 --- a/webrtc/modules/audio_coding/main/acm2/audio_coding_module_impl.cc +++ b/webrtc/modules/audio_coding/main/acm2/audio_coding_module_impl.cc @@ -1707,7 +1707,9 @@ int AudioCodingModuleImpl::PlayoutData10Ms(int desired_freq_hz, } audio_frame->id_ = id_; - audio_frame->energy_ = 0; + // The energy must be -1 in order to have the energy calculated later on in + // the AudioConferenceMixer module. + audio_frame->energy_ = static_cast(-1); audio_frame->timestamp_ = 0; return 0; } diff --git a/webrtc/modules/audio_coding/main/acm2/audio_coding_module_unittest.cc b/webrtc/modules/audio_coding/main/acm2/audio_coding_module_unittest.cc index 0f3e73ef51..93cf4d5796 100644 --- a/webrtc/modules/audio_coding/main/acm2/audio_coding_module_unittest.cc +++ b/webrtc/modules/audio_coding/main/acm2/audio_coding_module_unittest.cc @@ -58,8 +58,9 @@ class RtpUtility { class AudioCodingModuleTest : public ::testing::Test { protected: AudioCodingModuleTest() - : rtp_utility_(new RtpUtility(kFrameSizeSamples, kPayloadType)), - acm2_(AudioCodingModule::Create(1 /*id*/)) {} + : id_(1), + rtp_utility_(new RtpUtility(kFrameSizeSamples, kPayloadType)), + acm2_(AudioCodingModule::Create(id_)) {} ~AudioCodingModuleTest() {} @@ -92,6 +93,7 @@ class AudioCodingModuleTest : public ::testing::Test { ASSERT_EQ(0, acm2_->PlayoutData10Ms(-1, &audio_frame)); } + const int id_; scoped_ptr rtp_utility_; scoped_ptr acm2_; @@ -169,4 +171,18 @@ TEST_F(AudioCodingModuleTest, DISABLED_ON_ANDROID(NetEqCalls)) { EXPECT_EQ(kNumPlcCng, stats.decoded_plc_cng); } +TEST_F(AudioCodingModuleTest, VerifyOutputFrame) { + AudioFrame audio_frame; + const int kSampleRateHz = 32000; + EXPECT_EQ(0, acm2_->PlayoutData10Ms(kSampleRateHz, &audio_frame)); + // The energy must be -1 in order to have the energy calculated later on in + // the AudioConferenceMixer module. + EXPECT_EQ(static_cast(-1), audio_frame.energy_); + EXPECT_EQ(id_, audio_frame.id_); + EXPECT_EQ(0u, audio_frame.timestamp_); + EXPECT_GT(audio_frame.num_channels_, 0); + EXPECT_EQ(kSampleRateHz / 100, audio_frame.samples_per_channel_); + EXPECT_EQ(kSampleRateHz, audio_frame.sample_rate_hz_); +} + } // namespace webrtc