Remove APM internal usage of EchoCancellation
This CL:
- Changes EchoCancellationImpl to inherit privately from
EchoCancellation.
- Removes usage of AudioProcessing::echo_cancellation() inside most of
the audio processing module and unit tests.
- Default-enables metrics collection in AEC2.
This CL breaks audioproc_f backwards compatibility: It can no longer
use all recorded settings (drift compensation, suppression level), but
prints an error message when such settings are encountered.
Some code in audio_processing_unittest.cc still uses the old interface.
I'll handle that in a separate change, as it is not as straightforward
to preserve coverage.
Bug: webrtc:9535
Change-Id: Ia4d4b8d117ccbe516e5345c15d37298418590686
Reviewed-on: https://webrtc-review.googlesource.com/97603
Commit-Queue: Sam Zackrisson <saza@webrtc.org>
Reviewed-by: Gustaf Ullberg <gustaf@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#24724}diff --git a/modules/audio_processing/test/aec_dump_based_simulator.cc b/modules/audio_processing/test/aec_dump_based_simulator.cc
index 7038b9c..2d4f12f 100644
--- a/modules/audio_processing/test/aec_dump_based_simulator.cc
+++ b/modules/audio_processing/test/aec_dump_based_simulator.cc
@@ -15,6 +15,7 @@
#include "modules/audio_processing/test/protobuf_utils.h"
#include "rtc_base/checks.h"
+#include "rtc_base/logging.h"
#include "rtc_base/numerics/safe_conversions.h"
namespace webrtc {
@@ -146,15 +147,6 @@
}
}
- if (!settings_.stream_drift_samples) {
- if (msg.has_drift()) {
- ap_->echo_cancellation()->set_stream_drift_samples(msg.drift());
- }
- } else {
- ap_->echo_cancellation()->set_stream_drift_samples(
- *settings_.stream_drift_samples);
- }
-
if (!settings_.use_ts) {
if (msg.has_keypress()) {
ap_->set_stream_key_pressed(msg.keypress());
@@ -306,14 +298,11 @@
if (msg.has_aec_drift_compensation_enabled() ||
settings_.use_drift_compensation) {
- bool enable = settings_.use_drift_compensation
- ? *settings_.use_drift_compensation
- : msg.aec_drift_compensation_enabled();
- RTC_CHECK_EQ(AudioProcessing::kNoError,
- ap_->echo_cancellation()->enable_drift_compensation(enable));
- if (settings_.use_verbose_logging) {
- std::cout << " aec_drift_compensation_enabled: "
- << (enable ? "true" : "false") << std::endl;
+ if (settings_.use_drift_compensation
+ ? *settings_.use_drift_compensation
+ : msg.aec_drift_compensation_enabled()) {
+ RTC_LOG(LS_ERROR)
+ << "Ignoring deprecated setting: AEC2 drift compensation";
}
}
@@ -330,15 +319,20 @@
}
if (msg.has_aec_suppression_level() || settings_.aec_suppression_level) {
- int level = settings_.aec_suppression_level
- ? *settings_.aec_suppression_level
- : msg.aec_suppression_level();
- RTC_CHECK_EQ(
- AudioProcessing::kNoError,
- ap_->echo_cancellation()->set_suppression_level(
- static_cast<webrtc::EchoCancellation::SuppressionLevel>(level)));
- if (settings_.use_verbose_logging) {
- std::cout << " aec_suppression_level: " << level << std::endl;
+ auto level = static_cast<webrtc::EchoCancellation::SuppressionLevel>(
+ settings_.aec_suppression_level ? *settings_.aec_suppression_level
+ : msg.aec_suppression_level());
+ if (level ==
+ webrtc::EchoCancellation::SuppressionLevel::kLowSuppression) {
+ RTC_LOG(LS_ERROR)
+ << "Ignoring deprecated setting: AEC2 low suppression";
+ } else {
+ apm_config.echo_canceller.legacy_moderate_suppression_level =
+ (level ==
+ webrtc::EchoCancellation::SuppressionLevel::kModerateSuppression);
+ if (settings_.use_verbose_logging) {
+ std::cout << " aec_suppression_level: " << level << std::endl;
+ }
}
}
diff --git a/modules/audio_processing/test/audio_processing_simulator.cc b/modules/audio_processing/test/audio_processing_simulator.cc
index d5e8ea4..c49ddcb 100644
--- a/modules/audio_processing/test/audio_processing_simulator.cc
+++ b/modules/audio_processing/test/audio_processing_simulator.cc
@@ -705,6 +705,13 @@
settings_.pre_amplifier_gain_factor;
}
+ bool use_aec2 = settings_.use_aec && *settings_.use_aec;
+ bool use_aec3 = settings_.use_aec3 && *settings_.use_aec3;
+ bool use_aecm = settings_.use_aecm && *settings_.use_aecm;
+ if (use_aec2 || use_aec3 || use_aecm) {
+ apm_config.echo_canceller.enabled = true;
+ apm_config.echo_canceller.mobile_mode = use_aecm;
+ }
if (settings_.use_aec3 && *settings_.use_aec3) {
EchoCanceller3Config cfg;
if (settings_.aec3_settings_filename) {
@@ -713,6 +720,21 @@
}
echo_control_factory.reset(new EchoCanceller3Factory(cfg));
}
+ if (settings_.use_drift_compensation && *settings_.use_drift_compensation) {
+ RTC_LOG(LS_ERROR) << "Ignoring deprecated setting: AEC2 drift compensation";
+ }
+ if (settings_.aec_suppression_level) {
+ auto level = static_cast<webrtc::EchoCancellation::SuppressionLevel>(
+ *settings_.aec_suppression_level);
+ if (level == webrtc::EchoCancellation::SuppressionLevel::kLowSuppression) {
+ RTC_LOG(LS_ERROR) << "Ignoring deprecated setting: AEC2 low suppression";
+ } else {
+ apm_config.echo_canceller.legacy_moderate_suppression_level =
+ (level ==
+ webrtc::EchoCancellation::SuppressionLevel::kModerateSuppression);
+ }
+ }
+
if (settings_.use_hpf) {
apm_config.high_pass_filter.enabled = *settings_.use_hpf;
}
@@ -745,14 +767,6 @@
ap_->ApplyConfig(apm_config);
- if (settings_.use_aec) {
- RTC_CHECK_EQ(AudioProcessing::kNoError,
- ap_->echo_cancellation()->Enable(*settings_.use_aec));
- }
- if (settings_.use_aecm) {
- RTC_CHECK_EQ(AudioProcessing::kNoError,
- ap_->echo_control_mobile()->Enable(*settings_.use_aecm));
- }
if (settings_.use_agc) {
RTC_CHECK_EQ(AudioProcessing::kNoError,
ap_->gain_control()->Enable(*settings_.use_agc));
@@ -790,19 +804,6 @@
static_cast<webrtc::GainControl::Mode>(*settings_.agc_mode)));
}
- if (settings_.use_drift_compensation) {
- RTC_CHECK_EQ(AudioProcessing::kNoError,
- ap_->echo_cancellation()->enable_drift_compensation(
- *settings_.use_drift_compensation));
- }
-
- if (settings_.aec_suppression_level) {
- RTC_CHECK_EQ(AudioProcessing::kNoError,
- ap_->echo_cancellation()->set_suppression_level(
- static_cast<webrtc::EchoCancellation::SuppressionLevel>(
- *settings_.aec_suppression_level)));
- }
-
if (settings_.aecm_routing_mode) {
RTC_CHECK_EQ(AudioProcessing::kNoError,
ap_->echo_control_mobile()->set_routing_mode(
diff --git a/modules/audio_processing/test/audioproc_float_impl.cc b/modules/audio_processing/test/audioproc_float_impl.cc
index a465734..60c46f3 100644
--- a/modules/audio_processing/test/audioproc_float_impl.cc
+++ b/modules/audio_processing/test/audioproc_float_impl.cc
@@ -106,9 +106,6 @@
DEFINE_int(extended_filter,
kParameterNotSpecifiedValue,
"Activate (1) or deactivate(0) the AEC extended filter mode");
-DEFINE_int(drift_compensation,
- kParameterNotSpecifiedValue,
- "Activate (1) or deactivate(0) the drift compensation");
DEFINE_int(aec3,
kParameterNotSpecifiedValue,
"Activate (1) or deactivate(0) the experimental AEC mode AEC3");
@@ -259,8 +256,6 @@
&settings.aec_suppression_level);
SetSettingIfFlagSet(FLAG_delay_agnostic, &settings.use_delay_agnostic);
SetSettingIfFlagSet(FLAG_extended_filter, &settings.use_extended_filter);
- SetSettingIfFlagSet(FLAG_drift_compensation,
- &settings.use_drift_compensation);
SetSettingIfFlagSet(FLAG_refined_adaptive_filter,
&settings.use_refined_adaptive_filter);
@@ -361,11 +356,12 @@
*settings.reverse_output_num_channels <= 0,
"Error: --reverse_output_num_channels must be positive!\n");
- ReportConditionalErrorAndExit(
- settings.aec_suppression_level &&
- ((*settings.aec_suppression_level) < 0 ||
- (*settings.aec_suppression_level) > 2),
- "Error: --aec_suppression_level must be specified between 0 and 2.\n");
+ ReportConditionalErrorAndExit(settings.aec_suppression_level &&
+ ((*settings.aec_suppression_level) < 1 ||
+ (*settings.aec_suppression_level) > 2),
+ "Error: --aec_suppression_level must be "
+ "specified between 1 and 2. 0 is "
+ "deprecated.\n");
ReportConditionalErrorAndExit(
settings.aecm_routing_mode && ((*settings.aecm_routing_mode) < 0 ||
diff --git a/modules/audio_processing/test/debug_dump_replayer.cc b/modules/audio_processing/test/debug_dump_replayer.cc
index 6350c8c..94d03d3 100644
--- a/modules/audio_processing/test/debug_dump_replayer.cc
+++ b/modules/audio_processing/test/debug_dump_replayer.cc
@@ -123,7 +123,6 @@
RTC_CHECK_EQ(AudioProcessing::kNoError,
apm_->set_stream_delay_ms(msg.delay()));
- apm_->echo_cancellation()->set_stream_drift_samples(msg.drift());
if (msg.has_keypress()) {
apm_->set_stream_key_pressed(msg.keypress());
} else {
@@ -213,16 +212,11 @@
apm_config.echo_canceller.enabled = msg.aec_enabled() || msg.aecm_enabled();
apm_config.echo_canceller.mobile_mode = msg.aecm_enabled();
- RTC_CHECK(msg.has_aec_drift_compensation_enabled());
- RTC_CHECK_EQ(AudioProcessing::kNoError,
- apm_->echo_cancellation()->enable_drift_compensation(
- msg.aec_drift_compensation_enabled()));
-
RTC_CHECK(msg.has_aec_suppression_level());
- RTC_CHECK_EQ(AudioProcessing::kNoError,
- apm_->echo_cancellation()->set_suppression_level(
- static_cast<EchoCancellation::SuppressionLevel>(
- msg.aec_suppression_level())));
+ apm_config.echo_canceller.legacy_moderate_suppression_level =
+ static_cast<EchoCancellation::SuppressionLevel>(
+ msg.aec_suppression_level()) ==
+ EchoCancellation::SuppressionLevel::kModerateSuppression;
RTC_CHECK(msg.has_aecm_comfort_noise_enabled());
RTC_CHECK_EQ(AudioProcessing::kNoError,
diff --git a/modules/audio_processing/test/debug_dump_test.cc b/modules/audio_processing/test/debug_dump_test.cc
index 8b83c0f..509a904 100644
--- a/modules/audio_processing/test/debug_dump_test.cc
+++ b/modules/audio_processing/test/debug_dump_test.cc
@@ -359,12 +359,13 @@
TEST_F(DebugDumpTest, ToggleAec) {
Config config;
- DebugDumpGenerator generator(config, AudioProcessing::Config());
+ AudioProcessing::Config apm_config;
+ DebugDumpGenerator generator(config, apm_config);
generator.StartRecording();
generator.Process(100);
- EchoCancellation* aec = generator.apm()->echo_cancellation();
- EXPECT_EQ(AudioProcessing::kNoError, aec->Enable(!aec->is_enabled()));
+ apm_config.echo_canceller.enabled = true;
+ generator.apm()->ApplyConfig(apm_config);
generator.Process(100);
generator.StopRecording();
@@ -374,12 +375,13 @@
TEST_F(DebugDumpTest, ToggleDelayAgnosticAec) {
Config config;
config.Set<DelayAgnostic>(new DelayAgnostic(true));
- DebugDumpGenerator generator(config, AudioProcessing::Config());
+ AudioProcessing::Config apm_config;
+ DebugDumpGenerator generator(config, apm_config);
generator.StartRecording();
generator.Process(100);
- EchoCancellation* aec = generator.apm()->echo_cancellation();
- EXPECT_EQ(AudioProcessing::kNoError, aec->Enable(!aec->is_enabled()));
+ apm_config.echo_canceller.enabled = true;
+ generator.apm()->ApplyConfig(apm_config);
generator.Process(100);
generator.StopRecording();
@@ -548,15 +550,13 @@
AudioProcessing::Config apm_config;
apm_config.echo_canceller.enabled = true;
apm_config.echo_canceller.mobile_mode = false;
+ apm_config.echo_canceller.legacy_moderate_suppression_level = true;
DebugDumpGenerator generator(config, apm_config);
- EchoCancellation* aec = generator.apm()->echo_cancellation();
- EXPECT_EQ(AudioProcessing::kNoError,
- aec->set_suppression_level(EchoCancellation::kLowSuppression));
generator.StartRecording();
generator.Process(100);
- EXPECT_EQ(AudioProcessing::kNoError,
- aec->set_suppression_level(EchoCancellation::kHighSuppression));
+ apm_config.echo_canceller.legacy_moderate_suppression_level = false;
+ generator.apm()->ApplyConfig(apm_config);
generator.Process(100);
generator.StopRecording();
VerifyDebugDump(generator.dump_file_name());
diff --git a/modules/audio_processing/test/wav_based_simulator.cc b/modules/audio_processing/test/wav_based_simulator.cc
index 908a2e6..865ced7 100644
--- a/modules/audio_processing/test/wav_based_simulator.cc
+++ b/modules/audio_processing/test/wav_based_simulator.cc
@@ -79,9 +79,6 @@
ap_->set_stream_delay_ms(
settings_.stream_delay ? *settings_.stream_delay : 0));
}
-
- ap_->echo_cancellation()->set_stream_drift_samples(
- settings_.stream_drift_samples ? *settings_.stream_drift_samples : 0);
}
void WavBasedSimulator::PrepareReverseProcessStreamCall() {