From 21299d4e00781e199a53ba33ec192cdce920acec Mon Sep 17 00:00:00 2001 From: "andrew@webrtc.org" Date: Wed, 14 May 2014 19:00:59 +0000 Subject: [PATCH] Remove the use of AudioFrame::energy_ from AudioProcessing and VoE. We want to remove energy_ entirely as we've seen that carrying around this potentially invalid value is dangerous. Results in the removal of AudioBuffer::is_muted(). This wasn't used in practice any longer, after the level calculation moved directly to channel.cc Instead, now use ProcessMuted() in channel.cc, to shortcut the level computation when the signal is muted. BUG=3315 TESTED=Muting the channel in voe_cmd_test results in rms=127. R=bjornv@webrtc.org, kwiberg@webrtc.org Review URL: https://webrtc-codereview.appspot.com/12529004 git-svn-id: http://webrtc.googlecode.com/svn/trunk@6159 4adac7df-926f-26a2-2b94-8c16560cd09d --- webrtc/modules/audio_processing/audio_buffer.cc | 9 --------- webrtc/modules/audio_processing/audio_buffer.h | 3 --- .../modules/audio_processing/level_estimator_impl.cc | 10 +++------- webrtc/modules/audio_processing/rms_level.cc | 8 ++++---- webrtc/modules/audio_processing/rms_level.h | 6 ++++++ .../test/audio_processing_unittest.cc | 9 --------- .../modules/utility/source/audio_frame_operations.cc | 1 - .../source/audio_frame_operations_unittest.cc | 4 ---- webrtc/voice_engine/channel.cc | 12 ++++++++---- 9 files changed, 21 insertions(+), 41 deletions(-) diff --git a/webrtc/modules/audio_processing/audio_buffer.cc b/webrtc/modules/audio_processing/audio_buffer.cc index 024b700cac..eb9bea57ff 100644 --- a/webrtc/modules/audio_processing/audio_buffer.cc +++ b/webrtc/modules/audio_processing/audio_buffer.cc @@ -99,7 +99,6 @@ AudioBuffer::AudioBuffer(int input_samples_per_channel, num_mixed_low_pass_channels_(0), reference_copied_(false), activity_(AudioFrame::kVadUnknown), - is_muted_(false), data_(NULL), keyboard_data_(NULL), channels_(new ChannelBuffer(proc_samples_per_channel_, @@ -223,7 +222,6 @@ void AudioBuffer::InitForNewData() { num_mixed_low_pass_channels_ = 0; reference_copied_ = false; activity_ = AudioFrame::kVadUnknown; - is_muted_ = false; } const int16_t* AudioBuffer::data(int channel) const { @@ -307,10 +305,6 @@ AudioFrame::VADActivity AudioBuffer::activity() const { return activity_; } -bool AudioBuffer::is_muted() const { - return is_muted_; -} - int AudioBuffer::num_channels() const { return num_proc_channels_; } @@ -336,9 +330,6 @@ void AudioBuffer::DeinterleaveFrom(AudioFrame* frame) { assert(frame->samples_per_channel_ == proc_samples_per_channel_); InitForNewData(); activity_ = frame->vad_activity_; - if (frame->energy_ == 0) { - is_muted_ = true; - } if (num_proc_channels_ == 1) { // We can get away with a pointer assignment in this case. diff --git a/webrtc/modules/audio_processing/audio_buffer.h b/webrtc/modules/audio_processing/audio_buffer.h index 2b93510638..c05ffc90bf 100644 --- a/webrtc/modules/audio_processing/audio_buffer.h +++ b/webrtc/modules/audio_processing/audio_buffer.h @@ -71,8 +71,6 @@ class AudioBuffer { void set_activity(AudioFrame::VADActivity activity); AudioFrame::VADActivity activity() const; - bool is_muted() const; - // Use for int16 interleaved data. void DeinterleaveFrom(AudioFrame* audioFrame); void InterleaveTo(AudioFrame* audioFrame) const; @@ -106,7 +104,6 @@ class AudioBuffer { int num_mixed_low_pass_channels_; bool reference_copied_; AudioFrame::VADActivity activity_; - bool is_muted_; // If non-null, use this instead of channels_->channel(0). This is an // optimization for the case num_proc_channels_ == 1 that allows us to point diff --git a/webrtc/modules/audio_processing/level_estimator_impl.cc b/webrtc/modules/audio_processing/level_estimator_impl.cc index d209d40783..cfe295a6a0 100644 --- a/webrtc/modules/audio_processing/level_estimator_impl.cc +++ b/webrtc/modules/audio_processing/level_estimator_impl.cc @@ -12,6 +12,7 @@ #include "webrtc/modules/audio_processing/audio_buffer.h" #include "webrtc/modules/audio_processing/include/audio_processing.h" +#include "webrtc/modules/audio_processing/rms_level.h" #include "webrtc/system_wrappers/interface/critical_section_wrapper.h" namespace webrtc { @@ -29,13 +30,8 @@ int LevelEstimatorImpl::ProcessStream(AudioBuffer* audio) { } RMSLevel* rms_level = static_cast(handle(0)); - if (audio->is_muted()) { - rms_level->ProcessMuted(audio->samples_per_channel() * - audio->num_channels()); - } else { - for (int i = 0; i < audio->num_channels(); ++i) { - rms_level->Process(audio->data(i), audio->samples_per_channel()); - } + for (int i = 0; i < audio->num_channels(); ++i) { + rms_level->Process(audio->data(i), audio->samples_per_channel()); } return AudioProcessing::kNoError; diff --git a/webrtc/modules/audio_processing/rms_level.cc b/webrtc/modules/audio_processing/rms_level.cc index 963622b812..14136bf304 100644 --- a/webrtc/modules/audio_processing/rms_level.cc +++ b/webrtc/modules/audio_processing/rms_level.cc @@ -15,16 +15,16 @@ namespace webrtc { -static const float kMaxSquaredLevel = 32768.0 * 32768.0; +static const float kMaxSquaredLevel = 32768 * 32768; RMSLevel::RMSLevel() - : sum_square_(0.0), + : sum_square_(0), sample_count_(0) {} RMSLevel::~RMSLevel() {} void RMSLevel::Reset() { - sum_square_ = 0.0; + sum_square_ = 0; sample_count_ = 0; } @@ -40,7 +40,7 @@ void RMSLevel::ProcessMuted(int length) { } int RMSLevel::RMS() { - if (sample_count_ == 0 || sum_square_ == 0.0) { + if (sample_count_ == 0 || sum_square_ == 0) { Reset(); return kMinLevel; } diff --git a/webrtc/modules/audio_processing/rms_level.h b/webrtc/modules/audio_processing/rms_level.h index 1b19803ba7..055d271bb1 100644 --- a/webrtc/modules/audio_processing/rms_level.h +++ b/webrtc/modules/audio_processing/rms_level.h @@ -8,6 +8,9 @@ * be found in the AUTHORS file in the root of the source tree. */ +#ifndef WEBRTC_MODULES_AUDIO_PROCESSING_RMS_LEVEL_H_ +#define WEBRTC_MODULES_AUDIO_PROCESSING_RMS_LEVEL_H_ + #include "webrtc/typedefs.h" namespace webrtc { @@ -49,3 +52,6 @@ class RMSLevel { }; } // namespace webrtc + +#endif // WEBRTC_MODULES_AUDIO_PROCESSING_RMS_LEVEL_H_ + diff --git a/webrtc/modules/audio_processing/test/audio_processing_unittest.cc b/webrtc/modules/audio_processing/test/audio_processing_unittest.cc index 0c5b67df51..bfb77664ca 100644 --- a/webrtc/modules/audio_processing/test/audio_processing_unittest.cc +++ b/webrtc/modules/audio_processing/test/audio_processing_unittest.cc @@ -1229,15 +1229,6 @@ TEST_F(ApmTest, LevelEstimator) { EXPECT_EQ(apm_->kNoError, apm_->ProcessStream(frame_)); EXPECT_EQ(70, apm_->level_estimator()->RMS()); - // Min value if energy_ == 0. - SetFrameTo(frame_, 10000); - uint32_t energy = frame_->energy_; // Save default to restore below. - frame_->energy_ = 0; - EXPECT_EQ(apm_->kNoError, apm_->ProcessStream(frame_)); - EXPECT_EQ(apm_->kNoError, apm_->ProcessStream(frame_)); - EXPECT_EQ(127, apm_->level_estimator()->RMS()); - frame_->energy_ = energy; - // Verify reset after enable/disable. SetFrameTo(frame_, 32767); EXPECT_EQ(apm_->kNoError, apm_->ProcessStream(frame_)); diff --git a/webrtc/modules/utility/source/audio_frame_operations.cc b/webrtc/modules/utility/source/audio_frame_operations.cc index 18dba52b65..e3b0010476 100644 --- a/webrtc/modules/utility/source/audio_frame_operations.cc +++ b/webrtc/modules/utility/source/audio_frame_operations.cc @@ -72,7 +72,6 @@ void AudioFrameOperations::SwapStereoChannels(AudioFrame* frame) { void AudioFrameOperations::Mute(AudioFrame& frame) { memset(frame.data_, 0, sizeof(int16_t) * frame.samples_per_channel_ * frame.num_channels_); - frame.energy_ = 0; } int AudioFrameOperations::Scale(float left, float right, AudioFrame& frame) { diff --git a/webrtc/modules/utility/source/audio_frame_operations_unittest.cc b/webrtc/modules/utility/source/audio_frame_operations_unittest.cc index 34c08a89e6..f4d881cf87 100644 --- a/webrtc/modules/utility/source/audio_frame_operations_unittest.cc +++ b/webrtc/modules/utility/source/audio_frame_operations_unittest.cc @@ -142,17 +142,13 @@ TEST_F(AudioFrameOperationsTest, SwapStereoChannelsFailsOnMono) { TEST_F(AudioFrameOperationsTest, MuteSucceeds) { SetFrameData(&frame_, 1000, 1000); - frame_.energy_ = 1000 * 1000 * frame_.samples_per_channel_ * - frame_.num_channels_; AudioFrameOperations::Mute(frame_); AudioFrame muted_frame; muted_frame.samples_per_channel_ = 320; muted_frame.num_channels_ = 2; SetFrameData(&muted_frame, 0, 0); - muted_frame.energy_ = 0; VerifyFramesAreEqual(muted_frame, frame_); - EXPECT_EQ(muted_frame.energy_, frame_.energy_); } // TODO(andrew): should not allow negative scales. diff --git a/webrtc/voice_engine/channel.cc b/webrtc/voice_engine/channel.cc index 6bbce78268..f919c3d09f 100644 --- a/webrtc/voice_engine/channel.cc +++ b/webrtc/voice_engine/channel.cc @@ -3689,9 +3689,9 @@ Channel::PrepareEncodeAndSend(int mixingFrequency) MixOrReplaceAudioWithFile(mixingFrequency); } - if (Mute()) - { - AudioFrameOperations::Mute(_audioFrame); + bool is_muted = Mute(); // Cache locally as Mute() takes a lock. + if (is_muted) { + AudioFrameOperations::Mute(_audioFrame); } if (channel_state_.Get().input_external_media) @@ -3714,7 +3714,11 @@ Channel::PrepareEncodeAndSend(int mixingFrequency) if (_includeAudioLevelIndication) { int length = _audioFrame.samples_per_channel_ * _audioFrame.num_channels_; - rms_level_.Process(_audioFrame.data_, length); + if (is_muted) { + rms_level_.ProcessMuted(length); + } else { + rms_level_.Process(_audioFrame.data_, length); + } } return 0;