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/audio_processing_unittest.cc b/modules/audio_processing/audio_processing_unittest.cc
index a894817..9401ebd 100644
--- a/modules/audio_processing/audio_processing_unittest.cc
+++ b/modules/audio_processing/audio_processing_unittest.cc
@@ -188,11 +188,7 @@
EXPECT_NOERR(ap->gain_control()->Enable(true));
#elif defined(WEBRTC_AUDIOPROC_FLOAT_PROFILE)
apm_config.echo_canceller.mobile_mode = false;
- EXPECT_NOERR(ap->echo_cancellation()->enable_drift_compensation(true));
- EXPECT_NOERR(ap->echo_cancellation()->enable_metrics(true));
- EXPECT_NOERR(ap->echo_cancellation()->enable_delay_logging(true));
- EXPECT_NOERR(ap->echo_cancellation()->set_suppression_level(
- EchoCancellation::SuppressionLevel::kModerateSuppression));
+ apm_config.echo_canceller.legacy_moderate_suppression_level = true;
EXPECT_NOERR(ap->gain_control()->set_mode(GainControl::kAdaptiveAnalog));
EXPECT_NOERR(ap->gain_control()->set_analog_level_limits(0, 255));
@@ -598,7 +594,6 @@
void ApmTest::ProcessWithDefaultStreamParameters(AudioFrame* frame) {
EXPECT_EQ(apm_->kNoError, apm_->set_stream_delay_ms(0));
- apm_->echo_cancellation()->set_stream_drift_samples(0);
EXPECT_EQ(apm_->kNoError,
apm_->gain_control()->set_stream_analog_level(127));
EXPECT_EQ(apm_->kNoError, apm_->ProcessStream(frame));
@@ -683,13 +678,8 @@
delete frame;
if (frame_count == 250) {
- int median;
- int std;
- float poor_fraction;
// Discard the first delay metrics to avoid convergence effects.
- EXPECT_EQ(apm_->kNoError,
- apm_->echo_cancellation()->GetDelayMetrics(&median, &std,
- &poor_fraction));
+ static_cast<void>(apm_->GetStatistics(true /* has_remote_tracks */));
}
}
@@ -712,12 +702,10 @@
expected_median - rtc::dchecked_cast<int>(96 / samples_per_ms), delay_min,
delay_max);
// Verify delay metrics.
- int median;
- int std;
- float poor_fraction;
- EXPECT_EQ(apm_->kNoError,
- apm_->echo_cancellation()->GetDelayMetrics(&median, &std,
- &poor_fraction));
+ AudioProcessingStats stats =
+ apm_->GetStatistics(true /* has_remote_tracks */);
+ ASSERT_TRUE(stats.delay_median_ms.has_value());
+ int32_t median = *stats.delay_median_ms;
EXPECT_GE(expected_median_high, median);
EXPECT_LE(expected_median_low, median);
}
@@ -739,19 +727,16 @@
ProcessStreamChooser(format));
// Other stream parameters set correctly.
- EXPECT_EQ(apm_->kNoError, apm_->echo_cancellation()->Enable(true));
- EXPECT_EQ(apm_->kNoError,
- apm_->echo_cancellation()->enable_drift_compensation(true));
+ AudioProcessing::Config apm_config = apm_->GetConfig();
+ apm_config.echo_canceller.enabled = true;
+ apm_config.echo_canceller.mobile_mode = false;
+ apm_->ApplyConfig(apm_config);
EXPECT_EQ(apm_->kNoError, apm_->set_stream_delay_ms(100));
- apm_->echo_cancellation()->set_stream_drift_samples(0);
EXPECT_EQ(apm_->kStreamParameterNotSetError,
ProcessStreamChooser(format));
EXPECT_EQ(apm_->kNoError, apm_->gain_control()->Enable(false));
- EXPECT_EQ(apm_->kNoError,
- apm_->echo_cancellation()->enable_drift_compensation(false));
// -- Missing delay --
- EXPECT_EQ(apm_->kNoError, apm_->echo_cancellation()->Enable(true));
EXPECT_EQ(apm_->kNoError, ProcessStreamChooser(format));
EXPECT_EQ(apm_->kStreamParameterNotSetError,
ProcessStreamChooser(format));
@@ -765,33 +750,11 @@
// Other stream parameters set correctly.
EXPECT_EQ(apm_->kNoError, apm_->gain_control()->Enable(true));
EXPECT_EQ(apm_->kNoError,
- apm_->echo_cancellation()->enable_drift_compensation(true));
- apm_->echo_cancellation()->set_stream_drift_samples(0);
- EXPECT_EQ(apm_->kNoError,
apm_->gain_control()->set_stream_analog_level(127));
EXPECT_EQ(apm_->kStreamParameterNotSetError,
ProcessStreamChooser(format));
EXPECT_EQ(apm_->kNoError, apm_->gain_control()->Enable(false));
- // -- Missing drift --
- EXPECT_EQ(apm_->kStreamParameterNotSetError,
- ProcessStreamChooser(format));
-
- // Resets after successful ProcessStream().
- EXPECT_EQ(apm_->kNoError, apm_->set_stream_delay_ms(100));
- apm_->echo_cancellation()->set_stream_drift_samples(0);
- EXPECT_EQ(apm_->kNoError, ProcessStreamChooser(format));
- EXPECT_EQ(apm_->kStreamParameterNotSetError,
- ProcessStreamChooser(format));
-
- // Other stream parameters set correctly.
- EXPECT_EQ(apm_->kNoError, apm_->gain_control()->Enable(true));
- EXPECT_EQ(apm_->kNoError, apm_->set_stream_delay_ms(100));
- EXPECT_EQ(apm_->kNoError,
- apm_->gain_control()->set_stream_analog_level(127));
- EXPECT_EQ(apm_->kStreamParameterNotSetError,
- ProcessStreamChooser(format));
-
// -- No stream parameters --
EXPECT_EQ(apm_->kNoError,
AnalyzeReverseStreamChooser(format));
@@ -800,7 +763,6 @@
// -- All there --
EXPECT_EQ(apm_->kNoError, apm_->set_stream_delay_ms(100));
- apm_->echo_cancellation()->set_stream_drift_samples(0);
EXPECT_EQ(apm_->kNoError,
apm_->gain_control()->set_stream_analog_level(127));
EXPECT_EQ(apm_->kNoError, ProcessStreamChooser(format));
@@ -924,79 +886,13 @@
}
}
-TEST_F(ApmTest, EchoCancellation) {
- EXPECT_EQ(apm_->kNoError,
- apm_->echo_cancellation()->enable_drift_compensation(true));
- EXPECT_TRUE(apm_->echo_cancellation()->is_drift_compensation_enabled());
- EXPECT_EQ(apm_->kNoError,
- apm_->echo_cancellation()->enable_drift_compensation(false));
- EXPECT_FALSE(apm_->echo_cancellation()->is_drift_compensation_enabled());
-
- EchoCancellation::SuppressionLevel level[] = {
- EchoCancellation::kLowSuppression,
- EchoCancellation::kModerateSuppression,
- EchoCancellation::kHighSuppression,
- };
- for (size_t i = 0; i < arraysize(level); i++) {
- EXPECT_EQ(apm_->kNoError,
- apm_->echo_cancellation()->set_suppression_level(level[i]));
- EXPECT_EQ(level[i],
- apm_->echo_cancellation()->suppression_level());
- }
-
- EchoCancellation::Metrics metrics;
- EXPECT_EQ(apm_->kNotEnabledError,
- apm_->echo_cancellation()->GetMetrics(&metrics));
-
- EXPECT_EQ(apm_->kNoError, apm_->echo_cancellation()->Enable(true));
- EXPECT_TRUE(apm_->echo_cancellation()->is_enabled());
-
- EXPECT_EQ(apm_->kNoError,
- apm_->echo_cancellation()->enable_metrics(true));
- EXPECT_TRUE(apm_->echo_cancellation()->are_metrics_enabled());
- EXPECT_EQ(apm_->kNoError,
- apm_->echo_cancellation()->enable_metrics(false));
- EXPECT_FALSE(apm_->echo_cancellation()->are_metrics_enabled());
-
- EXPECT_EQ(apm_->kNoError,
- apm_->echo_cancellation()->enable_delay_logging(true));
- EXPECT_TRUE(apm_->echo_cancellation()->is_delay_logging_enabled());
- EXPECT_EQ(apm_->kNoError,
- apm_->echo_cancellation()->enable_delay_logging(false));
- EXPECT_FALSE(apm_->echo_cancellation()->is_delay_logging_enabled());
-
- EXPECT_EQ(apm_->kNoError, apm_->echo_cancellation()->Enable(false));
- EXPECT_FALSE(apm_->echo_cancellation()->is_enabled());
-
- int median = 0;
- int std = 0;
- float poor_fraction = 0;
- EXPECT_EQ(apm_->kNotEnabledError, apm_->echo_cancellation()->GetDelayMetrics(
- &median, &std, &poor_fraction));
-
- EXPECT_EQ(apm_->kNoError, apm_->echo_cancellation()->Enable(true));
- EXPECT_TRUE(apm_->echo_cancellation()->is_enabled());
- EXPECT_EQ(apm_->kNoError, apm_->echo_cancellation()->Enable(false));
- EXPECT_FALSE(apm_->echo_cancellation()->is_enabled());
-
- EXPECT_EQ(apm_->kNoError, apm_->echo_cancellation()->Enable(true));
- EXPECT_TRUE(apm_->echo_cancellation()->is_enabled());
- EXPECT_TRUE(apm_->echo_cancellation()->aec_core() != NULL);
- EXPECT_EQ(apm_->kNoError, apm_->echo_cancellation()->Enable(false));
- EXPECT_FALSE(apm_->echo_cancellation()->is_enabled());
- EXPECT_FALSE(apm_->echo_cancellation()->aec_core() != NULL);
-}
-
TEST_F(ApmTest, DISABLED_EchoCancellationReportsCorrectDelays) {
// TODO(bjornv): Fix this test to work with DA-AEC.
// Enable AEC only.
- EXPECT_EQ(apm_->kNoError,
- apm_->echo_cancellation()->enable_drift_compensation(false));
- EXPECT_EQ(apm_->kNoError,
- apm_->echo_cancellation()->enable_metrics(false));
- EXPECT_EQ(apm_->kNoError,
- apm_->echo_cancellation()->enable_delay_logging(true));
- EXPECT_EQ(apm_->kNoError, apm_->echo_cancellation()->Enable(true));
+ AudioProcessing::Config apm_config = apm_->GetConfig();
+ apm_config.echo_canceller.enabled = true;
+ apm_config.echo_canceller.mobile_mode = false;
+ apm_->ApplyConfig(apm_config);
Config config;
config.Set<DelayAgnostic>(new DelayAgnostic(false));
apm_->SetExtraOptions(config);
@@ -1464,8 +1360,9 @@
}
TEST_F(ApmTest, AllProcessingDisabledByDefault) {
- EXPECT_FALSE(apm_->echo_cancellation()->is_enabled());
- EXPECT_FALSE(apm_->echo_control_mobile()->is_enabled());
+ AudioProcessing::Config config = apm_->GetConfig();
+ EXPECT_FALSE(config.echo_canceller.enabled);
+ EXPECT_FALSE(config.high_pass_filter.enabled);
EXPECT_FALSE(apm_->gain_control()->is_enabled());
EXPECT_FALSE(apm_->high_pass_filter()->is_enabled());
EXPECT_FALSE(apm_->level_estimator()->is_enabled());
@@ -1548,7 +1445,6 @@
frame_->vad_activity_ = AudioFrame::kVadUnknown;
ASSERT_EQ(kNoErr, apm_->set_stream_delay_ms(0));
- apm_->echo_cancellation()->set_stream_drift_samples(0);
ASSERT_EQ(kNoErr,
apm_->gain_control()->set_stream_analog_level(analog_level));
ASSERT_EQ(kNoErr, apm_->ProcessStream(frame_));
@@ -1608,7 +1504,10 @@
// first few frames of data being unaffected by the AEC.
// TODO(andrew): This test, and the one below, rely rather tenuously on the
// behavior of the AEC. Think of something more robust.
- EXPECT_EQ(apm_->kNoError, apm_->echo_cancellation()->Enable(true));
+ AudioProcessing::Config apm_config = apm_->GetConfig();
+ apm_config.echo_canceller.enabled = true;
+ apm_config.echo_canceller.mobile_mode = false;
+ apm_->ApplyConfig(apm_config);
// Make sure we have extended filter enabled. This makes sure nothing is
// touched until we have a farend frame.
Config config;
@@ -1617,10 +1516,8 @@
SetFrameTo(frame_, 1000);
frame_copy.CopyFrom(*frame_);
EXPECT_EQ(apm_->kNoError, apm_->set_stream_delay_ms(0));
- apm_->echo_cancellation()->set_stream_drift_samples(0);
EXPECT_EQ(apm_->kNoError, apm_->ProcessStream(frame_));
EXPECT_EQ(apm_->kNoError, apm_->set_stream_delay_ms(0));
- apm_->echo_cancellation()->set_stream_drift_samples(0);
EXPECT_EQ(apm_->kNoError, apm_->ProcessStream(frame_));
EXPECT_TRUE(FrameDataAreEqual(*frame_, frame_copy));
@@ -1632,7 +1529,6 @@
SetFrameTo(frame_, 1000);
frame_copy.CopyFrom(*frame_);
EXPECT_EQ(apm_->kNoError, apm_->set_stream_delay_ms(0));
- apm_->echo_cancellation()->set_stream_drift_samples(0);
EXPECT_EQ(apm_->kNoError, apm_->ProcessStream(frame_));
EXPECT_FALSE(FrameDataAreEqual(*frame_, frame_copy));
}
@@ -1704,7 +1600,6 @@
EXPECT_NOERR(apm_->gain_control()->set_stream_analog_level(msg.level()));
EXPECT_NOERR(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 {
@@ -1925,8 +1820,6 @@
EXPECT_NOERR(apm_->set_stream_delay_ms(0));
EXPECT_NOERR(fapm->set_stream_delay_ms(0));
- apm_->echo_cancellation()->set_stream_drift_samples(0);
- fapm->echo_cancellation()->set_stream_drift_samples(0);
EXPECT_NOERR(apm_->gain_control()->set_stream_analog_level(analog_level));
EXPECT_NOERR(fapm->gain_control()->set_stream_analog_level(analog_level));
@@ -1963,8 +1856,6 @@
analog_level = fapm->gain_control()->stream_analog_level();
EXPECT_EQ(apm_->gain_control()->stream_analog_level(),
fapm->gain_control()->stream_analog_level());
- EXPECT_EQ(apm_->echo_cancellation()->stream_has_echo(),
- fapm->echo_cancellation()->stream_has_echo());
EXPECT_NEAR(apm_->noise_suppression()->speech_probability(),
fapm->noise_suppression()->speech_probability(),
0.01);
@@ -2064,7 +1955,6 @@
frame_->vad_activity_ = AudioFrame::kVadUnknown;
EXPECT_EQ(apm_->kNoError, apm_->set_stream_delay_ms(0));
- apm_->echo_cancellation()->set_stream_drift_samples(0);
EXPECT_EQ(apm_->kNoError,
apm_->gain_control()->set_stream_analog_level(analog_level));
@@ -2435,7 +2325,6 @@
processing_config.reverse_output_stream(), rev_out_cb.channels()));
EXPECT_NOERR(ap->set_stream_delay_ms(0));
- ap->echo_cancellation()->set_stream_drift_samples(0);
EXPECT_NOERR(ap->gain_control()->set_stream_analog_level(analog_level));
EXPECT_NOERR(ap->ProcessStream(
@@ -2894,25 +2783,17 @@
}
// Disable all components except for an AEC and the residual echo detector.
- AudioProcessing::Config config;
- config.residual_echo_detector.enabled = true;
- config.high_pass_filter.enabled = false;
- config.gain_controller2.enabled = false;
- apm->ApplyConfig(config);
+ AudioProcessing::Config apm_config;
+ apm_config.residual_echo_detector.enabled = true;
+ apm_config.high_pass_filter.enabled = false;
+ apm_config.gain_controller2.enabled = false;
+ apm_config.echo_canceller.enabled = true;
+ apm_config.echo_canceller.mobile_mode = !use_AEC2;
+ apm->ApplyConfig(apm_config);
EXPECT_EQ(apm->gain_control()->Enable(false), 0);
EXPECT_EQ(apm->level_estimator()->Enable(false), 0);
EXPECT_EQ(apm->noise_suppression()->Enable(false), 0);
EXPECT_EQ(apm->voice_detection()->Enable(false), 0);
-
- if (use_AEC2) {
- EXPECT_EQ(apm->echo_control_mobile()->Enable(false), 0);
- EXPECT_EQ(apm->echo_cancellation()->enable_metrics(true), 0);
- EXPECT_EQ(apm->echo_cancellation()->enable_delay_logging(true), 0);
- EXPECT_EQ(apm->echo_cancellation()->Enable(true), 0);
- } else {
- EXPECT_EQ(apm->echo_cancellation()->Enable(false), 0);
- EXPECT_EQ(apm->echo_control_mobile()->Enable(true), 0);
- }
return apm;
}