Move AGC2 config ToString to the right place and update Validate()

The APM config to string mapping must be in one place (namely,
in `audio_processing.cc`). This CL moves the AGC2 config to string
impl to the right place.

This CL also updates `GainController2::Validate()` and adds the
missing unit tests for the parameters that have recently been added.

Stack buffer size in `AudioProcessing::Config::ToString()` increased
because of the extra params. Syntax near `multi_channel_capture` fixed.
Output string format verified with a JS linter.

Bug: webrtc:7494
Change-Id: I692e1549b7d40c970d88a14c8e83da16325fb54c
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/187080
Commit-Queue: Alessio Bazzica <alessiob@webrtc.org>
Reviewed-by: Minyue Li <minyue@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#32400}

modules/audio_processing/audio_processing_impl.cc

@@ -611,10 +611,8 @@ void AudioProcessingImpl::ApplyConfig(const AudioProcessing::Config& config) {
 
   const bool config_ok = GainController2::Validate(config_.gain_controller2);
   if (!config_ok) {
-    RTC_LOG(LS_ERROR) << "AudioProcessing module config error\n"
+    RTC_LOG(LS_ERROR)
-                         "Gain Controller 2: "
+        << "Invalid Gain Controller 2 config; using the default config.";
-                      << GainController2::ToString(config_.gain_controller2)
-                      << "\nReverting to default parameter set";
     config_.gain_controller2 = AudioProcessing::Config::GainController2();
   }
 

modules/audio_processing/gain_controller2.cc

@@ -65,8 +65,7 @@ void GainController2::NotifyAnalogLevel(int level) {
 
 void GainController2::ApplyConfig(
     const AudioProcessing::Config::GainController2& config) {
-  RTC_DCHECK(Validate(config))
+  RTC_DCHECK(Validate(config));
-      << " the invalid config was " << ToString(config);
 
   config_ = config;
   if (config.fixed_digital.gain_db != config_.fixed_digital.gain_db) {
@@ -84,55 +83,19 @@ void GainController2::ApplyConfig(
 
 bool GainController2::Validate(
     const AudioProcessing::Config::GainController2& config) {
-  return config.fixed_digital.gain_db >= 0.f &&
+  const auto& fixed = config.fixed_digital;
-         config.fixed_digital.gain_db < 50.f &&
+  const auto& adaptive = config.adaptive_digital;
-         config.adaptive_digital.extra_saturation_margin_db >= 0.f &&
+  return fixed.gain_db >= 0.f && fixed.gain_db < 50.f &&
-         config.adaptive_digital.extra_saturation_margin_db <= 100.f;
+         adaptive.vad_probability_attack > 0.f &&
-}
+         adaptive.vad_probability_attack <= 1.f &&
-
+         adaptive.level_estimator_adjacent_speech_frames_threshold >= 1 &&
-std::string GainController2::ToString(
+         adaptive.initial_saturation_margin_db >= 0.f &&
-    const AudioProcessing::Config::GainController2& config) {
+         adaptive.initial_saturation_margin_db <= 100.f &&
-  rtc::StringBuilder ss;
+         adaptive.extra_saturation_margin_db >= 0.f &&
-  std::string adaptive_digital_level_estimator;
+         adaptive.extra_saturation_margin_db <= 100.f &&
-  using LevelEstimatorType =
+         adaptive.gain_applier_adjacent_speech_frames_threshold >= 1 &&
-      AudioProcessing::Config::GainController2::LevelEstimator;
+         adaptive.max_gain_change_db_per_second > 0.f &&
-  switch (config.adaptive_digital.level_estimator) {
+         adaptive.max_output_noise_level_dbfs <= 0.f;
-    case LevelEstimatorType::kRms:
-      adaptive_digital_level_estimator = "RMS";
-      break;
-    case LevelEstimatorType::kPeak:
-      adaptive_digital_level_estimator = "peak";
-      break;
-  }
-  // clang-format off
-  // clang formatting doesn't respect custom nested style.
-  ss << "{"
-        "enabled: " << (config.enabled ? "true" : "false") << ", "
-        "fixed_digital: {gain_db: " << config.fixed_digital.gain_db << "}, "
-        "adaptive_digital: {"
-          "enabled: "
-            << (config.adaptive_digital.enabled ? "true" : "false") << ", "
-          "level_estimator: {"
-            "type: " << adaptive_digital_level_estimator << ", "
-            "adjacent_speech_frames_threshold: "
-              << config.adaptive_digital
-                  .level_estimator_adjacent_speech_frames_threshold << ", "
-            "initial_saturation_margin_db: "
-              << config.adaptive_digital.initial_saturation_margin_db << ", "
-            "extra_saturation_margin_db: "
-              << config.adaptive_digital.extra_saturation_margin_db << "}, "
-          "gain_applier: {"
-            "adjacent_speech_frames_threshold: "
-              << config.adaptive_digital
-                  .gain_applier_adjacent_speech_frames_threshold << ", "
-            "max_gain_change_db_per_second: "
-              << config.adaptive_digital.max_gain_change_db_per_second << ", "
-            "max_output_noise_level_dbfs: "
-              << config.adaptive_digital.max_output_noise_level_dbfs << "}"
-        "}"
-        "}";
-  // clang-format on
-  return ss.Release();
 }
 
 }  // namespace webrtc

modules/audio_processing/gain_controller2.h

@@ -38,8 +38,6 @@ class GainController2 {
 
   void ApplyConfig(const AudioProcessing::Config::GainController2& config);
   static bool Validate(const AudioProcessing::Config::GainController2& config);
-  static std::string ToString(
-      const AudioProcessing::Config::GainController2& config);
 
  private:
   static int instance_count_;

modules/audio_processing/gain_controller2_unittest.cc

@@ -104,36 +104,107 @@ float GainAfterProcessingFile(GainController2* gain_controller) {
 
 }  // namespace
 
-TEST(GainController2, CreateApplyConfig) {
+TEST(GainController2, CheckDefaultConfig) {
-  // Instances GainController2 and applies different configurations.
-  std::unique_ptr<GainController2> gain_controller2(new GainController2());
-
-  // Check that the default config is valid.
   AudioProcessing::Config::GainController2 config;
   EXPECT_TRUE(GainController2::Validate(config));
-  gain_controller2->ApplyConfig(config);
-
-  // Check that attenuation is not allowed.
-  config.fixed_digital.gain_db = -5.f;
-  EXPECT_FALSE(GainController2::Validate(config));
-
-  // Check that valid configurations are applied.
-  for (const float& fixed_gain_db : {0.f, 5.f, 10.f, 40.f}) {
-    config.fixed_digital.gain_db = fixed_gain_db;
-    EXPECT_TRUE(GainController2::Validate(config));
-    gain_controller2->ApplyConfig(config);
-  }
 }
 
-TEST(GainController2, ToString) {
+TEST(GainController2, CheckFixedDigitalConfig) {
-  // Tests GainController2::ToString(). Only test the enabled property.
   AudioProcessing::Config::GainController2 config;
+  // Attenuation is not allowed.
+  config.fixed_digital.gain_db = -5.f;
+  EXPECT_FALSE(GainController2::Validate(config));
+  // No gain is allowed.
+  config.fixed_digital.gain_db = 0.f;
+  EXPECT_TRUE(GainController2::Validate(config));
+  // Positive gain is allowed.
+  config.fixed_digital.gain_db = 15.f;
+  EXPECT_TRUE(GainController2::Validate(config));
+}
 
-  config.enabled = false;
+TEST(GainController2, CheckAdaptiveDigitalVadProbabilityAttackConfig) {
-  EXPECT_EQ("{enabled: false", GainController2::ToString(config).substr(0, 15));
+  AudioProcessing::Config::GainController2 config;
+  // Reject invalid attack.
+  config.adaptive_digital.vad_probability_attack = -123.f;
+  EXPECT_FALSE(GainController2::Validate(config));
+  config.adaptive_digital.vad_probability_attack = 0.f;
+  EXPECT_FALSE(GainController2::Validate(config));
+  config.adaptive_digital.vad_probability_attack = 42.f;
+  EXPECT_FALSE(GainController2::Validate(config));
+  // Accept valid attack.
+  config.adaptive_digital.vad_probability_attack = 0.1f;
+  EXPECT_TRUE(GainController2::Validate(config));
+  config.adaptive_digital.vad_probability_attack = 1.f;
+  EXPECT_TRUE(GainController2::Validate(config));
+}
 
-  config.enabled = true;
+TEST(GainController2,
-  EXPECT_EQ("{enabled: true", GainController2::ToString(config).substr(0, 14));
+     CheckAdaptiveDigitalLevelEstimatorSpeechFramesThresholdConfig) {
+  AudioProcessing::Config::GainController2 config;
+  config.adaptive_digital.level_estimator_adjacent_speech_frames_threshold = 0;
+  EXPECT_FALSE(GainController2::Validate(config));
+  config.adaptive_digital.level_estimator_adjacent_speech_frames_threshold = 1;
+  EXPECT_TRUE(GainController2::Validate(config));
+  config.adaptive_digital.level_estimator_adjacent_speech_frames_threshold = 7;
+  EXPECT_TRUE(GainController2::Validate(config));
+}
+
+TEST(GainController2, CheckAdaptiveDigitalInitialSaturationMarginConfig) {
+  AudioProcessing::Config::GainController2 config;
+  config.adaptive_digital.initial_saturation_margin_db = -1.f;
+  EXPECT_FALSE(GainController2::Validate(config));
+  config.adaptive_digital.initial_saturation_margin_db = 0.f;
+  EXPECT_TRUE(GainController2::Validate(config));
+  config.adaptive_digital.initial_saturation_margin_db = 50.f;
+  EXPECT_TRUE(GainController2::Validate(config));
+}
+
+TEST(GainController2, CheckAdaptiveDigitalExtraSaturationMarginConfig) {
+  AudioProcessing::Config::GainController2 config;
+  config.adaptive_digital.extra_saturation_margin_db = -1.f;
+  EXPECT_FALSE(GainController2::Validate(config));
+  config.adaptive_digital.extra_saturation_margin_db = 0.f;
+  EXPECT_TRUE(GainController2::Validate(config));
+  config.adaptive_digital.extra_saturation_margin_db = 50.f;
+  EXPECT_TRUE(GainController2::Validate(config));
+}
+
+TEST(GainController2,
+     CheckAdaptiveDigitalGainApplierSpeechFramesThresholdConfig) {
+  AudioProcessing::Config::GainController2 config;
+  config.adaptive_digital.gain_applier_adjacent_speech_frames_threshold = 0;
+  EXPECT_FALSE(GainController2::Validate(config));
+  config.adaptive_digital.gain_applier_adjacent_speech_frames_threshold = 1;
+  EXPECT_TRUE(GainController2::Validate(config));
+  config.adaptive_digital.gain_applier_adjacent_speech_frames_threshold = 7;
+  EXPECT_TRUE(GainController2::Validate(config));
+}
+
+TEST(GainController2, CheckAdaptiveDigitalMaxGainChangeSpeedConfig) {
+  AudioProcessing::Config::GainController2 config;
+  config.adaptive_digital.max_gain_change_db_per_second = -1.f;
+  EXPECT_FALSE(GainController2::Validate(config));
+  config.adaptive_digital.max_gain_change_db_per_second = 0.f;
+  EXPECT_FALSE(GainController2::Validate(config));
+  config.adaptive_digital.max_gain_change_db_per_second = 5.f;
+  EXPECT_TRUE(GainController2::Validate(config));
+}
+
+TEST(GainController2, CheckAdaptiveDigitalMaxOutputNoiseLevelConfig) {
+  AudioProcessing::Config::GainController2 config;
+  config.adaptive_digital.max_output_noise_level_dbfs = 5.f;
+  EXPECT_FALSE(GainController2::Validate(config));
+  config.adaptive_digital.max_output_noise_level_dbfs = 0.f;
+  EXPECT_TRUE(GainController2::Validate(config));
+  config.adaptive_digital.max_output_noise_level_dbfs = -5.f;
+  EXPECT_TRUE(GainController2::Validate(config));
+}
+
+// Checks that the default config is applied.
+TEST(GainController2, ApplyDefaultConfig) {
+  auto gain_controller2 = std::make_unique<GainController2>();
+  AudioProcessing::Config::GainController2 config;
+  gain_controller2->ApplyConfig(config);
 }
 
 TEST(GainController2FixedDigital, GainShouldChangeOnSetGain) {

modules/audio_processing/include/audio_processing.cc

@@ -71,19 +71,15 @@ AudioProcessing::Config::Pipeline::Pipeline()
     : maximum_internal_processing_rate(GetDefaultMaxInternalRate()) {}
 
 std::string AudioProcessing::Config::ToString() const {
-  char buf[1024];
+  char buf[2048];
   rtc::SimpleStringBuilder builder(buf);
   builder << "AudioProcessing::Config{ "
              "pipeline: {"
              "maximum_internal_processing_rate: "
           << pipeline.maximum_internal_processing_rate
           << ", multi_channel_render: " << pipeline.multi_channel_render
-          << ", "
+          << ", multi_channel_capture: " << pipeline.multi_channel_capture
-             ", multi_channel_capture: "
+          << "}, pre_amplifier: { enabled: " << pre_amplifier.enabled
-          << pipeline.multi_channel_capture
-          << "}, "
-             "pre_amplifier: { enabled: "
-          << pre_amplifier.enabled
           << ", fixed_gain_factor: " << pre_amplifier.fixed_gain_factor
           << " }, high_pass_filter: { enabled: " << high_pass_filter.enabled
           << " }, echo_canceller: { enabled: " << echo_canceller.enabled
@@ -106,18 +102,29 @@ std::string AudioProcessing::Config::ToString() const {
           << " }, gain_controller2: { enabled: " << gain_controller2.enabled
           << ", fixed_digital: { gain_db: "
           << gain_controller2.fixed_digital.gain_db
-          << " }, adaptive_digital: { enabled: "
+          << "}, adaptive_digital: { enabled: "
-          << gain_controller2.adaptive_digital.enabled << ", level_estimator: "
+          << gain_controller2.adaptive_digital.enabled
+          << ", level_estimator: { type: "
           << GainController2LevelEstimatorToString(
                  gain_controller2.adaptive_digital.level_estimator)
-          << ", use_saturation_protector: "
+          << ", adjacent_speech_frames_threshold: "
-          << gain_controller2.adaptive_digital.use_saturation_protector
+          << gain_controller2.adaptive_digital
+                 .level_estimator_adjacent_speech_frames_threshold
+          << ", initial_saturation_margin_db: "
+          << gain_controller2.adaptive_digital.initial_saturation_margin_db
           << ", extra_saturation_margin_db: "
           << gain_controller2.adaptive_digital.extra_saturation_margin_db
+          << "}, gain_applier: { adjacent_speech_frames_threshold: "
+          << gain_controller2.adaptive_digital
+                 .gain_applier_adjacent_speech_frames_threshold
+          << ", max_gain_change_db_per_second: "
+          << gain_controller2.adaptive_digital.max_gain_change_db_per_second
+          << ", max_output_noise_level_dbfs: "
+          << gain_controller2.adaptive_digital.max_output_noise_level_dbfs
           << " } }, residual_echo_detector: { enabled: "
           << residual_echo_detector.enabled
           << " }, level_estimation: { enabled: " << level_estimation.enabled
-          << " } }";
+          << " }}}";
   return builder.str();
 }