From e76daab8b36f8c2a16d59a116425a3a2f98022f6 Mon Sep 17 00:00:00 2001
From: Alessio Bazzica <alessiob@webrtc.org>
Date: Tue, 5 Jul 2022 16:47:26 +0200
Subject: [PATCH] `AgcManagerDirect`: stop enforcing min mic level override
 with 0 level

https://webrtc-review.googlesource.com/c/src/+/250141 introduced a bug
due to which the min mic level override is always enforced, if specified
even if the user manually adjusts the mic level to zero.

This CL fixes that bug, the changes run behind a kill switch.

TESTED=Test video call on Chromium on Mac; input volume not adjusted after zeroing it from the system preferences UI

Bug: chromium:1275566
Change-Id: I18ce2e5970d3002b301f51f84544583c64982d57
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/267844
Reviewed-by: Hanna Silen <silen@webrtc.org>
Commit-Queue: Alessio Bazzica <alessiob@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#37460}
---
 .../agc/agc_manager_direct.cc                 | 24 +++++++++++++++----
 .../audio_processing/agc/agc_manager_direct.h |  1 +
 .../agc/agc_manager_direct_unittest.cc        | 14 -----------
 3 files changed, 20 insertions(+), 19 deletions(-)

diff --git a/modules/audio_processing/agc/agc_manager_direct.cc b/modules/audio_processing/agc/agc_manager_direct.cc
index 73bf183a2b..f385438249 100644
--- a/modules/audio_processing/agc/agc_manager_direct.cc
+++ b/modules/audio_processing/agc/agc_manager_direct.cc
@@ -62,12 +62,23 @@ bool UseMaxAnalogChannelLevel() {
   return field_trial::IsEnabled("WebRTC-UseMaxAnalogAgcChannelLevel");
 }
 
-// Minimum mic level override. If specified, the override replaces
-// `kMinMicLevel` and it is applied even when clipping is detected.
+// Minimum mic level override.
+// `kMinMicLevelOverride`: if specified, the override replaces `kMinMicLevel`
+// and it is applied even when clipping is detected.
+// `EnforceMinMicLevelOverrideOnZeroLevel()`: returns true if the analog
+// controller must enforce the minimum mic level override even when the mic
+// level has manually been set to zero.
 #if defined(WEBRTC_MAC)
 constexpr absl::optional<int> kMinMicLevelOverride = 20;
+bool EnforceMinMicLevelOverrideOnZeroLevel() {
+  return field_trial::IsEnabled(
+      "WebRTC-Audio-AgcAnalogFixZeroMicLevelBugKillSwitch");
+}
 #else
 constexpr absl::optional<int> kMinMicLevelOverride = absl::nullopt;
+constexpr bool EnforceMinMicLevelOverrideOnZeroLevel() {
+  return false;
+}
 #endif
 
 int ClampLevel(int mic_level, int min_mic_level) {
@@ -455,6 +466,8 @@ AgcManagerDirect::AgcManagerDirect(
     int clipped_wait_frames,
     const ClippingPredictorConfig& clipping_config)
     : min_mic_level_override_(kMinMicLevelOverride),
+      enforce_min_mic_level_override_on_zero_level_(
+          EnforceMinMicLevelOverrideOnZeroLevel()),
       data_dumper_(new ApmDataDumper(instance_counter_.fetch_add(1) + 1)),
       use_min_channel_level_(!UseMaxAnalogChannelLevel()),
       num_capture_channels_(num_capture_channels),
@@ -706,9 +719,10 @@ void AgcManagerDirect::AggregateChannelLevels() {
       }
     }
   }
-  // TODO(crbug.com/1275566): Do not enforce minimum if the user has manually
-  // set the mic level to zero.
-  if (min_mic_level_override_.has_value()) {
+
+  if (min_mic_level_override_.has_value() &&
+      (enforce_min_mic_level_override_on_zero_level_ ||
+       stream_analog_level_ > 0)) {
     stream_analog_level_ =
         std::max(stream_analog_level_, *min_mic_level_override_);
   }
diff --git a/modules/audio_processing/agc/agc_manager_direct.h b/modules/audio_processing/agc/agc_manager_direct.h
index 86abeecb91..a437206ceb 100644
--- a/modules/audio_processing/agc/agc_manager_direct.h
+++ b/modules/audio_processing/agc/agc_manager_direct.h
@@ -128,6 +128,7 @@ class AgcManagerDirect final {
   void AggregateChannelLevels();
 
   const absl::optional<int> min_mic_level_override_;
+  const bool enforce_min_mic_level_override_on_zero_level_;
   std::unique_ptr<ApmDataDumper> data_dumper_;
   static std::atomic<int> instance_counter_;
   const bool use_min_channel_level_;
diff --git a/modules/audio_processing/agc/agc_manager_direct_unittest.cc b/modules/audio_processing/agc/agc_manager_direct_unittest.cc
index dce6aaa675..adbb2f272c 100644
--- a/modules/audio_processing/agc/agc_manager_direct_unittest.cc
+++ b/modules/audio_processing/agc/agc_manager_direct_unittest.cc
@@ -838,19 +838,6 @@ TEST_F(AgcManagerDirectTest, ClippingDoesNotPullLowVolumeBackUp) {
   EXPECT_EQ(initial_volume, manager_.stream_analog_level());
 }
 
-#if defined(WEBRTC_MAC)
-// TODO(crbug.com/1275566): Fix AGC, remove test below.
-TEST_F(AgcManagerDirectTest, BumpsToMinLevelOnZeroMicVolume) {
-  FirstProcess();
-
-  EXPECT_CALL(*agc_, GetRmsErrorDb(_))
-      .WillRepeatedly(DoAll(SetArgPointee<0>(30), Return(true)));
-  manager_.set_stream_analog_level(0);
-  CallProcess(10);
-  EXPECT_EQ(20, manager_.stream_analog_level());
-}
-#else
-// TODO(crbug.com/1275566): Fix AGC, reenable test below on Mac.
 TEST_F(AgcManagerDirectTest, TakesNoActionOnZeroMicVolume) {
   FirstProcess();
 
@@ -860,7 +847,6 @@ TEST_F(AgcManagerDirectTest, TakesNoActionOnZeroMicVolume) {
   CallProcess(10);
   EXPECT_EQ(0, manager_.stream_analog_level());
 }
-#endif
 
 TEST_F(AgcManagerDirectTest, ClippingDetectionLowersVolume) {
   SetVolumeAndProcess(255);