APM: add field trial to disable `TransientSuppressor` Regardless of the APM config, the transient suppressor (TS) submodule won't be created if the `WebRTC-ApmTransientSuppressorKillSwitch` field trial, disabled by default, is enabled. Bug: webrtc:13663 Change-Id: Ic1ef9aa57c728296d671d4ef253630c581a86610 Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/286382 Reviewed-by: Hanna Silen <silen@webrtc.org> Commit-Queue: Alessio Bazzica <alessiob@webrtc.org> Cr-Commit-Position: refs/heads/main@{#38839}
diff --git a/modules/audio_processing/audio_processing_impl.cc b/modules/audio_processing/audio_processing_impl.cc index 2cb4858..9724666 100644 --- a/modules/audio_processing/audio_processing_impl.cc +++ b/modules/audio_processing/audio_processing_impl.cc
@@ -394,51 +394,65 @@ }; } -// Switches all gain control to AGC2 if experimenting with input volume -// controller. -const AudioProcessing::Config AdjustConfig( +// If `disallow_transient_supporessor_usage` is true, disables transient +// suppression. When `input_volume_controller_config_override` is specified, +// switches all gain control to AGC2. +AudioProcessing::Config AdjustConfig( const AudioProcessing::Config& config, + bool disallow_transient_supporessor_usage, const absl::optional<InputVolumeController::Config>& input_volume_controller_config_override) { - const bool analog_agc_enabled = + AudioProcessing::Config adjusted_config = config; + + // Override the transient suppressor configuration. + if (disallow_transient_supporessor_usage) { + adjusted_config.transient_suppression.enabled = false; + } + + // Override the auto gain control configuration if the AGC1 analog gain + // controller is active and `input_volume_controller_config_override` is + // specified. + const bool agc1_analog_enabled = config.gain_controller1.enabled && (config.gain_controller1.mode == AudioProcessing::Config::GainController1::kAdaptiveAnalog || config.gain_controller1.analog_gain_controller.enabled); - - // Do not update the config if none of the analog AGCs is active - // regardless of the input volume controller override. - if (!analog_agc_enabled || - !input_volume_controller_config_override.has_value()) { - return config; + if (agc1_analog_enabled && + input_volume_controller_config_override.has_value()) { + // Check that the unadjusted AGC config meets the preconditions. + const bool hybrid_agc_config_detected = + config.gain_controller1.enabled && + config.gain_controller1.analog_gain_controller.enabled && + !config.gain_controller1.analog_gain_controller + .enable_digital_adaptive && + config.gain_controller2.enabled && + config.gain_controller2.adaptive_digital.enabled; + const bool full_agc1_config_detected = + config.gain_controller1.enabled && + config.gain_controller1.analog_gain_controller.enabled && + config.gain_controller1.analog_gain_controller + .enable_digital_adaptive && + !config.gain_controller2.enabled; + const bool one_and_only_one_input_volume_controller = + hybrid_agc_config_detected != full_agc1_config_detected; + if (!one_and_only_one_input_volume_controller || + config.gain_controller2.input_volume_controller.enabled) { + RTC_LOG(LS_ERROR) << "Cannot adjust AGC config (precondition failed)"; + if (!one_and_only_one_input_volume_controller) + RTC_LOG(LS_ERROR) + << "One and only one input volume controller must be enabled."; + if (config.gain_controller2.input_volume_controller.enabled) + RTC_LOG(LS_ERROR) + << "The AGC2 input volume controller must be disabled."; + } else { + adjusted_config.gain_controller1.enabled = false; + adjusted_config.gain_controller1.analog_gain_controller.enabled = false; + adjusted_config.gain_controller2.enabled = true; + adjusted_config.gain_controller2.adaptive_digital.enabled = true; + adjusted_config.gain_controller2.input_volume_controller.enabled = true; + } } - const bool hybrid_agc_config_detected = - config.gain_controller1.enabled && - config.gain_controller1.analog_gain_controller.enabled && - !config.gain_controller1.analog_gain_controller.enable_digital_adaptive && - config.gain_controller2.enabled && - config.gain_controller2.adaptive_digital.enabled; - - const bool full_agc1_config_detected = - config.gain_controller1.enabled && - config.gain_controller1.analog_gain_controller.enabled && - config.gain_controller1.analog_gain_controller.enable_digital_adaptive && - !config.gain_controller2.enabled; - - if (hybrid_agc_config_detected == full_agc1_config_detected || - config.gain_controller2.input_volume_controller.enabled) { - RTC_LOG(LS_ERROR) << "Unexpected AGC config: Config not adjusted."; - return config; - } - - AudioProcessing::Config adjusted_config = config; - adjusted_config.gain_controller1.enabled = false; - adjusted_config.gain_controller1.analog_gain_controller.enabled = false; - adjusted_config.gain_controller2.enabled = true; - adjusted_config.gain_controller2.adaptive_digital.enabled = true; - adjusted_config.gain_controller2.input_volume_controller.enabled = true; - return adjusted_config; } @@ -583,13 +597,17 @@ GetInputVolumeControllerConfigOverride()), use_denormal_disabler_( !field_trial::IsEnabled("WebRTC-ApmDenormalDisablerKillSwitch")), + disallow_transient_supporessor_usage_( + field_trial::IsEnabled("WebRTC-ApmTransientSuppressorKillSwitch")), transient_suppressor_vad_mode_(GetTransientSuppressorVadMode()), capture_runtime_settings_(RuntimeSettingQueueSize()), render_runtime_settings_(RuntimeSettingQueueSize()), capture_runtime_settings_enqueuer_(&capture_runtime_settings_), render_runtime_settings_enqueuer_(&render_runtime_settings_), echo_control_factory_(std::move(echo_control_factory)), - config_(AdjustConfig(config, input_volume_controller_config_override_)), + config_(AdjustConfig(config, + disallow_transient_supporessor_usage_, + input_volume_controller_config_override_)), submodule_states_(!!capture_post_processor, !!render_pre_processor, !!capture_analyzer), @@ -824,11 +842,9 @@ MutexLock lock_render(&mutex_render_); MutexLock lock_capture(&mutex_capture_); - // TODO(bugs.webrtc.org/7494): Replace `adjusted_config` with `config` after - // "WebRTC-Audio-InputVolumeControllerExperiment" field trial is removed. const auto adjusted_config = - AdjustConfig(config, input_volume_controller_config_override_); - + AdjustConfig(config, disallow_transient_supporessor_usage_, + input_volume_controller_config_override_); RTC_LOG(LS_INFO) << "AudioProcessing::ApplyConfig: " << adjusted_config.ToString();
diff --git a/modules/audio_processing/audio_processing_impl.h b/modules/audio_processing/audio_processing_impl.h index 9a30c8b..66e98dc 100644 --- a/modules/audio_processing/audio_processing_impl.h +++ b/modules/audio_processing/audio_processing_impl.h
@@ -191,14 +191,19 @@ static std::atomic<int> instance_count_; const bool use_setup_specific_default_aec3_config_; - // TODO(bugs.webrtc.org/7494): Remove the the config when the field trial is - // removed. "WebRTC-Audio-InputVolumeControllerExperiment" field trial - // override for the input volume controller config. + // TODO(bugs.webrtc.org/7494): Remove when the linked field trial is removed. + // Override base on the "WebRTC-Audio-InputVolumeControllerExperiment" field + // trial for the AGC2 input volume controller configuration. const absl::optional<InputVolumeController::Config> input_volume_controller_config_override_; const bool use_denormal_disabler_; + // When true, the transient suppressor submodule is never created regardless + // of the APM configuration. + // TODO(bugs.webrtc.org/13663): Remove when the linked field trial is removed. + const bool disallow_transient_supporessor_usage_; + const TransientSuppressor::VadMode transient_suppressor_vad_mode_; SwapQueue<RuntimeSetting> capture_runtime_settings_;
diff --git a/modules/audio_processing/audio_processing_impl_unittest.cc b/modules/audio_processing/audio_processing_impl_unittest.cc index ea61dae..346b5f5 100644 --- a/modules/audio_processing/audio_processing_impl_unittest.cc +++ b/modules/audio_processing/audio_processing_impl_unittest.cc
@@ -960,6 +960,55 @@ } } +TEST(AudioProcessingImplTest, CanDisableTransientSuppressor) { + // Do not explicitly disable "WebRTC-ApmTransientSuppressorKillSwitch" since + // to check that, by default, it is disabled. + auto apm = AudioProcessingBuilder() + .SetConfig({.transient_suppression = {.enabled = false}}) + .Create(); + EXPECT_FALSE(apm->GetConfig().transient_suppression.enabled); +} + +TEST(AudioProcessingImplTest, CanEnableTransientSuppressor) { + // Do not explicitly disable "WebRTC-ApmTransientSuppressorKillSwitch" since + // to check that, by default, it is disabled. + auto apm = AudioProcessingBuilder() + .SetConfig({.transient_suppression = {.enabled = true}}) + .Create(); + EXPECT_TRUE(apm->GetConfig().transient_suppression.enabled); +} + +TEST(AudioProcessingImplTest, CanDisableTransientSuppressorIfUsageAllowed) { + // Disable the field trial that disallows to enable transient suppression. + test::ScopedFieldTrials field_trials( + "WebRTC-ApmTransientSuppressorKillSwitch/Disabled/"); + auto apm = AudioProcessingBuilder() + .SetConfig({.transient_suppression = {.enabled = false}}) + .Create(); + EXPECT_FALSE(apm->GetConfig().transient_suppression.enabled); +} + +TEST(AudioProcessingImplTest, CanEnableTransientSuppressorIfUsageAllowed) { + // Disable the field trial that disallows to enable transient suppression. + test::ScopedFieldTrials field_trials( + "WebRTC-ApmTransientSuppressorKillSwitch/Disabled/"); + auto apm = AudioProcessingBuilder() + .SetConfig({.transient_suppression = {.enabled = true}}) + .Create(); + EXPECT_TRUE(apm->GetConfig().transient_suppression.enabled); +} + +TEST(AudioProcessingImplTest, + CannotEnableTransientSuppressorIfUsageDisallowed) { + // Enable the field trial that disallows to enable transient suppression. + test::ScopedFieldTrials field_trials( + "WebRTC-ApmTransientSuppressorKillSwitch/Enabled/"); + auto apm = AudioProcessingBuilder() + .SetConfig({.transient_suppression = {.enabled = true}}) + .Create(); + EXPECT_FALSE(apm->GetConfig().transient_suppression.enabled); +} + // Tests that the minimum startup volume is applied at the startup. TEST_P(InputVolumeStartupParameterizedTest, VerifyStartupMinVolumeAppliedAtStartup) {