From d547d862d5bfb2cfaa91f8ddae36be8b5e7e46d0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Per=20=C3=85hgren?= Date: Fri, 3 May 2019 15:48:47 +0200 Subject: [PATCH] Remove the enable flag from AEC2 and AECM MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This CL removes the redundant enable flags from AEC2 and AECM Bug: webrtc:5298 Change-Id: Icc575abf1c368dda02ca77f057d166f1c921f662 Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/135100 Commit-Queue: Per Åhgren Reviewed-by: Sam Zackrisson Cr-Commit-Position: refs/heads/master@{#27848} --- .../audio_processing/audio_processing_impl.cc | 14 ++--- .../echo_cancellation_bit_exact_unittest.cc | 1 - .../echo_cancellation_impl.cc | 53 +++---------------- .../audio_processing/echo_cancellation_impl.h | 3 -- .../echo_cancellation_impl_unittest.cc | 28 ---------- .../echo_control_mobile_bit_exact_unittest.cc | 1 - .../echo_control_mobile_impl.cc | 39 -------------- .../echo_control_mobile_impl.h | 5 -- .../echo_control_mobile_unittest.cc | 8 --- 9 files changed, 9 insertions(+), 143 deletions(-) diff --git a/modules/audio_processing/audio_processing_impl.cc b/modules/audio_processing/audio_processing_impl.cc index ac8afdac64..13db45f743 100644 --- a/modules/audio_processing/audio_processing_impl.cc +++ b/modules/audio_processing/audio_processing_impl.cc @@ -1765,10 +1765,8 @@ AudioProcessing::Config AudioProcessingImpl::GetConfig() const { bool AudioProcessingImpl::UpdateActiveSubmoduleStates() { return submodule_states_.Update( config_.high_pass_filter.enabled, - private_submodules_->echo_cancellation && - private_submodules_->echo_cancellation->is_enabled(), - private_submodules_->echo_control_mobile && - private_submodules_->echo_control_mobile->is_enabled(), + !!private_submodules_->echo_cancellation, + !!private_submodules_->echo_control_mobile, config_.residual_echo_detector.enabled, public_submodules_->noise_suppression->is_enabled(), public_submodules_->gain_control->is_enabled(), @@ -1860,8 +1858,6 @@ void AudioProcessingImpl::InitializeEchoController() { proc_split_sample_rate_hz(), num_reverse_channels(), num_output_channels()); - private_submodules_->echo_control_mobile->Enable(true); - private_submodules_->echo_cancellation.reset(); aec_render_signal_queue_.reset(); return; @@ -1897,8 +1893,6 @@ void AudioProcessingImpl::InitializeEchoController() { proc_sample_rate_hz(), num_reverse_channels(), num_output_channels(), num_proc_channels()); - private_submodules_->echo_cancellation->Enable(true); - private_submodules_->echo_cancellation->set_suppression_level( config_.echo_canceller.legacy_moderate_suppression_level ? EchoCancellationImpl::SuppressionLevel::kModerateSuppression @@ -1991,9 +1985,7 @@ void AudioProcessingImpl::WriteAecDumpConfigMessage(bool forced) { private_submodules_->echo_cancellation->suppression_level()) : 0; - apm_config.aecm_enabled = - private_submodules_->echo_control_mobile && - private_submodules_->echo_control_mobile->is_enabled(); + apm_config.aecm_enabled = !!private_submodules_->echo_control_mobile; apm_config.aecm_comfort_noise_enabled = private_submodules_->echo_control_mobile && private_submodules_->echo_control_mobile->is_comfort_noise_enabled(); diff --git a/modules/audio_processing/echo_cancellation_bit_exact_unittest.cc b/modules/audio_processing/echo_cancellation_bit_exact_unittest.cc index 37898c0054..80f36a8c0e 100644 --- a/modules/audio_processing/echo_cancellation_bit_exact_unittest.cc +++ b/modules/audio_processing/echo_cancellation_bit_exact_unittest.cc @@ -26,7 +26,6 @@ void SetupComponent(int sample_rate_hz, bool drift_compensation_enabled, EchoCancellationImpl* echo_canceller) { echo_canceller->Initialize(sample_rate_hz, 1, 1, 1); - echo_canceller->Enable(true); echo_canceller->set_suppression_level(suppression_level); echo_canceller->enable_drift_compensation(drift_compensation_enabled); diff --git a/modules/audio_processing/echo_cancellation_impl.cc b/modules/audio_processing/echo_cancellation_impl.cc index ac13ebe077..21ba177b5d 100644 --- a/modules/audio_processing/echo_cancellation_impl.cc +++ b/modules/audio_processing/echo_cancellation_impl.cc @@ -120,10 +120,6 @@ EchoCancellationImpl::~EchoCancellationImpl() = default; void EchoCancellationImpl::ProcessRenderAudio( rtc::ArrayView packed_render_audio) { - if (!enabled_) { - return; - } - RTC_DCHECK(stream_properties_); size_t handle_index = 0; size_t buffer_index = 0; @@ -143,10 +139,6 @@ void EchoCancellationImpl::ProcessRenderAudio( int EchoCancellationImpl::ProcessCaptureAudio(AudioBuffer* audio, int stream_delay_ms) { - if (!enabled_) { - return AudioProcessing::kNoError; - } - const int stream_delay_ms_use = enforce_zero_stream_delay_ ? 0 : stream_delay_ms; @@ -198,27 +190,6 @@ int EchoCancellationImpl::ProcessCaptureAudio(AudioBuffer* audio, return AudioProcessing::kNoError; } -int EchoCancellationImpl::Enable(bool enable) { - if (enable && !enabled_) { - enabled_ = enable; // Must be set before Initialize() is called. - - // TODO(peah): Simplify once the Enable function has been removed from - // the public APM API. - RTC_DCHECK(stream_properties_); - Initialize(stream_properties_->sample_rate_hz, - stream_properties_->num_reverse_channels, - stream_properties_->num_output_channels, - stream_properties_->num_proc_channels); - } else { - enabled_ = enable; - } - return AudioProcessing::kNoError; -} - -bool EchoCancellationImpl::is_enabled() const { - return enabled_; -} - int EchoCancellationImpl::set_suppression_level(SuppressionLevel level) { if (MapSetting(level) == -1) { return AudioProcessing::kBadParameterError; @@ -256,7 +227,7 @@ int EchoCancellationImpl::enable_metrics(bool enable) { } bool EchoCancellationImpl::are_metrics_enabled() const { - return enabled_ && metrics_enabled_; + return metrics_enabled_; } // TODO(ajm): we currently just use the metrics from the first AEC. Think more @@ -266,7 +237,7 @@ int EchoCancellationImpl::GetMetrics(Metrics* metrics) { return AudioProcessing::kNullPointerError; } - if (!enabled_ || !metrics_enabled_) { + if (!metrics_enabled_) { return AudioProcessing::kNotEnabledError; } @@ -313,7 +284,7 @@ int EchoCancellationImpl::enable_delay_logging(bool enable) { } bool EchoCancellationImpl::is_delay_logging_enabled() const { - return enabled_ && delay_logging_enabled_; + return delay_logging_enabled_; } bool EchoCancellationImpl::is_delay_agnostic_enabled() const { @@ -321,12 +292,8 @@ bool EchoCancellationImpl::is_delay_agnostic_enabled() const { } std::string EchoCancellationImpl::GetExperimentsDescription() { - if (enabled_) { - return refined_adaptive_filter_enabled_ - ? "Legacy AEC;RefinedAdaptiveFilter;" - : "Legacy AEC;"; - } - return ""; + return refined_adaptive_filter_enabled_ ? "Legacy AEC;RefinedAdaptiveFilter;" + : "Legacy AEC;"; } bool EchoCancellationImpl::is_refined_adaptive_filter_enabled() const { @@ -353,7 +320,7 @@ int EchoCancellationImpl::GetDelayMetrics(int* median, return AudioProcessing::kNullPointerError; } - if (!enabled_ || !delay_logging_enabled_) { + if (!delay_logging_enabled_) { return AudioProcessing::kNotEnabledError; } @@ -367,9 +334,6 @@ int EchoCancellationImpl::GetDelayMetrics(int* median, } struct AecCore* EchoCancellationImpl::aec_core() const { - if (!enabled_) { - return NULL; - } return WebRtcAec_aec_core(cancellers_[0]->state()); } @@ -381,10 +345,6 @@ void EchoCancellationImpl::Initialize(int sample_rate_hz, new StreamProperties(sample_rate_hz, num_reverse_channels, num_output_channels, num_proc_channels)); - if (!enabled_) { - return; - } - const size_t num_cancellers_required = NumCancellersRequired(stream_properties_->num_output_channels, stream_properties_->num_reverse_channels); @@ -405,7 +365,6 @@ void EchoCancellationImpl::Initialize(int sample_rate_hz, } int EchoCancellationImpl::GetSystemDelayInSamples() const { - RTC_DCHECK(enabled_); // Report the delay for the first AEC component. return WebRtcAec_system_delay(WebRtcAec_aec_core(cancellers_[0]->state())); } diff --git a/modules/audio_processing/echo_cancellation_impl.h b/modules/audio_processing/echo_cancellation_impl.h index 25412cd8dd..a80d139c51 100644 --- a/modules/audio_processing/echo_cancellation_impl.h +++ b/modules/audio_processing/echo_cancellation_impl.h @@ -35,8 +35,6 @@ class EchoCancellationImpl { void ProcessRenderAudio(rtc::ArrayView packed_render_audio); int ProcessCaptureAudio(AudioBuffer* audio, int stream_delay_ms); - int Enable(bool enable); - bool is_enabled() const; // Differences in clock speed on the primary and reverse streams can impact // the AEC performance. On the client-side, this could be seen when different @@ -158,7 +156,6 @@ class EchoCancellationImpl { void AllocateRenderQueue(); int Configure(); - bool enabled_ = false; bool drift_compensation_enabled_; bool metrics_enabled_; SuppressionLevel suppression_level_; diff --git a/modules/audio_processing/echo_cancellation_impl_unittest.cc b/modules/audio_processing/echo_cancellation_impl_unittest.cc index 22741ee1f9..11075646d7 100644 --- a/modules/audio_processing/echo_cancellation_impl_unittest.cc +++ b/modules/audio_processing/echo_cancellation_impl_unittest.cc @@ -21,10 +21,6 @@ TEST(EchoCancellationInternalTest, ExtendedFilter) { EchoCancellationImpl echo_canceller; echo_canceller.Initialize(AudioProcessing::kSampleRate32kHz, 2, 2, 2); - EXPECT_TRUE(echo_canceller.aec_core() == nullptr); - - echo_canceller.Enable(true); - AecCore* aec_core = echo_canceller.aec_core(); ASSERT_TRUE(aec_core != NULL); // Disabled by default. @@ -50,11 +46,6 @@ TEST(EchoCancellationInternalTest, DelayAgnostic) { EchoCancellationImpl echo_canceller; echo_canceller.Initialize(AudioProcessing::kSampleRate32kHz, 1, 1, 1); - EXPECT_TRUE(echo_canceller.aec_core() == NULL); - - EXPECT_EQ(0, echo_canceller.Enable(true)); - EXPECT_TRUE(echo_canceller.is_enabled()); - AecCore* aec_core = echo_canceller.aec_core(); ASSERT_TRUE(aec_core != NULL); // Enabled by default. @@ -97,12 +88,6 @@ TEST(EchoCancellationInternalTest, InterfaceConfiguration) { } EchoCancellationImpl::Metrics metrics; - EXPECT_EQ(AudioProcessing::kNotEnabledError, - echo_canceller.GetMetrics(&metrics)); - - EXPECT_EQ(0, echo_canceller.Enable(true)); - EXPECT_TRUE(echo_canceller.is_enabled()); - EXPECT_EQ(0, echo_canceller.enable_metrics(true)); EXPECT_TRUE(echo_canceller.are_metrics_enabled()); EXPECT_EQ(0, echo_canceller.enable_metrics(false)); @@ -113,26 +98,13 @@ TEST(EchoCancellationInternalTest, InterfaceConfiguration) { EXPECT_EQ(0, echo_canceller.enable_delay_logging(false)); EXPECT_FALSE(echo_canceller.is_delay_logging_enabled()); - EXPECT_EQ(0, echo_canceller.Enable(false)); - EXPECT_FALSE(echo_canceller.is_enabled()); - int median = 0; int std = 0; float poor_fraction = 0; EXPECT_EQ(AudioProcessing::kNotEnabledError, echo_canceller.GetDelayMetrics(&median, &std, &poor_fraction)); - EXPECT_EQ(0, echo_canceller.Enable(true)); - EXPECT_TRUE(echo_canceller.is_enabled()); - EXPECT_EQ(0, echo_canceller.Enable(false)); - EXPECT_FALSE(echo_canceller.is_enabled()); - - EXPECT_EQ(0, echo_canceller.Enable(true)); - EXPECT_TRUE(echo_canceller.is_enabled()); EXPECT_TRUE(echo_canceller.aec_core() != NULL); - EXPECT_EQ(0, echo_canceller.Enable(false)); - EXPECT_FALSE(echo_canceller.is_enabled()); - EXPECT_FALSE(echo_canceller.aec_core() != NULL); } } // namespace webrtc diff --git a/modules/audio_processing/echo_control_mobile_bit_exact_unittest.cc b/modules/audio_processing/echo_control_mobile_bit_exact_unittest.cc index 7844daf522..510eda4fd1 100644 --- a/modules/audio_processing/echo_control_mobile_bit_exact_unittest.cc +++ b/modules/audio_processing/echo_control_mobile_bit_exact_unittest.cc @@ -29,7 +29,6 @@ void SetupComponent(int sample_rate_hz, EchoControlMobileImpl* echo_control_mobile) { echo_control_mobile->Initialize( sample_rate_hz > 16000 ? 16000 : sample_rate_hz, 1, 1); - echo_control_mobile->Enable(true); echo_control_mobile->set_routing_mode(routing_mode); echo_control_mobile->enable_comfort_noise(comfort_noise_enabled); } diff --git a/modules/audio_processing/echo_control_mobile_impl.cc b/modules/audio_processing/echo_control_mobile_impl.cc index 3655eb1e21..0495b395cc 100644 --- a/modules/audio_processing/echo_control_mobile_impl.cc +++ b/modules/audio_processing/echo_control_mobile_impl.cc @@ -106,10 +106,6 @@ EchoControlMobileImpl::~EchoControlMobileImpl() {} void EchoControlMobileImpl::ProcessRenderAudio( rtc::ArrayView packed_render_audio) { - if (!enabled_) { - return; - } - RTC_DCHECK(stream_properties_); size_t buffer_index = 0; @@ -158,10 +154,6 @@ size_t EchoControlMobileImpl::NumCancellersRequired( int EchoControlMobileImpl::ProcessCaptureAudio(AudioBuffer* audio, int stream_delay_ms) { - if (!enabled_) { - return AudioProcessing::kNoError; - } - RTC_DCHECK(stream_properties_); RTC_DCHECK_GE(160, audio->num_frames_per_band()); RTC_DCHECK_EQ(audio->num_channels(), stream_properties_->num_output_channels); @@ -202,33 +194,6 @@ int EchoControlMobileImpl::ProcessCaptureAudio(AudioBuffer* audio, return AudioProcessing::kNoError; } -int EchoControlMobileImpl::Enable(bool enable) { - // Ensure AEC and AECM are not both enabled. - RTC_DCHECK(stream_properties_); - - if (enable && - stream_properties_->sample_rate_hz > AudioProcessing::kSampleRate16kHz) { - return AudioProcessing::kBadSampleRateError; - } - - if (enable && !enabled_) { - enabled_ = enable; // Must be set before Initialize() is called. - - // TODO(peah): Simplify once the Enable function has been removed from - // the public APM API. - Initialize(stream_properties_->sample_rate_hz, - stream_properties_->num_reverse_channels, - stream_properties_->num_output_channels); - } else { - enabled_ = enable; - } - return AudioProcessing::kNoError; -} - -bool EchoControlMobileImpl::is_enabled() const { - return enabled_; -} - int EchoControlMobileImpl::set_routing_mode(RoutingMode mode) { if (MapSetting(mode) == -1) { return AudioProcessing::kBadParameterError; @@ -256,10 +221,6 @@ void EchoControlMobileImpl::Initialize(int sample_rate_hz, stream_properties_.reset(new StreamProperties( sample_rate_hz, num_reverse_channels, num_output_channels)); - if (!enabled_) { - return; - } - // AECM only supports 16 kHz or lower sample rates. RTC_DCHECK_LE(stream_properties_->sample_rate_hz, AudioProcessing::kSampleRate16kHz); diff --git a/modules/audio_processing/echo_control_mobile_impl.h b/modules/audio_processing/echo_control_mobile_impl.h index d5a8b0b445..e4437975dd 100644 --- a/modules/audio_processing/echo_control_mobile_impl.h +++ b/modules/audio_processing/echo_control_mobile_impl.h @@ -30,9 +30,6 @@ class EchoControlMobileImpl { ~EchoControlMobileImpl(); - int Enable(bool enable); - bool is_enabled() const; - // Recommended settings for particular audio routes. In general, the louder // the echo is expected to be, the higher this value should be set. The // preferred setting may vary from device to device. @@ -75,8 +72,6 @@ class EchoControlMobileImpl { int Configure(); - bool enabled_ = false; - RoutingMode routing_mode_; bool comfort_noise_enabled_; diff --git a/modules/audio_processing/echo_control_mobile_unittest.cc b/modules/audio_processing/echo_control_mobile_unittest.cc index 6b054d1aea..84e1c845ca 100644 --- a/modules/audio_processing/echo_control_mobile_unittest.cc +++ b/modules/audio_processing/echo_control_mobile_unittest.cc @@ -21,10 +21,6 @@ TEST(EchoControlMobileTest, InterfaceConfiguration) { EchoControlMobileImpl aecm; aecm.Initialize(AudioProcessing::kSampleRate16kHz, 2, 2); - // Turn AECM on - EXPECT_EQ(0, aecm.Enable(true)); - EXPECT_TRUE(aecm.is_enabled()); - // Toggle routing modes std::array routing_modes = { EchoControlMobileImpl::kQuietEarpieceOrHeadset, @@ -43,10 +39,6 @@ TEST(EchoControlMobileTest, InterfaceConfiguration) { EXPECT_FALSE(aecm.is_comfort_noise_enabled()); EXPECT_EQ(0, aecm.enable_comfort_noise(true)); EXPECT_TRUE(aecm.is_comfort_noise_enabled()); - - // Turn AECM off - EXPECT_EQ(0, aecm.Enable(false)); - EXPECT_FALSE(aecm.is_enabled()); } } // namespace webrtc