Add one-stop-shop for built-in AEC toggling in APM
This does not change what AEC functionality is available.
However, a client that only uses this interface - and not the submodule
pointer accessors - gets simpler code, and is guaranteed not to run any
two AECs in tandem.
The submodule interface EchoControlMobile is being deprecated in
https://webrtc-review.googlesource.com/c/src/+/89392
Bug: webrtc:9535
Change-Id: Id9326074e566be6d8768010fc421c457beff402c
Reviewed-on: https://webrtc-review.googlesource.com/89386
Commit-Queue: Sam Zackrisson <saza@webrtc.org>
Reviewed-by: Per Åhgren <peah@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#24066}diff --git a/modules/audio_processing/audio_processing_impl.cc b/modules/audio_processing/audio_processing_impl.cc
index 7e5955a..e229b45 100644
--- a/modules/audio_processing/audio_processing_impl.cc
+++ b/modules/audio_processing/audio_processing_impl.cc
@@ -385,8 +385,10 @@
rtc::CritScope cs_capture(&crit_capture_);
// Mark Echo Controller enabled if a factory is injected.
- capture_nonlocked_.echo_controller_enabled =
- static_cast<bool>(echo_control_factory_);
+ if (echo_control_factory_) {
+ config_.echo_cancellation.enabled = true;
+ capture_nonlocked_.echo_controller_enabled = true;
+ }
public_submodules_->echo_cancellation.reset(
new EchoCancellationImpl(&crit_render_, &crit_capture_));
@@ -491,6 +493,15 @@
}
int AudioProcessingImpl::InitializeLocked() {
+ // Ensure at most one built-in AEC is active, by arbitrarily preferring AEC2.
+ if (public_submodules_->echo_cancellation->is_enabled()) {
+ echo_control_mobile()->Enable(false);
+ config_.echo_cancellation.enabled = true;
+ config_.echo_cancellation.mobile_mode = false;
+ } else if (public_submodules_->echo_control_mobile->is_enabled()) {
+ config_.echo_cancellation.enabled = true;
+ config_.echo_cancellation.mobile_mode = true;
+ }
UpdateActiveSubmoduleStates();
const int render_audiobuffer_num_output_frames =
@@ -666,6 +677,13 @@
rtc::CritScope cs_render(&crit_render_);
rtc::CritScope cs_capture(&crit_capture_);
+ capture_nonlocked_.echo_controller_enabled =
+ config_.echo_cancellation.enabled && private_submodules_->echo_controller;
+ echo_cancellation()->Enable(config_.echo_cancellation.enabled &&
+ !config_.echo_cancellation.mobile_mode);
+ echo_control_mobile()->Enable(config_.echo_cancellation.enabled &&
+ config_.echo_cancellation.mobile_mode);
+
InitializeLowCutFilter();
RTC_LOG(LS_INFO) << "Highpass filter activated: "
@@ -1169,8 +1187,8 @@
HandleCaptureRuntimeSettings();
// Ensure that not both the AEC and AECM are active at the same time.
- // TODO(peah): Simplify once the public API Enable functions for these
- // are moved to APM.
+ // TODO(bugs.webrtc.org/9535): Simplify once the public API Enable functions
+ // for these are moved to APM.
RTC_DCHECK(!(public_submodules_->echo_cancellation->is_enabled() &&
public_submodules_->echo_control_mobile->is_enabled()));
@@ -1197,7 +1215,7 @@
levels.peak, 1, RmsLevel::kMinLevelDb, 64);
}
- if (private_submodules_->echo_controller) {
+ if (capture_nonlocked_.echo_controller_enabled) {
// Detect and flag any change in the analog gain.
int analog_mic_level = gain_control()->stream_analog_level();
capture_.echo_path_gain_change =
@@ -1221,7 +1239,7 @@
capture_buffer->SplitIntoFrequencyBands();
}
- if (private_submodules_->echo_controller) {
+ if (capture_nonlocked_.echo_controller_enabled) {
// Force down-mixing of the number of channels after the detection of
// capture signal saturation.
// TODO(peah): Look into ensuring that this kind of tampering with the
@@ -1231,7 +1249,7 @@
// TODO(peah): Move the AEC3 low-cut filter to this place.
if (private_submodules_->low_cut_filter &&
- !private_submodules_->echo_controller) {
+ !capture_nonlocked_.echo_controller_enabled) {
private_submodules_->low_cut_filter->Process(capture_buffer);
}
RETURN_ON_ERR(
@@ -1241,11 +1259,11 @@
// Ensure that the stream delay was set before the call to the
// AEC ProcessCaptureAudio function.
if (public_submodules_->echo_cancellation->is_enabled() &&
- !private_submodules_->echo_controller && !was_stream_delay_set()) {
+ !was_stream_delay_set()) {
return AudioProcessing::kStreamParameterNotSetError;
}
- if (private_submodules_->echo_controller) {
+ if (capture_nonlocked_.echo_controller_enabled) {
data_dumper_->DumpRaw("stream_delay", stream_delay_ms());
if (was_stream_delay_set()) {
@@ -1285,8 +1303,7 @@
return AudioProcessing::kStreamParameterNotSetError;
}
- if (!(private_submodules_->echo_controller ||
- public_submodules_->echo_cancellation->is_enabled())) {
+ if (public_submodules_->echo_control_mobile->is_enabled()) {
RETURN_ON_ERR(public_submodules_->echo_control_mobile->ProcessCaptureAudio(
capture_buffer, stream_delay_ms()));
}
@@ -1504,7 +1521,7 @@
}
// TODO(peah): Perform the queueing ínside QueueRenderAudiuo().
- if (private_submodules_->echo_controller) {
+ if (capture_nonlocked_.echo_controller_enabled) {
private_submodules_->echo_controller->AnalyzeRender(render_buffer);
}
@@ -1627,7 +1644,7 @@
const {
AudioProcessingStatistics stats;
EchoCancellation::Metrics metrics;
- if (private_submodules_->echo_controller) {
+ if (capture_nonlocked_.echo_controller_enabled) {
rtc::CritScope cs_capture(&crit_capture_);
auto ec_metrics = private_submodules_->echo_controller->GetMetrics();
float erl = static_cast<float>(ec_metrics.echo_return_loss);
@@ -1663,7 +1680,7 @@
AudioProcessingStats stats;
if (has_remote_tracks) {
EchoCancellation::Metrics metrics;
- if (private_submodules_->echo_controller) {
+ if (capture_nonlocked_.echo_controller_enabled) {
rtc::CritScope cs_capture(&crit_capture_);
auto ec_metrics = private_submodules_->echo_controller->GetMetrics();
stats.echo_return_loss = ec_metrics.echo_return_loss;
@@ -1853,8 +1870,8 @@
static const int kMinDiffDelayMs = 60;
if (echo_cancellation()->is_enabled()) {
- // Activate delay_jumps_ counters if we know echo_cancellation is running.
- // If a stream has echo we know that the echo_cancellation is in process.
+ // Activate delay_jumps_ counters if we know AEC2 is running.
+ // If a stream has echo we know that echo cancellation is in process.
if (capture_.stream_delay_jumps == -1 &&
echo_cancellation()->stream_has_echo()) {
capture_.stream_delay_jumps = 0;
diff --git a/modules/audio_processing/audio_processing_impl.h b/modules/audio_processing/audio_processing_impl.h
index 44d0d08..8d0c4ce 100644
--- a/modules/audio_processing/audio_processing_impl.h
+++ b/modules/audio_processing/audio_processing_impl.h
@@ -393,6 +393,8 @@
int prev_analog_mic_level;
} capture_ RTC_GUARDED_BY(crit_capture_);
+ // This struct requires EITHER crit_capture_ OR crit_render_ to read, and
+ // requires both locks to write.
struct ApmCaptureNonLockedState {
ApmCaptureNonLockedState(bool intelligibility_enabled)
: capture_processing_format(kSampleRate16kHz),
diff --git a/modules/audio_processing/audio_processing_unittest.cc b/modules/audio_processing/audio_processing_unittest.cc
index 4b244fc..3b7c930 100644
--- a/modules/audio_processing/audio_processing_unittest.cc
+++ b/modules/audio_processing/audio_processing_unittest.cc
@@ -176,23 +176,24 @@
}
void EnableAllAPComponents(AudioProcessing* ap) {
+ AudioProcessing::Config apm_config;
#if defined(WEBRTC_AUDIOPROC_FIXED_PROFILE)
- EXPECT_NOERR(ap->echo_control_mobile()->Enable(true));
+ apm_config.echo_cancellation.enabled = true;
+ apm_config.echo_cancellation.mobile_mode = true;
EXPECT_NOERR(ap->gain_control()->set_mode(GainControl::kAdaptiveDigital));
EXPECT_NOERR(ap->gain_control()->Enable(true));
#elif defined(WEBRTC_AUDIOPROC_FLOAT_PROFILE)
+ apm_config.echo_cancellation.enabled = true;
+ apm_config.echo_cancellation.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()->Enable(true));
EXPECT_NOERR(ap->gain_control()->set_mode(GainControl::kAdaptiveAnalog));
EXPECT_NOERR(ap->gain_control()->set_analog_level_limits(0, 255));
EXPECT_NOERR(ap->gain_control()->Enable(true));
#endif
-
- AudioProcessing::Config apm_config;
apm_config.high_pass_filter.enabled = true;
ap->ApplyConfig(apm_config);
diff --git a/modules/audio_processing/include/audio_processing.h b/modules/audio_processing/include/audio_processing.h
index 9d27f16..b543b6c 100644
--- a/modules/audio_processing/include/audio_processing.h
+++ b/modules/audio_processing/include/audio_processing.h
@@ -253,6 +253,13 @@
// by changing the default values in the AudioProcessing::Config struct.
// The config is applied by passing the struct to the ApplyConfig method.
struct Config {
+ // Configures whether acoustic echo cancellation is performed.
+ // Has a specific tuning for mobile devices.
+ struct EchoCancellation {
+ bool enabled = false;
+ bool mobile_mode = false;
+ } echo_cancellation;
+
struct ResidualEchoDetector {
bool enabled = true;
} residual_echo_detector;
@@ -796,7 +803,7 @@
class EchoCancellation {
public:
// EchoCancellation and EchoControlMobile may not be enabled simultaneously.
- // Enabling one will disable the other.
+ // If both are enabled, one (unspecified) will automatically be disabled.
virtual int Enable(bool enable) = 0;
virtual bool is_enabled() const = 0;
@@ -900,7 +907,7 @@
class EchoControlMobile {
public:
// EchoCancellation and EchoControlMobile may not be enabled simultaneously.
- // Enabling one will disable the other.
+ // If both are enabled, one (unspecified) will automatically be disabled.
virtual int Enable(bool enable) = 0;
virtual bool is_enabled() const = 0;
diff --git a/modules/audio_processing/test/debug_dump_replayer.cc b/modules/audio_processing/test/debug_dump_replayer.cc
index a06c76e..13dfabb 100644
--- a/modules/audio_processing/test/debug_dump_replayer.cc
+++ b/modules/audio_processing/test/debug_dump_replayer.cc
@@ -202,8 +202,10 @@
// AEC configs.
RTC_CHECK(msg.has_aec_enabled());
- RTC_CHECK_EQ(AudioProcessing::kNoError,
- apm_->echo_cancellation()->Enable(msg.aec_enabled()));
+ if (msg.aec_enabled()) {
+ apm_config.echo_cancellation.enabled = true;
+ apm_config.echo_cancellation.mobile_mode = false;
+ }
RTC_CHECK(msg.has_aec_drift_compensation_enabled());
RTC_CHECK_EQ(AudioProcessing::kNoError,
@@ -218,8 +220,10 @@
// AECM configs.
RTC_CHECK(msg.has_aecm_enabled());
- RTC_CHECK_EQ(AudioProcessing::kNoError,
- apm_->echo_control_mobile()->Enable(msg.aecm_enabled()));
+ if (msg.aecm_enabled()) {
+ apm_config.echo_cancellation.enabled = true;
+ apm_config.echo_cancellation.mobile_mode = true;
+ }
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 4e29433..f86ee6f 100644
--- a/modules/audio_processing/test/debug_dump_test.cc
+++ b/modules/audio_processing/test/debug_dump_test.cc
@@ -368,8 +368,9 @@
generator.StartRecording();
generator.Process(100);
- EchoCancellation* aec = generator.apm()->echo_cancellation();
- EXPECT_EQ(AudioProcessing::kNoError, aec->Enable(!aec->is_enabled()));
+ AudioProcessing::Config new_config;
+ new_config.echo_cancellation.enabled = true;
+ generator.apm()->ApplyConfig(new_config);
generator.Process(100);
generator.StopRecording();
@@ -407,6 +408,7 @@
// Arbitrarily set clipping gain to 17, which will never be the default.
config.Set<ExperimentalAgc>(new ExperimentalAgc(true, 0, 17));
bool enable_aec3 = true;
+ apm_config.echo_cancellation.enabled = true;
DebugDumpGenerator generator(config, apm_config, enable_aec3);
generator.StartRecording();
generator.Process(100);
@@ -463,7 +465,8 @@
TEST_F(DebugDumpTest, VerifyAec3ExperimentalString) {
Config config;
AudioProcessing::Config apm_config;
- DebugDumpGenerator generator(config, apm_config, true);
+ apm_config.echo_cancellation.enabled = true;
+ DebugDumpGenerator generator(config, apm_config, true /* enable_aec3 */);
generator.StartRecording();
generator.Process(100);
generator.StopRecording();
diff --git a/test/fuzzers/audio_processing_configs_fuzzer.cc b/test/fuzzers/audio_processing_configs_fuzzer.cc
index 545e455..4da38a9 100644
--- a/test/fuzzers/audio_processing_configs_fuzzer.cc
+++ b/test/fuzzers/audio_processing_configs_fuzzer.cc
@@ -128,7 +128,7 @@
apm->AttachAecDump(
absl::make_unique<testing::NiceMock<webrtc::test::MockAecDump>>());
- webrtc::AudioProcessing::Config apm_config;
+ webrtc::AudioProcessing::Config apm_config = apm->GetConfig();
apm_config.residual_echo_detector.enabled = red;
apm_config.high_pass_filter.enabled = hpf;
apm_config.gain_controller2.enabled = use_agc2_limiter;