Remove apm_helpers, consolidate audio config in WebRtcVoiceEngine
Refactorings to the audio processing module has, piece by piece,
decreased the workload of the apm_helpers helpers. It has come to a
point where it seems more reliable to consolidate what little is left
into the WebRtcVoiceEngine itself.
Bug: webrtc:9878
Change-Id: I6d983ace8e7ccb1b99d95178cf72608a657c7506
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/157443
Reviewed-by: Niels Moller <nisse@webrtc.org>
Reviewed-by: Alessio Bazzica <alessiob@webrtc.org>
Commit-Queue: Sam Zackrisson <saza@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#29553}
diff --git a/media/BUILD.gn b/media/BUILD.gn
index 371f6cd..4535334 100644
--- a/media/BUILD.gn
+++ b/media/BUILD.gn
@@ -319,8 +319,6 @@
sources = [
"engine/adm_helpers.cc",
"engine/adm_helpers.h",
- "engine/apm_helpers.cc",
- "engine/apm_helpers.h",
"engine/null_webrtc_video_engine.h",
"engine/payload_type_mapper.cc",
"engine/payload_type_mapper.h",
@@ -590,7 +588,6 @@
"base/video_adapter_unittest.cc",
"base/video_broadcaster_unittest.cc",
"base/video_common_unittest.cc",
- "engine/apm_helpers_unittest.cc",
"engine/encoder_simulcast_proxy_unittest.cc",
"engine/internal_decoder_factory_unittest.cc",
"engine/multiplex_codec_factory_unittest.cc",
diff --git a/media/engine/apm_helpers.cc b/media/engine/apm_helpers.cc
deleted file mode 100644
index 31bdd4b..0000000
--- a/media/engine/apm_helpers.cc
+++ /dev/null
@@ -1,51 +0,0 @@
-/*
- * Copyright (c) 2017 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.
- */
-
-#include "media/engine/apm_helpers.h"
-
-#include "modules/audio_processing/include/audio_processing.h"
-#include "rtc_base/checks.h"
-#include "rtc_base/logging.h"
-
-namespace webrtc {
-namespace apm_helpers {
-
-void Init(AudioProcessing* apm) {
- RTC_DCHECK(apm);
-
- constexpr int kMinVolumeLevel = 0;
- constexpr int kMaxVolumeLevel = 255;
-
- AudioProcessing::Config config = apm->GetConfig();
-#if defined(WEBRTC_IOS) || defined(WEBRTC_ANDROID)
- config.gain_controller1.mode = config.gain_controller1.kFixedDigital;
-#else
- config.gain_controller1.mode = config.gain_controller1.kAdaptiveAnalog;
-#endif
- RTC_LOG(LS_INFO) << "Setting AGC mode to " << config.gain_controller1.mode;
- // This is the initialization which used to happen in VoEBase::Init(), but
- // which is not covered by the WVoE::ApplyOptions().
- config.gain_controller1.analog_level_minimum = kMinVolumeLevel;
- config.gain_controller1.analog_level_maximum = kMaxVolumeLevel;
- apm->ApplyConfig(config);
-}
-
-void SetEcStatus(AudioProcessing* apm, bool enable, EcModes mode) {
- RTC_DCHECK(apm);
- RTC_DCHECK(mode == kEcConference || mode == kEcAecm) << "mode: " << mode;
- AudioProcessing::Config apm_config = apm->GetConfig();
- apm_config.echo_canceller.enabled = enable;
- apm_config.echo_canceller.mobile_mode = (mode == kEcAecm);
- apm_config.echo_canceller.legacy_moderate_suppression_level = false;
- apm->ApplyConfig(apm_config);
- RTC_LOG(LS_INFO) << "Echo control set to " << enable << " with mode " << mode;
-}
-} // namespace apm_helpers
-} // namespace webrtc
diff --git a/media/engine/apm_helpers.h b/media/engine/apm_helpers.h
deleted file mode 100644
index 7bedda7..0000000
--- a/media/engine/apm_helpers.h
+++ /dev/null
@@ -1,35 +0,0 @@
-/*
- * Copyright (c) 2017 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 MEDIA_ENGINE_APM_HELPERS_H_
-#define MEDIA_ENGINE_APM_HELPERS_H_
-
-#include <cstdint>
-
-namespace webrtc {
-
-class AudioProcessing;
-
-enum EcModes {
- kEcConference, // Conferencing default (aggressive AEC).
- kEcAecm, // AEC mobile.
-};
-
-namespace apm_helpers {
-
-void Init(AudioProcessing* apm);
-void SetEcStatus(AudioProcessing* apm, bool enable, EcModes mode);
-void SetEcMetricsStatus(AudioProcessing* apm, bool enable);
-void SetAecmMode(AudioProcessing* apm, bool enable_cng);
-
-} // namespace apm_helpers
-} // namespace webrtc
-
-#endif // MEDIA_ENGINE_APM_HELPERS_H_
diff --git a/media/engine/apm_helpers_unittest.cc b/media/engine/apm_helpers_unittest.cc
deleted file mode 100644
index dac24b3..0000000
--- a/media/engine/apm_helpers_unittest.cc
+++ /dev/null
@@ -1,73 +0,0 @@
-/*
- * Copyright (c) 2017 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.
- */
-
-#include "media/engine/apm_helpers.h"
-
-#include "api/scoped_refptr.h"
-#include "modules/audio_processing/include/audio_processing.h"
-#include "test/gmock.h"
-#include "test/gtest.h"
-
-namespace webrtc {
-namespace {
-
-struct TestHelper {
- TestHelper() {
- // This replicates the conditions from voe_auto_test.
- Config config;
- config.Set<ExperimentalAgc>(new ExperimentalAgc(false));
- apm_ = rtc::scoped_refptr<AudioProcessing>(
- AudioProcessingBuilder().Create(config));
- apm_helpers::Init(apm());
- }
-
- AudioProcessing* apm() { return apm_.get(); }
-
- const AudioProcessing* apm() const { return apm_.get(); }
-
- private:
- rtc::scoped_refptr<AudioProcessing> apm_;
-};
-} // namespace
-
-TEST(ApmHelpersTest, EcStatus_DefaultMode) {
- TestHelper helper;
- webrtc::AudioProcessing::Config config = helper.apm()->GetConfig();
- EXPECT_FALSE(config.echo_canceller.enabled);
-}
-
-TEST(ApmHelpersTest, EcStatus_EnableDisable) {
- TestHelper helper;
- webrtc::AudioProcessing::Config config;
-
- apm_helpers::SetEcStatus(helper.apm(), true, kEcAecm);
- config = helper.apm()->GetConfig();
- EXPECT_TRUE(config.echo_canceller.enabled);
- EXPECT_TRUE(config.echo_canceller.mobile_mode);
-
- apm_helpers::SetEcStatus(helper.apm(), false, kEcAecm);
- config = helper.apm()->GetConfig();
- EXPECT_FALSE(config.echo_canceller.enabled);
-
- apm_helpers::SetEcStatus(helper.apm(), true, kEcConference);
- config = helper.apm()->GetConfig();
- EXPECT_TRUE(config.echo_canceller.enabled);
- EXPECT_FALSE(config.echo_canceller.mobile_mode);
-
- apm_helpers::SetEcStatus(helper.apm(), false, kEcConference);
- config = helper.apm()->GetConfig();
- EXPECT_FALSE(config.echo_canceller.enabled);
-
- apm_helpers::SetEcStatus(helper.apm(), true, kEcAecm);
- config = helper.apm()->GetConfig();
- EXPECT_TRUE(config.echo_canceller.enabled);
- EXPECT_TRUE(config.echo_canceller.mobile_mode);
-}
-} // namespace webrtc
diff --git a/media/engine/webrtc_voice_engine.cc b/media/engine/webrtc_voice_engine.cc
index 08fe73f..ee8e5f0 100644
--- a/media/engine/webrtc_voice_engine.cc
+++ b/media/engine/webrtc_voice_engine.cc
@@ -26,7 +26,6 @@
#include "media/base/media_constants.h"
#include "media/base/stream_params.h"
#include "media/engine/adm_helpers.h"
-#include "media/engine/apm_helpers.h"
#include "media/engine/payload_type_mapper.h"
#include "media/engine/webrtc_media_engine.h"
#include "modules/audio_device/audio_device_impl.h"
@@ -256,7 +255,6 @@
#endif // WEBRTC_INCLUDE_INTERNAL_AUDIO_DEVICE
RTC_CHECK(adm());
webrtc::adm_helpers::Init(adm());
- webrtc::apm_helpers::Init(apm());
// Set up AudioState.
{
@@ -320,8 +318,8 @@
AudioOptions options = options_in; // The options are modified below.
// Set and adjust echo canceller options.
- // kEcConference is AEC with high suppression.
- webrtc::EcModes ec_mode = webrtc::kEcConference;
+ // Use desktop AEC by default, when not using hardware AEC.
+ bool use_mobile_software_aec = false;
#if defined(WEBRTC_IOS)
if (options.ios_force_software_aec_HACK &&
@@ -337,7 +335,7 @@
RTC_LOG(LS_INFO) << "Always disable AEC on iOS. Use built-in instead.";
}
#elif defined(WEBRTC_ANDROID)
- ec_mode = webrtc::kEcAecm;
+ use_mobile_software_aec = true;
#endif
// Set and adjust noise suppressor options.
@@ -401,8 +399,6 @@
<< "Disabling EC since built-in EC will be used instead";
}
}
- webrtc::apm_helpers::SetEcStatus(apm(), *options.echo_cancellation,
- ec_mode);
}
if (options.auto_gain_control) {
@@ -475,10 +471,26 @@
webrtc::AudioProcessing::Config apm_config = apm()->GetConfig();
+ if (options.echo_cancellation) {
+ apm_config.echo_canceller.enabled = *options.echo_cancellation;
+ apm_config.echo_canceller.mobile_mode = use_mobile_software_aec;
+ apm_config.echo_canceller.legacy_moderate_suppression_level = false;
+ }
+
if (options.auto_gain_control) {
const bool enabled = *options.auto_gain_control;
apm_config.gain_controller1.enabled = enabled;
- RTC_LOG(LS_INFO) << "Setting AGC to " << enabled;
+#if defined(WEBRTC_IOS) || defined(WEBRTC_ANDROID)
+ apm_config.gain_controller1.mode =
+ apm_config.gain_controller1.kFixedDigital;
+#else
+ apm_config.gain_controller1.mode =
+ apm_config.gain_controller1.kAdaptiveAnalog;
+#endif
+ constexpr int kMinVolumeLevel = 0;
+ constexpr int kMaxVolumeLevel = 255;
+ apm_config.gain_controller1.analog_level_minimum = kMinVolumeLevel;
+ apm_config.gain_controller1.analog_level_maximum = kMaxVolumeLevel;
}
if (options.tx_agc_target_dbov) {
apm_config.gain_controller1.target_level_dbfs = *options.tx_agc_target_dbov;
diff --git a/media/engine/webrtc_voice_engine.h b/media/engine/webrtc_voice_engine.h
index 8067ef0..add587f 100644
--- a/media/engine/webrtc_voice_engine.h
+++ b/media/engine/webrtc_voice_engine.h
@@ -24,7 +24,6 @@
#include "call/call.h"
#include "media/base/media_engine.h"
#include "media/base/rtp_utils.h"
-#include "media/engine/apm_helpers.h"
#include "rtc_base/buffer.h"
#include "rtc_base/constructor_magic.h"
#include "rtc_base/network_route.h"
diff --git a/media/engine/webrtc_voice_engine_unittest.cc b/media/engine/webrtc_voice_engine_unittest.cc
index bcffa40..fd054df 100644
--- a/media/engine/webrtc_voice_engine_unittest.cc
+++ b/media/engine/webrtc_voice_engine_unittest.cc
@@ -204,7 +204,7 @@
recv_parameters_.codecs.push_back(kPcmuCodec);
// Default Options.
- EXPECT_TRUE(IsEchoCancellationEnabled());
+ VerifyEchoCancellationSettings(/*enabled=*/true);
EXPECT_TRUE(IsHighPassFilterEnabled());
EXPECT_TRUE(IsTypingDetectionEnabled());
EXPECT_TRUE(apm_config_.noise_suppression.enabled);
@@ -755,8 +755,16 @@
EXPECT_TRUE(apm_config_.gain_controller1.enable_limiter);
}
- bool IsEchoCancellationEnabled() {
- return apm_config_.echo_canceller.enabled;
+ void VerifyEchoCancellationSettings(bool enabled) {
+ constexpr bool kDefaultUseAecm =
+#if defined(WEBRTC_ANDROID)
+ true;
+#else
+ false;
+#endif
+ EXPECT_EQ(apm_config_.echo_canceller.enabled, enabled);
+ EXPECT_EQ(apm_config_.echo_canceller.mobile_mode, kDefaultUseAecm);
+ EXPECT_FALSE(apm_config_.echo_canceller.use_legacy_aec);
}
bool IsHighPassFilterEnabled() {
@@ -2832,7 +2840,7 @@
// Nothing set in AudioOptions, so everything should be as default.
send_parameters_.options = cricket::AudioOptions();
SetSendParameters(send_parameters_);
- EXPECT_TRUE(IsEchoCancellationEnabled());
+ VerifyEchoCancellationSettings(/*enabled=*/true);
EXPECT_TRUE(IsHighPassFilterEnabled());
EXPECT_TRUE(IsTypingDetectionEnabled());
EXPECT_EQ(200u, GetRecvStreamConfig(kSsrcY).jitter_buffer_max_packets);
@@ -2856,18 +2864,18 @@
// Turn echo cancellation off
send_parameters_.options.echo_cancellation = false;
SetSendParameters(send_parameters_);
- EXPECT_FALSE(IsEchoCancellationEnabled());
+ VerifyEchoCancellationSettings(/*enabled=*/false);
// Turn echo cancellation back on, with settings, and make sure
// nothing else changed.
send_parameters_.options.echo_cancellation = true;
SetSendParameters(send_parameters_);
- EXPECT_TRUE(IsEchoCancellationEnabled());
+ VerifyEchoCancellationSettings(/*enabled=*/true);
// Turn off echo cancellation and delay agnostic aec.
send_parameters_.options.echo_cancellation = false;
SetSendParameters(send_parameters_);
- EXPECT_FALSE(IsEchoCancellationEnabled());
+ VerifyEchoCancellationSettings(/*enabled=*/false);
// Restore AEC to be on to work with the following tests.
send_parameters_.options.echo_cancellation = true;
@@ -2876,13 +2884,13 @@
// Turn off AGC
send_parameters_.options.auto_gain_control = false;
SetSendParameters(send_parameters_);
- EXPECT_TRUE(IsEchoCancellationEnabled());
+ VerifyEchoCancellationSettings(/*enabled=*/true);
EXPECT_FALSE(apm_config_.gain_controller1.enabled);
// Turn AGC back on
send_parameters_.options.auto_gain_control = true;
SetSendParameters(send_parameters_);
- EXPECT_TRUE(IsEchoCancellationEnabled());
+ VerifyEchoCancellationSettings(/*enabled=*/true);
EXPECT_TRUE(apm_config_.gain_controller1.enabled);
// Turn off other options.
@@ -2890,7 +2898,7 @@
send_parameters_.options.highpass_filter = false;
send_parameters_.options.stereo_swapping = true;
SetSendParameters(send_parameters_);
- EXPECT_TRUE(IsEchoCancellationEnabled());
+ VerifyEchoCancellationSettings(/*enabled=*/true);
EXPECT_FALSE(IsHighPassFilterEnabled());
EXPECT_TRUE(apm_config_.gain_controller1.enabled);
EXPECT_FALSE(apm_config_.noise_suppression.enabled);
@@ -2898,7 +2906,7 @@
// Set options again to ensure it has no impact.
SetSendParameters(send_parameters_);
- EXPECT_TRUE(IsEchoCancellationEnabled());
+ VerifyEchoCancellationSettings(/*enabled=*/true);
EXPECT_TRUE(apm_config_.gain_controller1.enabled);
EXPECT_FALSE(apm_config_.noise_suppression.enabled);
EXPECT_EQ(apm_config_.noise_suppression.level, kDefaultNsLevel);
@@ -2947,13 +2955,13 @@
parameters_options_all.options.auto_gain_control = true;
parameters_options_all.options.noise_suppression = true;
EXPECT_TRUE(channel1->SetSendParameters(parameters_options_all));
- EXPECT_TRUE(IsEchoCancellationEnabled());
+ VerifyEchoCancellationSettings(/*enabled=*/true);
VerifyGainControlEnabledCorrectly();
EXPECT_TRUE(apm_config_.noise_suppression.enabled);
EXPECT_EQ(apm_config_.noise_suppression.level, kDefaultNsLevel);
EXPECT_EQ(parameters_options_all.options, channel1->options());
EXPECT_TRUE(channel2->SetSendParameters(parameters_options_all));
- EXPECT_TRUE(IsEchoCancellationEnabled());
+ VerifyEchoCancellationSettings(/*enabled=*/true);
VerifyGainControlEnabledCorrectly();
EXPECT_EQ(parameters_options_all.options, channel2->options());
@@ -2961,7 +2969,7 @@
cricket::AudioSendParameters parameters_options_no_ns = send_parameters_;
parameters_options_no_ns.options.noise_suppression = false;
EXPECT_TRUE(channel1->SetSendParameters(parameters_options_no_ns));
- EXPECT_TRUE(IsEchoCancellationEnabled());
+ VerifyEchoCancellationSettings(/*enabled=*/true);
EXPECT_FALSE(apm_config_.noise_suppression.enabled);
EXPECT_EQ(apm_config_.noise_suppression.level, kDefaultNsLevel);
VerifyGainControlEnabledCorrectly();
@@ -2975,7 +2983,7 @@
cricket::AudioSendParameters parameters_options_no_agc = send_parameters_;
parameters_options_no_agc.options.auto_gain_control = false;
EXPECT_TRUE(channel2->SetSendParameters(parameters_options_no_agc));
- EXPECT_TRUE(IsEchoCancellationEnabled());
+ VerifyEchoCancellationSettings(/*enabled=*/true);
EXPECT_FALSE(apm_config_.gain_controller1.enabled);
EXPECT_TRUE(apm_config_.noise_suppression.enabled);
EXPECT_EQ(apm_config_.noise_suppression.level, kDefaultNsLevel);
@@ -2985,19 +2993,19 @@
EXPECT_EQ(expected_options, channel2->options());
EXPECT_TRUE(channel_->SetSendParameters(parameters_options_all));
- EXPECT_TRUE(IsEchoCancellationEnabled());
+ VerifyEchoCancellationSettings(/*enabled=*/true);
VerifyGainControlEnabledCorrectly();
EXPECT_TRUE(apm_config_.noise_suppression.enabled);
EXPECT_EQ(apm_config_.noise_suppression.level, kDefaultNsLevel);
channel1->SetSend(true);
- EXPECT_TRUE(IsEchoCancellationEnabled());
+ VerifyEchoCancellationSettings(/*enabled=*/true);
VerifyGainControlEnabledCorrectly();
EXPECT_FALSE(apm_config_.noise_suppression.enabled);
EXPECT_EQ(apm_config_.noise_suppression.level, kDefaultNsLevel);
channel2->SetSend(true);
- EXPECT_TRUE(IsEchoCancellationEnabled());
+ VerifyEchoCancellationSettings(/*enabled=*/true);
EXPECT_FALSE(apm_config_.gain_controller1.enabled);
EXPECT_TRUE(apm_config_.noise_suppression.enabled);
EXPECT_EQ(apm_config_.noise_suppression.level, kDefaultNsLevel);
@@ -3008,7 +3016,7 @@
parameters_options_no_agc_nor_ns.options.auto_gain_control = false;
parameters_options_no_agc_nor_ns.options.noise_suppression = false;
EXPECT_TRUE(channel2->SetSendParameters(parameters_options_no_agc_nor_ns));
- EXPECT_TRUE(IsEchoCancellationEnabled());
+ VerifyEchoCancellationSettings(/*enabled=*/true);
EXPECT_FALSE(apm_config_.gain_controller1.enabled);
EXPECT_FALSE(apm_config_.noise_suppression.enabled);
EXPECT_EQ(apm_config_.noise_suppression.level, kDefaultNsLevel);
