From 9489c3a2ea4b9a596bd6fdeb32b8e37cf2540fa7 Mon Sep 17 00:00:00 2001 From: Alex Loiko Date: Thu, 9 Aug 2018 15:04:24 +0200 Subject: [PATCH] Optionally disable digital gain control in ExperimentalAgc. This CL adds a flag to optionally disable the digital gain control in ExperimentalAgc. With the flag, Experimental Agc (henceforth AGC1) only controls the adaptive analog gain. This flag can be combined to that which activates AGC2. That way, one can enable the hybrid AGC configuration AGC1 analog only + AGC2 fixed+adaptive digital. Previously, there was a flag "use_agc2_digital_adaptive" in AgcManagerDirect. Our ambition was that to activate the hybrid mode described above with this flag. The behavior of the flag was not implemented. To activate the hybrid mode after this CL, set ExperimentalAgc::digital_adaptive_disabled=true and AudioProcessing::Config::GainController2::enabled=true. We also add flags for these settings in audioproc_f. Then the required settings are currently audioproc_f --agc2 1 --agc 1 --experimental_agc 1 \ --experimental_agc_disable_digital_adaptive 1 \ -i [INPUT] Bug: webrtc:7494 Change-Id: Iea798dc3899cec83d30ba71caba787262fcaef41 Reviewed-on: https://webrtc-review.googlesource.com/89740 Commit-Queue: Alex Loiko Reviewed-by: Alessio Bazzica Cr-Commit-Position: refs/heads/master@{#24249} --- modules/audio_processing/agc/BUILD.gn | 1 + .../agc/agc_manager_direct.cc | 63 ++++++++++--------- .../audio_processing/agc/agc_manager_direct.h | 10 ++- .../agc/agc_manager_direct_unittest.cc | 18 ++++++ modules/audio_processing/agc/mock_agc.h | 1 + .../audio_processing/audio_processing_impl.cc | 2 +- .../include/audio_processing.h | 6 +- .../test/audio_processing_simulator.cc | 4 +- .../test/audio_processing_simulator.h | 2 +- .../test/audioproc_float_impl.cc | 16 ++--- 10 files changed, 74 insertions(+), 49 deletions(-) diff --git a/modules/audio_processing/agc/BUILD.gn b/modules/audio_processing/agc/BUILD.gn index 9061989b54..65653a0d3e 100644 --- a/modules/audio_processing/agc/BUILD.gn +++ b/modules/audio_processing/agc/BUILD.gn @@ -21,6 +21,7 @@ rtc_source_set("agc") { "..:gain_control_interface", "../../..:webrtc_common", "../../../rtc_base:checks", + "../../../rtc_base:gtest_prod", "../../../rtc_base:logging", "../../../rtc_base:macromagic", "../../../rtc_base:safe_minmax", diff --git a/modules/audio_processing/agc/agc_manager_direct.cc b/modules/audio_processing/agc/agc_manager_direct.cc index 938683771b..5b5b9ea06b 100644 --- a/modules/audio_processing/agc/agc_manager_direct.cc +++ b/modules/audio_processing/agc/agc_manager_direct.cc @@ -84,6 +84,31 @@ int LevelFromGainError(int gain_error, int level) { return new_level; } +int InitializeGainControl(GainControl* gain_control, + bool disable_digital_adaptive) { + if (gain_control->set_mode(GainControl::kFixedDigital) != 0) { + RTC_LOG(LS_ERROR) << "set_mode(GainControl::kFixedDigital) failed."; + return -1; + } + const int target_level_dbfs = disable_digital_adaptive ? 0 : 2; + if (gain_control->set_target_level_dbfs(target_level_dbfs) != 0) { + RTC_LOG(LS_ERROR) << "set_target_level_dbfs() failed."; + return -1; + } + const int compression_gain_db = + disable_digital_adaptive ? 0 : kDefaultCompressionGain; + if (gain_control->set_compression_gain_db(compression_gain_db) != 0) { + RTC_LOG(LS_ERROR) << "set_compression_gain_db() failed."; + return -1; + } + const bool enable_limiter = !disable_digital_adaptive; + if (gain_control->enable_limiter(enable_limiter) != 0) { + RTC_LOG(LS_ERROR) << "enable_limiter() failed."; + return -1; + } + return 0; +} + } // namespace // Facility for dumping debug audio files. All methods are no-ops in the @@ -114,14 +139,14 @@ AgcManagerDirect::AgcManagerDirect(GainControl* gctrl, int startup_min_level, int clipped_level_min, bool use_agc2_level_estimation, - bool use_agc2_digital_adaptive) + bool disable_digital_adaptive) : AgcManagerDirect(use_agc2_level_estimation ? nullptr : new Agc(), gctrl, volume_callbacks, startup_min_level, clipped_level_min, use_agc2_level_estimation, - use_agc2_digital_adaptive) { + disable_digital_adaptive) { RTC_DCHECK(agc_); } @@ -146,7 +171,7 @@ AgcManagerDirect::AgcManagerDirect(Agc* agc, int startup_min_level, int clipped_level_min, bool use_agc2_level_estimation, - bool use_agc2_digital_adaptive) + bool disable_digital_adaptive) : data_dumper_(new ApmDataDumper(instance_counter_)), agc_(agc), gctrl_(gctrl), @@ -162,7 +187,7 @@ AgcManagerDirect::AgcManagerDirect(Agc* agc, check_volume_on_next_process_(true), // Check at startup. startup_(true), use_agc2_level_estimation_(use_agc2_level_estimation), - use_agc2_digital_adaptive_(use_agc2_digital_adaptive), + disable_digital_adaptive_(disable_digital_adaptive), startup_min_level_(ClampLevel(startup_min_level)), clipped_level_min_(clipped_level_min), file_preproc_(new DebugFile("agc_preproc.pcm")), @@ -174,9 +199,6 @@ AgcManagerDirect::AgcManagerDirect(Agc* agc, } else { RTC_DCHECK(agc); } - if (use_agc2_digital_adaptive_) { - RTC_NOTREACHED() << "Agc2 digital adaptive not implemented."; - } } AgcManagerDirect::~AgcManagerDirect() {} @@ -184,8 +206,8 @@ AgcManagerDirect::~AgcManagerDirect() {} int AgcManagerDirect::Initialize() { max_level_ = kMaxMicLevel; max_compression_gain_ = kMaxCompressionGain; - target_compression_ = kDefaultCompressionGain; - compression_ = target_compression_; + target_compression_ = disable_digital_adaptive_ ? 0 : kDefaultCompressionGain; + compression_ = disable_digital_adaptive_ ? 0 : target_compression_; compression_accumulator_ = compression_; capture_muted_ = false; check_volume_on_next_process_ = true; @@ -194,24 +216,7 @@ int AgcManagerDirect::Initialize() { data_dumper_->InitiateNewSetOfRecordings(); - if (gctrl_->set_mode(GainControl::kFixedDigital) != 0) { - RTC_LOG(LS_ERROR) << "set_mode(GainControl::kFixedDigital) failed."; - return -1; - } - if (gctrl_->set_target_level_dbfs(2) != 0) { - RTC_LOG(LS_ERROR) << "set_target_level_dbfs(2) failed."; - return -1; - } - if (gctrl_->set_compression_gain_db(kDefaultCompressionGain) != 0) { - RTC_LOG(LS_ERROR) - << "set_compression_gain_db(kDefaultCompressionGain) failed."; - return -1; - } - if (gctrl_->enable_limiter(true) != 0) { - RTC_LOG(LS_ERROR) << "enable_limiter(true) failed."; - return -1; - } - return 0; + return InitializeGainControl(gctrl_, disable_digital_adaptive_); } void AgcManagerDirect::AnalyzePreProcess(int16_t* audio, @@ -276,7 +281,9 @@ void AgcManagerDirect::Process(const int16_t* audio, agc_->Process(audio, length, sample_rate_hz); UpdateGain(); - UpdateCompressor(); + if (!disable_digital_adaptive_) { + UpdateCompressor(); + } file_postproc_->Write(audio, length); diff --git a/modules/audio_processing/agc/agc_manager_direct.h b/modules/audio_processing/agc/agc_manager_direct.h index de3d1cbd26..7d2a33011f 100644 --- a/modules/audio_processing/agc/agc_manager_direct.h +++ b/modules/audio_processing/agc/agc_manager_direct.h @@ -16,6 +16,7 @@ #include "modules/audio_processing/agc/agc.h" #include "modules/audio_processing/logging/apm_data_dumper.h" #include "rtc_base/constructormagic.h" +#include "rtc_base/gtest_prod_util.h" namespace webrtc { @@ -50,7 +51,7 @@ class AgcManagerDirect final { int startup_min_level, int clipped_level_min, bool use_agc2_level_estimation, - bool use_agc2_digital_adaptive); + bool disable_digital_adaptive); ~AgcManagerDirect(); @@ -71,6 +72,9 @@ class AgcManagerDirect final { private: friend class AgcManagerDirectTest; + FRIEND_TEST_ALL_PREFIXES(AgcManagerDirectStandaloneTest, + DisableDigitalDisablesDigital); + // Dependency injection for testing. Don't delete |agc| as the memory is owned // by the manager. AgcManagerDirect(Agc* agc, @@ -86,7 +90,7 @@ class AgcManagerDirect final { int startup_min_level, int clipped_level_min, bool use_agc2_level_estimation, - bool use_agc2_digital_adaptive); + bool disable_digital_adaptive); // Sets a new microphone level, after first checking that it hasn't been // updated by the user, in which case no action is taken. @@ -119,7 +123,7 @@ class AgcManagerDirect final { bool check_volume_on_next_process_; bool startup_; const bool use_agc2_level_estimation_; - const bool use_agc2_digital_adaptive_; + const bool disable_digital_adaptive_; int startup_min_level_; const int clipped_level_min_; diff --git a/modules/audio_processing/agc/agc_manager_direct_unittest.cc b/modules/audio_processing/agc/agc_manager_direct_unittest.cc index dae0769f6b..73ea55d06c 100644 --- a/modules/audio_processing/agc/agc_manager_direct_unittest.cc +++ b/modules/audio_processing/agc/agc_manager_direct_unittest.cc @@ -683,4 +683,22 @@ TEST_F(AgcManagerDirectTest, TakesNoActionOnZeroMicVolume) { EXPECT_EQ(0, volume_.GetMicVolume()); } +TEST(AgcManagerDirectStandaloneTest, DisableDigitalDisablesDigital) { + auto agc = std::unique_ptr(new testing::NiceMock()); + test::MockGainControl gctrl; + TestVolumeCallbacks volume; + + AgcManagerDirect manager(agc.release(), &gctrl, &volume, kInitialVolume, + kClippedMin, + /* use agc2 level estimation */ false, + /* disable digital adaptive */ true); + + EXPECT_CALL(gctrl, set_mode(GainControl::kFixedDigital)); + EXPECT_CALL(gctrl, set_target_level_dbfs(0)); + EXPECT_CALL(gctrl, set_compression_gain_db(0)); + EXPECT_CALL(gctrl, enable_limiter(false)); + + manager.Initialize(); +} + } // namespace webrtc diff --git a/modules/audio_processing/agc/mock_agc.h b/modules/audio_processing/agc/mock_agc.h index ff1e9fda32..4297e2a08b 100644 --- a/modules/audio_processing/agc/mock_agc.h +++ b/modules/audio_processing/agc/mock_agc.h @@ -19,6 +19,7 @@ namespace webrtc { class MockAgc : public Agc { public: + virtual ~MockAgc() {} MOCK_METHOD2(AnalyzePreproc, float(const int16_t* audio, size_t length)); MOCK_METHOD3(Process, void(const int16_t* audio, size_t length, int sample_rate_hz)); diff --git a/modules/audio_processing/audio_processing_impl.cc b/modules/audio_processing/audio_processing_impl.cc index c2cb5c205e..5e2bad31ae 100644 --- a/modules/audio_processing/audio_processing_impl.cc +++ b/modules/audio_processing/audio_processing_impl.cc @@ -373,7 +373,7 @@ AudioProcessingImpl::AudioProcessingImpl( #else config.Get().enabled, config.Get().enabled_agc2_level_estimator, - config.Get().enabled_agc2_digital_adaptive), + config.Get().digital_adaptive_disabled), #endif #if defined(WEBRTC_ANDROID) || defined(WEBRTC_IOS) capture_(false), diff --git a/modules/audio_processing/include/audio_processing.h b/modules/audio_processing/include/audio_processing.h index 291e531912..41b45aa9f5 100644 --- a/modules/audio_processing/include/audio_processing.h +++ b/modules/audio_processing/include/audio_processing.h @@ -121,10 +121,10 @@ struct ExperimentalAgc { explicit ExperimentalAgc(bool enabled) : enabled(enabled) {} ExperimentalAgc(bool enabled, bool enabled_agc2_level_estimator, - bool enabled_agc2_digital_adaptive) + bool digital_adaptive_disabled) : enabled(enabled), enabled_agc2_level_estimator(enabled_agc2_level_estimator), - enabled_agc2_digital_adaptive(enabled_agc2_digital_adaptive) {} + digital_adaptive_disabled(digital_adaptive_disabled) {} ExperimentalAgc(bool enabled, int startup_min_volume) : enabled(enabled), startup_min_volume(startup_min_volume) {} @@ -138,7 +138,7 @@ struct ExperimentalAgc { // Lowest microphone level that will be applied in response to clipping. int clipped_level_min = kClippedLevelMin; bool enabled_agc2_level_estimator = false; - bool enabled_agc2_digital_adaptive = false; + bool digital_adaptive_disabled = false; }; // Use to enable experimental noise suppression. It can be set in the diff --git a/modules/audio_processing/test/audio_processing_simulator.cc b/modules/audio_processing/test/audio_processing_simulator.cc index e9ca1518e8..ba827b15c9 100644 --- a/modules/audio_processing/test/audio_processing_simulator.cc +++ b/modules/audio_processing/test/audio_processing_simulator.cc @@ -618,8 +618,8 @@ void AudioProcessingSimulator::CreateAudioProcessor() { !settings_.use_experimental_agc || *settings_.use_experimental_agc, !!settings_.use_experimental_agc_agc2_level_estimator && *settings_.use_experimental_agc_agc2_level_estimator, - !!settings_.use_experimental_agc_agc2_digital_adaptive && - *settings_.use_experimental_agc_agc2_digital_adaptive)); + !!settings_.experimental_agc_disable_digital_adaptive && + *settings_.experimental_agc_disable_digital_adaptive)); 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 b0cf0fdf88..e63cc468dc 100644 --- a/modules/audio_processing/test/audio_processing_simulator.h +++ b/modules/audio_processing/test/audio_processing_simulator.h @@ -67,7 +67,7 @@ struct SimulationSettings { absl::optional use_aec3; absl::optional use_experimental_agc; absl::optional use_experimental_agc_agc2_level_estimator; - absl::optional use_experimental_agc_agc2_digital_adaptive; + absl::optional experimental_agc_disable_digital_adaptive; absl::optional aecm_routing_mode; absl::optional use_aecm_comfort_noise; absl::optional agc_mode; diff --git a/modules/audio_processing/test/audioproc_float_impl.cc b/modules/audio_processing/test/audioproc_float_impl.cc index 346a590935..47576692bd 100644 --- a/modules/audio_processing/test/audioproc_float_impl.cc +++ b/modules/audio_processing/test/audioproc_float_impl.cc @@ -118,14 +118,10 @@ DEFINE_int(aec3, DEFINE_int(experimental_agc, kParameterNotSpecifiedValue, "Activate (1) or deactivate(0) the experimental AGC"); -DEFINE_int( - experimental_agc_agc2_level_estimator, - kParameterNotSpecifiedValue, - "Activate (1) or deactivate(0) AGC2 level estimate in experimental AGC"); -DEFINE_int(experimental_agc_agc2_digital_adaptive, +DEFINE_int(experimental_agc_disable_digital_adaptive, kParameterNotSpecifiedValue, - "Activate (1) or deactivate(0) AGC2 digital adaptation in " - "experimental AGC"); + "Force-deactivate (1) digital adaptation in " + "experimental AGC. Digital adaptation is active by default (0)."); DEFINE_int( refined_adaptive_filter, kParameterNotSpecifiedValue, @@ -264,10 +260,8 @@ SimulationSettings CreateSettings() { SetSettingIfFlagSet(FLAG_aec3, &settings.use_aec3); SetSettingIfFlagSet(FLAG_experimental_agc, &settings.use_experimental_agc); - SetSettingIfFlagSet(FLAG_experimental_agc_agc2_level_estimator, - &settings.use_experimental_agc_agc2_level_estimator); - SetSettingIfFlagSet(FLAG_experimental_agc_agc2_digital_adaptive, - &settings.use_experimental_agc_agc2_digital_adaptive); + SetSettingIfFlagSet(FLAG_experimental_agc_disable_digital_adaptive, + &settings.experimental_agc_disable_digital_adaptive); SetSettingIfSpecified(FLAG_aecm_routing_mode, &settings.aecm_routing_mode); SetSettingIfFlagSet(FLAG_aecm_comfort_noise,