diff --git a/modules/audio_processing/audio_processing_impl.cc b/modules/audio_processing/audio_processing_impl.cc index 7e5955a98f..e229b455ac 100644 --- a/modules/audio_processing/audio_processing_impl.cc +++ b/modules/audio_processing/audio_processing_impl.cc @@ -385,8 +385,10 @@ AudioProcessingImpl::AudioProcessingImpl( rtc::CritScope cs_capture(&crit_capture_); // Mark Echo Controller enabled if a factory is injected. - capture_nonlocked_.echo_controller_enabled = - static_cast(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::MaybeInitialize( } 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 @@ void AudioProcessingImpl::ApplyConfig(const AudioProcessing::Config& config) { 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 @@ int AudioProcessingImpl::ProcessCaptureStreamLocked() { 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 @@ int AudioProcessingImpl::ProcessCaptureStreamLocked() { 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 @@ int AudioProcessingImpl::ProcessCaptureStreamLocked() { 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 @@ int AudioProcessingImpl::ProcessCaptureStreamLocked() { // 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 @@ int AudioProcessingImpl::ProcessCaptureStreamLocked() { // 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 @@ int AudioProcessingImpl::ProcessCaptureStreamLocked() { 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 @@ int AudioProcessingImpl::ProcessRenderStreamLocked() { } // 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 @@ AudioProcessing::AudioProcessingStatistics AudioProcessingImpl::GetStatistics() 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(ec_metrics.echo_return_loss); @@ -1663,7 +1680,7 @@ AudioProcessingStats AudioProcessingImpl::GetStatistics( 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 @@ void AudioProcessingImpl::MaybeUpdateHistograms() { 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 44d0d087a5..8d0c4ce77b 100644 --- a/modules/audio_processing/audio_processing_impl.h +++ b/modules/audio_processing/audio_processing_impl.h @@ -393,6 +393,8 @@ class AudioProcessingImpl : public AudioProcessing { 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 4b244fc8ab..3b7c930ecf 100644 --- a/modules/audio_processing/audio_processing_unittest.cc +++ b/modules/audio_processing/audio_processing_unittest.cc @@ -176,23 +176,24 @@ bool FrameDataAreEqual(const AudioFrame& frame1, const AudioFrame& frame2) { } 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 9d27f1627d..b543b6cf71 100644 --- a/modules/audio_processing/include/audio_processing.h +++ b/modules/audio_processing/include/audio_processing.h @@ -253,6 +253,13 @@ class AudioProcessing : public rtc::RefCountInterface { // 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 ProcessingConfig { 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 EchoCancellation { 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 a06c76e32a..13dfabb235 100644 --- a/modules/audio_processing/test/debug_dump_replayer.cc +++ b/modules/audio_processing/test/debug_dump_replayer.cc @@ -202,8 +202,10 @@ void DebugDumpReplayer::ConfigureApm(const audioproc::Config& msg) { // 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 @@ void DebugDumpReplayer::ConfigureApm(const audioproc::Config& msg) { // 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 4e2943395e..f86ee6feac 100644 --- a/modules/audio_processing/test/debug_dump_test.cc +++ b/modules/audio_processing/test/debug_dump_test.cc @@ -368,8 +368,9 @@ TEST_F(DebugDumpTest, ToggleDelayAgnosticAec) { 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 @@ TEST_F(DebugDumpTest, VerifyCombinedExperimentalStringInclusive) { // Arbitrarily set clipping gain to 17, which will never be the default. config.Set(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, VerifyCombinedExperimentalStringExclusive) { 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 545e455e7e..4da38a9f7a 100644 --- a/test/fuzzers/audio_processing_configs_fuzzer.cc +++ b/test/fuzzers/audio_processing_configs_fuzzer.cc @@ -128,7 +128,7 @@ std::unique_ptr CreateApm(test::FuzzDataHelper* fuzz_data, apm->AttachAecDump( absl::make_unique>()); - 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;