From f3aa6326b8e21f627b9fba72040122723251999b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Per=20=C3=85hgren?= Date: Fri, 3 Jan 2020 23:00:39 +0100 Subject: [PATCH] Replace the ExperimentalAgc config with the new config format MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This CL replaces the use of the ExperimentalAgc config with using the new config format. Beyond that, some further changes were made to how the analog and digital AGCs are initialized/called. While these can be made in a separate CL, I believe the code changes becomes more clear by bundling those with the replacement of the ExperimentalAgc config. TBR: saza@webrtc.org Bug: webrtc:5298 Change-Id: Ia19940f3abae048541e6716d0184b4caafc7d53e Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/163986 Reviewed-by: Per Åhgren Commit-Queue: Per Åhgren Cr-Commit-Position: refs/heads/master@{#30149} --- .../agc/agc_manager_direct_unittest.cc | 2 +- modules/audio_processing/agc/gain_control.h | 3 - .../audio_processing/audio_processing_impl.cc | 226 +++++++++++------- .../audio_processing/audio_processing_impl.h | 25 +- .../audio_processing_unittest.cc | 30 ++- modules/audio_processing/gain_control_impl.cc | 29 --- modules/audio_processing/gain_control_impl.h | 6 +- .../audio_processing/gain_control_unittest.cc | 1 - .../include/audio_processing.h | 15 ++ .../test/aec_dump_based_simulator.cc | 7 +- .../test/audio_processing_simulator.cc | 21 +- .../test/audio_processing_simulator.h | 7 +- .../test/audioproc_float_impl.cc | 34 +-- .../test/debug_dump_replayer.cc | 8 +- .../audio_processing/test/debug_dump_test.cc | 12 +- test/fuzzers/agc_fuzzer.cc | 2 - 16 files changed, 221 insertions(+), 207 deletions(-) diff --git a/modules/audio_processing/agc/agc_manager_direct_unittest.cc b/modules/audio_processing/agc/agc_manager_direct_unittest.cc index b7c569b6cf..c5e65adec1 100644 --- a/modules/audio_processing/agc/agc_manager_direct_unittest.cc +++ b/modules/audio_processing/agc/agc_manager_direct_unittest.cc @@ -37,7 +37,7 @@ const int kMinMicLevel = 12; class MockGainControl : public GainControl { public: virtual ~MockGainControl() {} - MOCK_METHOD1(Enable, int(bool enable)); + MOCK_METHOD0(Initialize, void()); MOCK_CONST_METHOD0(is_enabled, bool()); MOCK_METHOD1(set_stream_analog_level, int(int level)); MOCK_CONST_METHOD0(stream_analog_level, int()); diff --git a/modules/audio_processing/agc/gain_control.h b/modules/audio_processing/agc/gain_control.h index f31cbecbb3..f8c706b9ab 100644 --- a/modules/audio_processing/agc/gain_control.h +++ b/modules/audio_processing/agc/gain_control.h @@ -20,9 +20,6 @@ namespace webrtc { // Recommended to be enabled on the client-side. class GainControl { public: - virtual int Enable(bool enable) = 0; - virtual bool is_enabled() const = 0; - // When an analog mode is set, this must be called prior to |ProcessStream()| // to pass the current analog level from the audio HAL. Must be within the // range provided to |set_analog_level_limits()|. diff --git a/modules/audio_processing/audio_processing_impl.cc b/modules/audio_processing/audio_processing_impl.cc index 1c88581a03..28443116d0 100644 --- a/modules/audio_processing/audio_processing_impl.cc +++ b/modules/audio_processing/audio_processing_impl.cc @@ -334,18 +334,7 @@ AudioProcessingImpl::AudioProcessingImpl( std::move(render_pre_processor), std::move(echo_detector), std::move(capture_analyzer)), - constants_(config.Get().startup_min_volume, - config.Get().clipped_level_min, -#if defined(WEBRTC_ANDROID) || defined(WEBRTC_IOS) - /* enabled= */ false, - /* enabled_agc2_level_estimator= */ false, - /* digital_adaptive_disabled= */ false, -#else - config.Get().enabled, - config.Get().enabled_agc2_level_estimator, - config.Get().digital_adaptive_disabled, -#endif - !field_trial::IsEnabled( + constants_(!field_trial::IsEnabled( "WebRTC-ApmExperimentalMultiChannelRenderKillSwitch"), !field_trial::IsEnabled( "WebRTC-ApmExperimentalMultiChannelCaptureKillSwitch"), @@ -364,18 +353,29 @@ AudioProcessingImpl::AudioProcessingImpl( capture_nonlocked_.echo_controller_enabled = static_cast(echo_control_factory_); - submodules_.gain_control.reset(new GainControlImpl()); - // If no echo detector is injected, use the ResidualEchoDetector. if (!submodules_.echo_detector) { submodules_.echo_detector = new rtc::RefCountedObject(); } +#if !(defined(WEBRTC_ANDROID) || defined(WEBRTC_IOS)) // TODO(webrtc:5298): Remove once the use of ExperimentalNs has been // deprecated. -#if !(defined(WEBRTC_ANDROID) || defined(WEBRTC_IOS)) config_.transient_suppression.enabled = config.Get().enabled; + + // TODO(webrtc:5298): Remove once the use of ExperimentalAgc has been + // deprecated. + config_.gain_controller1.analog_gain_controller.enabled = + config.Get().enabled; + config_.gain_controller1.analog_gain_controller.startup_min_volume = + config.Get().startup_min_volume; + config_.gain_controller1.analog_gain_controller.clipped_level_min = + config.Get().clipped_level_min; + config_.gain_controller1.analog_gain_controller.enable_agc2_level_estimator = + config.Get().enabled_agc2_level_estimator; + config_.gain_controller1.analog_gain_controller.enable_digital_adaptive = + !config.Get().digital_adaptive_disabled; #endif } @@ -480,34 +480,7 @@ int AudioProcessingImpl::InitializeLocked() { AllocateRenderQueue(); - submodules_.gain_control->Initialize(num_proc_channels(), - proc_sample_rate_hz()); - if (constants_.use_experimental_agc) { - if (!submodules_.agc_manager.get() || - submodules_.agc_manager->num_channels() != - static_cast(num_proc_channels()) || - submodules_.agc_manager->sample_rate_hz() != - capture_nonlocked_.split_rate) { - int stream_analog_level = -1; - const bool re_creation = !!submodules_.agc_manager; - if (re_creation) { - stream_analog_level = submodules_.agc_manager->stream_analog_level(); - } - submodules_.agc_manager.reset(new AgcManagerDirect( - num_proc_channels(), constants_.agc_startup_min_volume, - constants_.agc_clipped_level_min, - constants_.use_experimental_agc_agc2_level_estimation, - constants_.use_experimental_agc_agc2_digital_adaptive, - capture_nonlocked_.split_rate)); - if (re_creation) { - submodules_.agc_manager->set_stream_analog_level(stream_analog_level); - } - } - submodules_.agc_manager->Initialize(); - submodules_.agc_manager->SetupDigitalGainControl( - submodules_.gain_control.get()); - submodules_.agc_manager->SetCaptureMuted(capture_.output_will_be_muted); - } + InitializeGainController1(); InitializeTransientSuppressor(); InitializeHighPassFilter(true); InitializeVoiceDetector(); @@ -650,7 +623,20 @@ void AudioProcessingImpl::ApplyConfig(const AudioProcessing::Config& config) { config_.gain_controller1.analog_level_minimum != config.gain_controller1.analog_level_minimum || config_.gain_controller1.analog_level_maximum != - config.gain_controller1.analog_level_maximum; + config.gain_controller1.analog_level_maximum || + config_.gain_controller1.analog_gain_controller.enabled != + config.gain_controller1.analog_gain_controller.enabled || + config_.gain_controller1.analog_gain_controller.startup_min_volume != + config.gain_controller1.analog_gain_controller.startup_min_volume || + config_.gain_controller1.analog_gain_controller.clipped_level_min != + config.gain_controller1.analog_gain_controller.clipped_level_min || + config_.gain_controller1.analog_gain_controller + .enable_agc2_level_estimator != + config.gain_controller1.analog_gain_controller + .enable_agc2_level_estimator || + config_.gain_controller1.analog_gain_controller.enable_digital_adaptive != + config.gain_controller1.analog_gain_controller + .enable_digital_adaptive; const bool agc2_config_changed = config_.gain_controller2.enabled != config.gain_controller2.enabled; @@ -687,7 +673,7 @@ void AudioProcessingImpl::ApplyConfig(const AudioProcessing::Config& config) { InitializeHighPassFilter(false); if (agc1_config_changed) { - ApplyAgc1Config(config_.gain_controller1); + InitializeGainController1(); } const bool config_ok = GainController2::Validate(config_.gain_controller2); @@ -722,29 +708,6 @@ void AudioProcessingImpl::ApplyConfig(const AudioProcessing::Config& config) { } } -void AudioProcessingImpl::ApplyAgc1Config( - const Config::GainController1& config) { - int error = submodules_.gain_control->Enable(config.enabled); - RTC_DCHECK_EQ(kNoError, error); - - if (!submodules_.agc_manager) { - error = submodules_.gain_control->set_mode( - Agc1ConfigModeToInterfaceMode(config.mode)); - RTC_DCHECK_EQ(kNoError, error); - error = submodules_.gain_control->set_target_level_dbfs( - config.target_level_dbfs); - RTC_DCHECK_EQ(kNoError, error); - error = submodules_.gain_control->set_compression_gain_db( - config.compression_gain_db); - RTC_DCHECK_EQ(kNoError, error); - error = submodules_.gain_control->enable_limiter(config.enable_limiter); - RTC_DCHECK_EQ(kNoError, error); - error = submodules_.gain_control->set_analog_level_limits( - config.analog_level_minimum, config.analog_level_maximum); - RTC_DCHECK_EQ(kNoError, error); - } -} - // TODO(webrtc:5298): Remove. void AudioProcessingImpl::SetExtraOptions(const webrtc::Config& config) {} @@ -934,9 +897,11 @@ void AudioProcessingImpl::HandleCaptureRuntimeSettings() { setting.GetFloat(&value); int int_value = static_cast(value + .5f); config_.gain_controller1.compression_gain_db = int_value; - int error = - submodules_.gain_control->set_compression_gain_db(int_value); - RTC_DCHECK_EQ(kNoError, error); + if (submodules_.gain_control) { + int error = + submodules_.gain_control->set_compression_gain_db(int_value); + RTC_DCHECK_EQ(kNoError, error); + } } break; } @@ -1012,7 +977,7 @@ void AudioProcessingImpl::QueueBandedRenderAudio(AudioBuffer* audio) { } } - if (!submodules_.agc_manager) { + if (!submodules_.agc_manager && submodules_.gain_control) { GainControlImpl::PackRenderAudioBuffer(*audio, &agc_render_queue_buffer_); // Insert the samples into the queue. if (!agc_render_signal_queue_->Insert(&agc_render_queue_buffer_)) { @@ -1099,8 +1064,10 @@ void AudioProcessingImpl::EmptyQueuedRenderAudio() { } } - while (agc_render_signal_queue_->Remove(&agc_capture_queue_buffer_)) { - submodules_.gain_control->ProcessRenderAudio(agc_capture_queue_buffer_); + if (submodules_.gain_control) { + while (agc_render_signal_queue_->Remove(&agc_capture_queue_buffer_)) { + submodules_.gain_control->ProcessRenderAudio(agc_capture_queue_buffer_); + } } while (red_render_signal_queue_->Remove(&red_capture_queue_buffer_)) { @@ -1221,8 +1188,7 @@ int AudioProcessingImpl::ProcessCaptureStreamLocked() { submodules_.echo_controller->AnalyzeCapture(capture_buffer); } - if (constants_.use_experimental_agc && - submodules_.gain_control->is_enabled()) { + if (submodules_.agc_manager) { submodules_.agc_manager->AnalyzePreProcess(capture_buffer); } @@ -1249,7 +1215,10 @@ int AudioProcessingImpl::ProcessCaptureStreamLocked() { /*use_split_band_data=*/true); } - RETURN_ON_ERR(submodules_.gain_control->AnalyzeCaptureAudio(*capture_buffer)); + if (submodules_.gain_control) { + RETURN_ON_ERR( + submodules_.gain_control->AnalyzeCaptureAudio(*capture_buffer)); + } RTC_DCHECK( !(submodules_.legacy_noise_suppressor && submodules_.noise_suppressor)); @@ -1314,19 +1283,21 @@ int AudioProcessingImpl::ProcessCaptureStreamLocked() { capture_.stats.voice_detected = absl::nullopt; } - if (constants_.use_experimental_agc && - submodules_.gain_control->is_enabled()) { + if (submodules_.agc_manager) { submodules_.agc_manager->Process(capture_buffer); absl::optional new_digital_gain = submodules_.agc_manager->GetDigitalComressionGain(); - if (new_digital_gain) { + if (new_digital_gain && submodules_.gain_control) { submodules_.gain_control->set_compression_gain_db(*new_digital_gain); } } - // TODO(peah): Add reporting from AEC3 whether there is echo. - RETURN_ON_ERR(submodules_.gain_control->ProcessCaptureAudio( - capture_buffer, /*stream_has_echo*/ false)); + + if (submodules_.gain_control) { + // TODO(peah): Add reporting from AEC3 whether there is echo. + RETURN_ON_ERR(submodules_.gain_control->ProcessCaptureAudio( + capture_buffer, /*stream_has_echo*/ false)); + } if (submodule_states_.CaptureMultiBandProcessingPresent() && SampleRateSupportsMultiBand( @@ -1655,9 +1626,11 @@ void AudioProcessingImpl::set_stream_analog_level(int level) { submodules_.agc_manager->set_stream_analog_level(level); data_dumper_->DumpRaw("experimental_gain_control_set_stream_analog_level", 1, &level); - } else { + } else if (submodules_.gain_control) { int error = submodules_.gain_control->set_stream_analog_level(level); RTC_DCHECK_EQ(kNoError, error); + } else { + capture_.cached_stream_analog_level_ = level; } } @@ -1665,8 +1638,11 @@ int AudioProcessingImpl::recommended_stream_analog_level() const { rtc::CritScope cs_capture(&crit_capture_); if (submodules_.agc_manager) { return submodules_.agc_manager->stream_analog_level(); + } else if (submodules_.gain_control) { + return submodules_.gain_control->stream_analog_level(); + } else { + return capture_.cached_stream_analog_level_; } - return submodules_.gain_control->stream_analog_level(); } void AudioProcessingImpl::AttachAecDump(std::unique_ptr aec_dump) { @@ -1723,7 +1699,7 @@ bool AudioProcessingImpl::UpdateActiveSubmoduleStates() { config_.high_pass_filter.enabled, !!submodules_.echo_control_mobile, config_.residual_echo_detector.enabled, !!submodules_.legacy_noise_suppressor || !!submodules_.noise_suppressor, - submodules_.gain_control->is_enabled(), !!submodules_.gain_controller2, + !!submodules_.gain_control, !!submodules_.gain_controller2, config_.pre_amplifier.enabled, capture_nonlocked_.echo_controller_enabled, config_.voice_detection.enabled, !!submodules_.transient_suppressor); } @@ -1854,6 +1830,71 @@ void AudioProcessingImpl::InitializeEchoController() { aecm_render_signal_queue_.reset(); } +void AudioProcessingImpl::InitializeGainController1() { + if (!config_.gain_controller1.enabled) { + submodules_.agc_manager.reset(); + submodules_.gain_control.reset(); + return; + } + + if (!submodules_.gain_control) { + submodules_.gain_control.reset(new GainControlImpl()); + } + + submodules_.gain_control->Initialize(num_proc_channels(), + proc_sample_rate_hz()); + + if (!config_.gain_controller1.analog_gain_controller.enabled) { + int error = submodules_.gain_control->set_mode( + Agc1ConfigModeToInterfaceMode(config_.gain_controller1.mode)); + RTC_DCHECK_EQ(kNoError, error); + error = submodules_.gain_control->set_target_level_dbfs( + config_.gain_controller1.target_level_dbfs); + RTC_DCHECK_EQ(kNoError, error); + error = submodules_.gain_control->set_compression_gain_db( + config_.gain_controller1.compression_gain_db); + RTC_DCHECK_EQ(kNoError, error); + error = submodules_.gain_control->enable_limiter( + config_.gain_controller1.enable_limiter); + RTC_DCHECK_EQ(kNoError, error); + error = submodules_.gain_control->set_analog_level_limits( + config_.gain_controller1.analog_level_minimum, + config_.gain_controller1.analog_level_maximum); + RTC_DCHECK_EQ(kNoError, error); + + submodules_.agc_manager.reset(); + return; + } + + if (!submodules_.agc_manager.get() || + submodules_.agc_manager->num_channels() != + static_cast(num_proc_channels()) || + submodules_.agc_manager->sample_rate_hz() != + capture_nonlocked_.split_rate) { + int stream_analog_level = -1; + const bool re_creation = !!submodules_.agc_manager; + if (re_creation) { + stream_analog_level = submodules_.agc_manager->stream_analog_level(); + } + submodules_.agc_manager.reset(new AgcManagerDirect( + num_proc_channels(), + config_.gain_controller1.analog_gain_controller.startup_min_volume, + config_.gain_controller1.analog_gain_controller.clipped_level_min, + config_.gain_controller1.analog_gain_controller + .enable_agc2_level_estimator, + !config_.gain_controller1.analog_gain_controller + .enable_digital_adaptive, + capture_nonlocked_.split_rate)); + if (re_creation) { + submodules_.agc_manager->set_stream_analog_level(stream_analog_level); + } + } + submodules_.agc_manager->Initialize(); + submodules_.agc_manager->SetupDigitalGainControl( + submodules_.gain_control.get()); + submodules_.agc_manager->SetCaptureMuted(capture_.output_will_be_muted); +} + void AudioProcessingImpl::InitializeGainController2() { if (config_.gain_controller2.enabled) { if (!submodules_.gain_controller2) { @@ -1957,7 +1998,8 @@ void AudioProcessingImpl::WriteAecDumpConfigMessage(bool forced) { std::string experiments_description = ""; // TODO(peah): Add semicolon-separated concatenations of experiment // descriptions for other submodules. - if (constants_.agc_clipped_level_min != kClippedLevelMin) { + if (config_.gain_controller1.analog_gain_controller.clipped_level_min != + kClippedLevelMin) { experiments_description += "AgcClippingLevelExperiment;"; } if (capture_nonlocked_.echo_controller_enabled) { @@ -1983,10 +2025,14 @@ void AudioProcessingImpl::WriteAecDumpConfigMessage(bool forced) { ? static_cast(submodules_.echo_control_mobile->routing_mode()) : 0; - apm_config.agc_enabled = submodules_.gain_control->is_enabled(); - apm_config.agc_mode = static_cast(submodules_.gain_control->mode()); + apm_config.agc_enabled = !!submodules_.gain_control; + + apm_config.agc_mode = submodules_.gain_control + ? static_cast(submodules_.gain_control->mode()) + : GainControl::kAdaptiveAnalog; apm_config.agc_limiter_enabled = - submodules_.gain_control->is_limiter_enabled(); + submodules_.gain_control ? submodules_.gain_control->is_limiter_enabled() + : false; apm_config.noise_robust_agc_enabled = !!submodules_.agc_manager; apm_config.hpf_enabled = config_.high_pass_filter.enabled; diff --git a/modules/audio_processing/audio_processing_impl.h b/modules/audio_processing/audio_processing_impl.h index ee3fb4d659..af5a0f63c4 100644 --- a/modules/audio_processing/audio_processing_impl.h +++ b/modules/audio_processing/audio_processing_impl.h @@ -243,6 +243,7 @@ class AudioProcessingImpl : public AudioProcessing { void InitializeHighPassFilter(bool forced_reset) RTC_EXCLUSIVE_LOCKS_REQUIRED(crit_capture_); void InitializeVoiceDetector() RTC_EXCLUSIVE_LOCKS_REQUIRED(crit_capture_); + void InitializeGainController1() RTC_EXCLUSIVE_LOCKS_REQUIRED(crit_capture_); void InitializeTransientSuppressor() RTC_EXCLUSIVE_LOCKS_REQUIRED(crit_capture_); void InitializeGainController2() RTC_EXCLUSIVE_LOCKS_REQUIRED(crit_capture_); @@ -263,8 +264,6 @@ class AudioProcessingImpl : public AudioProcessing { void HandleCaptureRuntimeSettings() RTC_EXCLUSIVE_LOCKS_REQUIRED(crit_capture_); void HandleRenderRuntimeSettings() RTC_EXCLUSIVE_LOCKS_REQUIRED(crit_render_); - void ApplyAgc1Config(const Config::GainController1& agc_config) - RTC_EXCLUSIVE_LOCKS_REQUIRED(crit_capture_); void EmptyQueuedRenderAudio(); void AllocateRenderQueue() @@ -381,29 +380,12 @@ class AudioProcessingImpl : public AudioProcessing { // APM constants. const struct ApmConstants { - ApmConstants(int agc_startup_min_volume, - int agc_clipped_level_min, - bool use_experimental_agc, - bool use_experimental_agc_agc2_level_estimation, - bool use_experimental_agc_agc2_digital_adaptive, - bool multi_channel_render_support, + ApmConstants(bool multi_channel_render_support, bool multi_channel_capture_support, bool enforce_split_band_hpf) - : agc_startup_min_volume(agc_startup_min_volume), - agc_clipped_level_min(agc_clipped_level_min), - use_experimental_agc(use_experimental_agc), - use_experimental_agc_agc2_level_estimation( - use_experimental_agc_agc2_level_estimation), - use_experimental_agc_agc2_digital_adaptive( - use_experimental_agc_agc2_digital_adaptive), - multi_channel_render_support(multi_channel_render_support), + : multi_channel_render_support(multi_channel_render_support), multi_channel_capture_support(multi_channel_capture_support), enforce_split_band_hpf(enforce_split_band_hpf) {} - int agc_startup_min_volume; - int agc_clipped_level_min; - bool use_experimental_agc; - bool use_experimental_agc_agc2_level_estimation; - bool use_experimental_agc_agc2_digital_adaptive; bool multi_channel_render_support; bool multi_channel_capture_support; bool enforce_split_band_hpf; @@ -435,6 +417,7 @@ class AudioProcessingImpl : public AudioProcessing { size_t num_keyboard_frames = 0; const float* keyboard_data = nullptr; } keyboard_info; + int cached_stream_analog_level_ = 0; } capture_ RTC_GUARDED_BY(crit_capture_); struct ApmCaptureNonLockedState { diff --git a/modules/audio_processing/audio_processing_unittest.cc b/modules/audio_processing/audio_processing_unittest.cc index 8f9e53529f..ca05f71499 100644 --- a/modules/audio_processing/audio_processing_unittest.cc +++ b/modules/audio_processing/audio_processing_unittest.cc @@ -430,10 +430,9 @@ ApmTest::ApmTest() far_file_(NULL), near_file_(NULL), out_file_(NULL) { - Config config; - config.Set(new ExperimentalAgc(false)); - apm_.reset(AudioProcessingBuilder().Create(config)); + apm_.reset(AudioProcessingBuilder().Create()); AudioProcessing::Config apm_config = apm_->GetConfig(); + apm_config.gain_controller1.analog_gain_controller.enabled = false; apm_config.pipeline.maximum_internal_processing_rate = 48000; apm_->ApplyConfig(apm_config); } @@ -967,42 +966,49 @@ TEST_F(ApmTest, GainControl) { #if RTC_DCHECK_IS_ON && GTEST_HAS_DEATH_TEST && !defined(WEBRTC_ANDROID) TEST_F(ApmTest, GainControlDiesOnTooLowTargetLevelDbfs) { auto config = apm_->GetConfig(); + config.gain_controller1.enabled = true; config.gain_controller1.target_level_dbfs = -1; EXPECT_DEATH(apm_->ApplyConfig(config), ""); } TEST_F(ApmTest, GainControlDiesOnTooHighTargetLevelDbfs) { auto config = apm_->GetConfig(); + config.gain_controller1.enabled = true; config.gain_controller1.target_level_dbfs = 32; EXPECT_DEATH(apm_->ApplyConfig(config), ""); } TEST_F(ApmTest, GainControlDiesOnTooLowCompressionGainDb) { auto config = apm_->GetConfig(); + config.gain_controller1.enabled = true; config.gain_controller1.compression_gain_db = -1; EXPECT_DEATH(apm_->ApplyConfig(config), ""); } TEST_F(ApmTest, GainControlDiesOnTooHighCompressionGainDb) { auto config = apm_->GetConfig(); + config.gain_controller1.enabled = true; config.gain_controller1.compression_gain_db = 91; EXPECT_DEATH(apm_->ApplyConfig(config), ""); } TEST_F(ApmTest, GainControlDiesOnTooLowAnalogLevelLowerLimit) { auto config = apm_->GetConfig(); + config.gain_controller1.enabled = true; config.gain_controller1.analog_level_minimum = -1; EXPECT_DEATH(apm_->ApplyConfig(config), ""); } TEST_F(ApmTest, GainControlDiesOnTooHighAnalogLevelUpperLimit) { auto config = apm_->GetConfig(); + config.gain_controller1.enabled = true; config.gain_controller1.analog_level_maximum = 65536; EXPECT_DEATH(apm_->ApplyConfig(config), ""); } TEST_F(ApmTest, GainControlDiesOnInvertedAnalogLevelLimits) { auto config = apm_->GetConfig(); + config.gain_controller1.enabled = true; config.gain_controller1.analog_level_minimum = 512; config.gain_controller1.analog_level_maximum = 255; EXPECT_DEATH(apm_->ApplyConfig(config), ""); @@ -1010,6 +1016,7 @@ TEST_F(ApmTest, GainControlDiesOnInvertedAnalogLevelLimits) { TEST_F(ApmTest, ApmDiesOnTooLowAnalogLevel) { auto config = apm_->GetConfig(); + config.gain_controller1.enabled = true; config.gain_controller1.analog_level_minimum = 255; config.gain_controller1.analog_level_maximum = 512; apm_->ApplyConfig(config); @@ -1018,6 +1025,7 @@ TEST_F(ApmTest, ApmDiesOnTooLowAnalogLevel) { TEST_F(ApmTest, ApmDiesOnTooHighAnalogLevel) { auto config = apm_->GetConfig(); + config.gain_controller1.enabled = true; config.gain_controller1.analog_level_minimum = 255; config.gain_controller1.analog_level_maximum = 512; apm_->ApplyConfig(config); @@ -1533,9 +1541,10 @@ TEST_F(ApmTest, Process) { if (test->num_input_channels() != test->num_output_channels()) continue; - Config config; - config.Set(new ExperimentalAgc(false)); - apm_.reset(AudioProcessingBuilder().Create(config)); + apm_.reset(AudioProcessingBuilder().Create()); + AudioProcessing::Config apm_config = apm_->GetConfig(); + apm_config.gain_controller1.analog_gain_controller.enabled = false; + apm_->ApplyConfig(apm_config); EnableAllComponents(); @@ -1818,10 +1827,11 @@ class AudioProcessingTest size_t num_reverse_input_channels, size_t num_reverse_output_channels, const std::string& output_file_prefix) { - Config config; - config.Set(new ExperimentalAgc(false)); - std::unique_ptr ap( - AudioProcessingBuilder().Create(config)); + std::unique_ptr ap(AudioProcessingBuilder().Create()); + AudioProcessing::Config apm_config = ap->GetConfig(); + apm_config.gain_controller1.analog_gain_controller.enabled = false; + ap->ApplyConfig(apm_config); + EnableAllAPComponents(ap.get()); ProcessingConfig processing_config = { diff --git a/modules/audio_processing/gain_control_impl.cc b/modules/audio_processing/gain_control_impl.cc index 841d901933..b5454c05ed 100644 --- a/modules/audio_processing/gain_control_impl.cc +++ b/modules/audio_processing/gain_control_impl.cc @@ -112,10 +112,6 @@ GainControlImpl::~GainControlImpl() = default; void GainControlImpl::ProcessRenderAudio( rtc::ArrayView packed_render_audio) { - if (!enabled_) { - return; - } - for (size_t ch = 0; ch < mono_agcs_.size(); ++ch) { WebRtcAgc_AddFarend(mono_agcs_[ch]->state, packed_render_audio.data(), packed_render_audio.size()); @@ -151,10 +147,6 @@ void GainControlImpl::PackRenderAudioBuffer( } int GainControlImpl::AnalyzeCaptureAudio(const AudioBuffer& audio) { - if (!enabled_) { - return AudioProcessing::kNoError; - } - RTC_DCHECK(num_proc_channels_); RTC_DCHECK_GE(AudioBuffer::kMaxSplitFrameLength, audio.num_frames_per_band()); RTC_DCHECK_EQ(audio.num_channels(), *num_proc_channels_); @@ -203,10 +195,6 @@ int GainControlImpl::AnalyzeCaptureAudio(const AudioBuffer& audio) { int GainControlImpl::ProcessCaptureAudio(AudioBuffer* audio, bool stream_has_echo) { - if (!enabled_) { - return AudioProcessing::kNoError; - } - if (mode_ == kAdaptiveAnalog && !was_analog_level_set_) { return AudioProcessing::kStreamParameterNotSetError; } @@ -309,19 +297,6 @@ int GainControlImpl::stream_analog_level() const { return analog_capture_level_; } -int GainControlImpl::Enable(bool enable) { - if (enable && !enabled_) { - enabled_ = enable; // Must be set before Initialize() is called. - - RTC_DCHECK(num_proc_channels_); - RTC_DCHECK(sample_rate_hz_); - Initialize(*num_proc_channels_, *sample_rate_hz_); - } else { - enabled_ = enable; - } - return AudioProcessing::kNoError; -} - int GainControlImpl::set_mode(Mode mode) { if (MapSetting(mode) == -1) { return AudioProcessing::kBadParameterError; @@ -381,10 +356,6 @@ void GainControlImpl::Initialize(size_t num_proc_channels, int sample_rate_hz) { num_proc_channels_ = num_proc_channels; sample_rate_hz_ = sample_rate_hz; - if (!enabled_) { - return; - } - mono_agcs_.resize(*num_proc_channels_); capture_levels_.resize(*num_proc_channels_); for (size_t ch = 0; ch < mono_agcs_.size(); ++ch) { diff --git a/modules/audio_processing/gain_control_impl.h b/modules/audio_processing/gain_control_impl.h index 5ddf5ec8b8..b65d697945 100644 --- a/modules/audio_processing/gain_control_impl.h +++ b/modules/audio_processing/gain_control_impl.h @@ -44,11 +44,9 @@ class GainControlImpl : public GainControl { std::vector* packed_buffer); // GainControl implementation. - bool is_enabled() const override { return enabled_; } int stream_analog_level() const override; bool is_limiter_enabled() const override { return limiter_enabled_; } Mode mode() const override { return mode_; } - int Enable(bool enable) override; int set_mode(Mode mode) override; int compression_gain_db() const override { return compression_gain_db_; } int set_analog_level_limits(int minimum, int maximum) override; @@ -70,8 +68,6 @@ class GainControlImpl : public GainControl { std::unique_ptr data_dumper_; - bool enabled_ = false; - const bool use_legacy_gain_applier_; Mode mode_; int minimum_capture_level_; @@ -79,7 +75,7 @@ class GainControlImpl : public GainControl { bool limiter_enabled_; int target_level_dbfs_; int compression_gain_db_; - int analog_capture_level_; + int analog_capture_level_ = 0; bool was_analog_level_set_; bool stream_is_saturated_; diff --git a/modules/audio_processing/gain_control_unittest.cc b/modules/audio_processing/gain_control_unittest.cc index c1078b409b..6e0149915c 100644 --- a/modules/audio_processing/gain_control_unittest.cc +++ b/modules/audio_processing/gain_control_unittest.cc @@ -52,7 +52,6 @@ void SetupComponent(int sample_rate_hz, GainControlImpl* gain_controller) { gain_controller->Initialize(1, sample_rate_hz); GainControl* gc = static_cast(gain_controller); - gc->Enable(true); gc->set_mode(mode); gc->set_stream_analog_level(stream_analog_level); gc->set_target_level_dbfs(target_level_dbfs); diff --git a/modules/audio_processing/include/audio_processing.h b/modules/audio_processing/include/audio_processing.h index fe4b0dc460..d76d1a82ad 100644 --- a/modules/audio_processing/include/audio_processing.h +++ b/modules/audio_processing/include/audio_processing.h @@ -60,6 +60,10 @@ static const int kAgcStartupMinVolume = 85; static const int kAgcStartupMinVolume = 0; #endif // defined(WEBRTC_CHROMIUM_BUILD) static constexpr int kClippedLevelMin = 70; + +// To be deprecated: Please instead use the flag in the +// AudioProcessing::Config::AnalogGainController. +// TODO(webrtc:5298): Remove. struct ExperimentalAgc { ExperimentalAgc() = default; explicit ExperimentalAgc(bool enabled) : enabled(enabled) {} @@ -314,6 +318,17 @@ class RTC_EXPORT AudioProcessing : public rtc::RefCountInterface { // Must be set if an analog mode is used. Limited to [0, 65535]. int analog_level_minimum = 0; int analog_level_maximum = 255; + + // Enables the analog gain controller functionality. + struct AnalogGainController { + bool enabled = false; + int startup_min_volume = kAgcStartupMinVolume; + // Lowest analog microphone level that will be applied in response to + // clipping. + int clipped_level_min = kClippedLevelMin; + bool enable_agc2_level_estimator = false; + bool enable_digital_adaptive = true; + } analog_gain_controller; } gain_controller1; // Enables the next generation AGC functionality. This feature replaces the diff --git a/modules/audio_processing/test/aec_dump_based_simulator.cc b/modules/audio_processing/test/aec_dump_based_simulator.cc index 95a3e37dbf..142e707ee2 100644 --- a/modules/audio_processing/test/aec_dump_based_simulator.cc +++ b/modules/audio_processing/test/aec_dump_based_simulator.cc @@ -364,11 +364,10 @@ void AecDumpBasedSimulator::HandleMessage( } } - // TODO(peah): Add support for controlling the Experimental AGC from the - // command line. if (msg.has_noise_robust_agc_enabled()) { - config.Set( - new ExperimentalAgc(msg.noise_robust_agc_enabled())); + apm_config.gain_controller1.analog_gain_controller.enabled = + settings_.use_analog_agc ? *settings_.use_analog_agc + : msg.noise_robust_agc_enabled(); if (settings_.use_verbose_logging) { std::cout << " noise_robust_agc_enabled: " << (msg.noise_robust_agc_enabled() ? "true" : "false") diff --git a/modules/audio_processing/test/audio_processing_simulator.cc b/modules/audio_processing/test/audio_processing_simulator.cc index f314732982..84cd9a08b8 100644 --- a/modules/audio_processing/test/audio_processing_simulator.cc +++ b/modules/audio_processing/test/audio_processing_simulator.cc @@ -494,15 +494,20 @@ void AudioProcessingSimulator::CreateAudioProcessor() { apm_config.gain_controller1.compression_gain_db = *settings_.agc_compression_gain; } + if (settings_.use_analog_agc) { + apm_config.gain_controller1.analog_gain_controller.enabled = + *settings_.use_analog_agc; + } + if (settings_.use_analog_agc_agc2_level_estimator) { + apm_config.gain_controller1.analog_gain_controller + .enable_agc2_level_estimator = + *settings_.use_analog_agc_agc2_level_estimator; + } + if (settings_.analog_agc_disable_digital_adaptive) { + apm_config.gain_controller1.analog_gain_controller.enable_digital_adaptive = + *settings_.analog_agc_disable_digital_adaptive; + } - config.Set(new ExperimentalAgc( - !settings_.use_experimental_agc || *settings_.use_experimental_agc, - !!settings_.use_experimental_agc_agc2_level_estimator && - *settings_.use_experimental_agc_agc2_level_estimator, - !!settings_.experimental_agc_disable_digital_adaptive && - *settings_.experimental_agc_disable_digital_adaptive, - !!settings_.experimental_agc_analyze_before_aec && - *settings_.experimental_agc_analyze_before_aec)); if (settings_.use_ed) { apm_config.residual_echo_detector.enabled = *settings_.use_ed; } diff --git a/modules/audio_processing/test/audio_processing_simulator.h b/modules/audio_processing/test/audio_processing_simulator.h index c902d7c9ea..c28dd6d9be 100644 --- a/modules/audio_processing/test/audio_processing_simulator.h +++ b/modules/audio_processing/test/audio_processing_simulator.h @@ -57,14 +57,13 @@ struct SimulationSettings { absl::optional use_hpf; absl::optional use_ns; absl::optional use_ts; + absl::optional use_analog_agc; absl::optional use_vad; absl::optional use_le; absl::optional use_all; absl::optional use_legacy_ns; - absl::optional use_experimental_agc; - absl::optional use_experimental_agc_agc2_level_estimator; - absl::optional experimental_agc_disable_digital_adaptive; - absl::optional experimental_agc_analyze_before_aec; + absl::optional use_analog_agc_agc2_level_estimator; + absl::optional analog_agc_disable_digital_adaptive; absl::optional agc_mode; absl::optional agc_target_level; absl::optional use_agc_limiter; diff --git a/modules/audio_processing/test/audioproc_float_impl.cc b/modules/audio_processing/test/audioproc_float_impl.cc index c4d2ec26b5..ec637c1dcb 100644 --- a/modules/audio_processing/test/audioproc_float_impl.cc +++ b/modules/audio_processing/test/audioproc_float_impl.cc @@ -101,6 +101,10 @@ ABSL_FLAG(int, ts, kParameterNotSpecifiedValue, "Activate (1) or deactivate(0) the transient suppressor"); +ABSL_FLAG(int, + analog_agc, + kParameterNotSpecifiedValue, + "Activate (1) or deactivate(0) the transient suppressor"); ABSL_FLAG(int, vad, kParameterNotSpecifiedValue, @@ -119,21 +123,12 @@ ABSL_FLAG(int, kParameterNotSpecifiedValue, "Activate (1) or deactivate(0) the legacy NS"); ABSL_FLAG(int, - experimental_agc, - kParameterNotSpecifiedValue, - "Activate (1) or deactivate(0) the experimental AGC"); -ABSL_FLAG(int, - experimental_agc_disable_digital_adaptive, + analog_agc_disable_digital_adaptive, kParameterNotSpecifiedValue, "Force-deactivate (1) digital adaptation in " "experimental AGC. Digital adaptation is active by default (0)."); ABSL_FLAG(int, - experimental_agc_analyze_before_aec, - kParameterNotSpecifiedValue, - "Make level estimation happen before AEC" - " in the experimental AGC. After AEC is the default (0)"); -ABSL_FLAG(int, - experimental_agc_agc2_level_estimator, + analog_agc_agc2_level_estimator, kParameterNotSpecifiedValue, "AGC2 level estimation" " in the experimental AGC. AGC1 level estimation is the default (0)"); @@ -334,6 +329,7 @@ SimulationSettings CreateSettings() { settings.use_le = true; settings.use_vad = true; settings.use_ts = true; + settings.use_analog_agc = true; settings.use_ns = true; settings.use_hpf = true; settings.use_agc = true; @@ -377,20 +373,16 @@ SimulationSettings CreateSettings() { SetSettingIfFlagSet(absl::GetFlag(FLAGS_hpf), &settings.use_hpf); SetSettingIfFlagSet(absl::GetFlag(FLAGS_ns), &settings.use_ns); SetSettingIfFlagSet(absl::GetFlag(FLAGS_ts), &settings.use_ts); + SetSettingIfFlagSet(absl::GetFlag(FLAGS_analog_agc), + &settings.use_analog_agc); SetSettingIfFlagSet(absl::GetFlag(FLAGS_vad), &settings.use_vad); SetSettingIfFlagSet(absl::GetFlag(FLAGS_le), &settings.use_le); SetSettingIfFlagSet(absl::GetFlag(FLAGS_use_legacy_ns), &settings.use_legacy_ns); - SetSettingIfFlagSet(absl::GetFlag(FLAGS_experimental_agc), - &settings.use_experimental_agc); - SetSettingIfFlagSet( - absl::GetFlag(FLAGS_experimental_agc_disable_digital_adaptive), - &settings.experimental_agc_disable_digital_adaptive); - SetSettingIfFlagSet(absl::GetFlag(FLAGS_experimental_agc_analyze_before_aec), - &settings.experimental_agc_analyze_before_aec); - SetSettingIfFlagSet( - absl::GetFlag(FLAGS_experimental_agc_agc2_level_estimator), - &settings.use_experimental_agc_agc2_level_estimator); + SetSettingIfFlagSet(absl::GetFlag(FLAGS_analog_agc_disable_digital_adaptive), + &settings.analog_agc_disable_digital_adaptive); + SetSettingIfFlagSet(absl::GetFlag(FLAGS_analog_agc_agc2_level_estimator), + &settings.use_analog_agc_agc2_level_estimator); SetSettingIfSpecified(absl::GetFlag(FLAGS_agc_mode), &settings.agc_mode); SetSettingIfSpecified(absl::GetFlag(FLAGS_agc_target_level), &settings.agc_target_level); diff --git a/modules/audio_processing/test/debug_dump_replayer.cc b/modules/audio_processing/test/debug_dump_replayer.cc index d5cf6732a0..26ca4290c3 100644 --- a/modules/audio_processing/test/debug_dump_replayer.cc +++ b/modules/audio_processing/test/debug_dump_replayer.cc @@ -180,11 +180,6 @@ void DebugDumpReplayer::MaybeRecreateApm(const audioproc::Config& msg) { // These configurations cannot be changed on the fly. Config config; RTC_CHECK(msg.has_aec_delay_agnostic_enabled()); - - RTC_CHECK(msg.has_noise_robust_agc_enabled()); - config.Set( - new ExperimentalAgc(msg.noise_robust_agc_enabled())); - RTC_CHECK(msg.has_aec_extended_filter_enabled()); // We only create APM once, since changes on these fields should not @@ -235,6 +230,9 @@ void DebugDumpReplayer::ConfigureApm(const audioproc::Config& msg) { static_cast( msg.agc_mode()); apm_config.gain_controller1.enable_limiter = msg.agc_limiter_enabled(); + RTC_CHECK(msg.has_noise_robust_agc_enabled()); + apm_config.gain_controller1.analog_gain_controller.enabled = + msg.noise_robust_agc_enabled(); apm_->ApplyConfig(apm_config); } diff --git a/modules/audio_processing/test/debug_dump_test.cc b/modules/audio_processing/test/debug_dump_test.cc index 21458aa5d7..71478a988c 100644 --- a/modules/audio_processing/test/debug_dump_test.cc +++ b/modules/audio_processing/test/debug_dump_test.cc @@ -210,6 +210,7 @@ void DebugDumpGenerator::Process(size_t num_blocks) { ReadAndDeinterleave(&input_audio_, input_file_channels_, input_config_, input_->channels()); RTC_CHECK_EQ(AudioProcessing::kNoError, apm_->set_stream_delay_ms(100)); + apm_->set_stream_analog_level(100); if (enable_pre_amplifier_) { apm_->SetRuntimeSetting( AudioProcessing::RuntimeSetting::CreateCapturePreGain(1 + i % 10)); @@ -358,8 +359,10 @@ TEST_F(DebugDumpTest, VerifyCombinedExperimentalStringInclusive) { Config config; AudioProcessing::Config apm_config; apm_config.echo_canceller.enabled = true; + apm_config.gain_controller1.analog_gain_controller.enabled = true; + apm_config.gain_controller1.analog_gain_controller.startup_min_volume = 0; // Arbitrarily set clipping gain to 17, which will never be the default. - config.Set(new ExperimentalAgc(true, 0, 17)); + apm_config.gain_controller1.analog_gain_controller.clipped_level_min = 17; DebugDumpGenerator generator(config, apm_config); generator.StartRecording(); generator.Process(100); @@ -436,9 +439,12 @@ TEST_F(DebugDumpTest, VerifyAec3ExperimentalString) { TEST_F(DebugDumpTest, VerifyAgcClippingLevelExperimentalString) { Config config; + AudioProcessing::Config apm_config; + apm_config.gain_controller1.analog_gain_controller.enabled = true; + apm_config.gain_controller1.analog_gain_controller.startup_min_volume = 0; // Arbitrarily set clipping gain to 17, which will never be the default. - config.Set(new ExperimentalAgc(true, 0, 17)); - DebugDumpGenerator generator(config, AudioProcessing::Config()); + apm_config.gain_controller1.analog_gain_controller.clipped_level_min = 17; + DebugDumpGenerator generator(config, apm_config); generator.StartRecording(); generator.Process(100); generator.StopRecording(); diff --git a/test/fuzzers/agc_fuzzer.cc b/test/fuzzers/agc_fuzzer.cc index ac3f83b36e..890649ab14 100644 --- a/test/fuzzers/agc_fuzzer.cc +++ b/test/fuzzers/agc_fuzzer.cc @@ -67,9 +67,7 @@ void FuzzGainControllerConfig(test::FuzzDataHelper* fuzz_data, } gc->set_compression_gain_db(gain); gc->set_target_level_dbfs(target_level_dbfs); - gc->Enable(true); - static_cast(gc->is_enabled()); static_cast(gc->mode()); static_cast(gc->analog_level_minimum()); static_cast(gc->analog_level_maximum());