Min mic analog level: override minimum and behavior on Mac
This CL removes the `WebRTC-Audio-AgcMinMicLevelExperiment` field trial
and always enables the code path behind that flag on Mac. In summary,
the analog AGC behaves as follows on Mac:
1. the minimum level is overridden to 20
2. the minimum is applied even when clipping is detected
3. when the level is manually adjusted to 0, the minimum level is
enforced - i.e., 20
Note that the 3rd property had been unintentionally added when the
changes were added behind the aforementioned field trial. This will
be fixed in a follow-up CL.
Bug: chromium:1275566
Change-Id: If184c4455a0780fcd94f55141af34460c152e3c3
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/266488
Commit-Queue: Alessio Bazzica <alessiob@webrtc.org>
Reviewed-by: Hanna Silen <silen@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#37459}
diff --git a/modules/audio_processing/agc/agc_manager_direct.cc b/modules/audio_processing/agc/agc_manager_direct.cc
index af25c61..73bf183 100644
--- a/modules/audio_processing/agc/agc_manager_direct.cc
+++ b/modules/audio_processing/agc/agc_manager_direct.cc
@@ -62,29 +62,13 @@
return field_trial::IsEnabled("WebRTC-UseMaxAnalogAgcChannelLevel");
}
-// If the "WebRTC-Audio-AgcMinMicLevelExperiment" field trial is specified,
-// parses it and returns a value between 0 and 255 depending on the field-trial
-// string. Returns an unspecified value if the field trial is not specified, if
-// disabled or if it cannot be parsed. Example:
-// 'WebRTC-Audio-AgcMinMicLevelExperiment/Enabled-80' => returns 80.
-absl::optional<int> GetMinMicLevelOverride() {
- constexpr char kMinMicLevelFieldTrial[] =
- "WebRTC-Audio-AgcMinMicLevelExperiment";
- if (!webrtc::field_trial::IsEnabled(kMinMicLevelFieldTrial)) {
- return absl::nullopt;
- }
- const auto field_trial_string =
- webrtc::field_trial::FindFullName(kMinMicLevelFieldTrial);
- int min_mic_level = -1;
- sscanf(field_trial_string.c_str(), "Enabled-%d", &min_mic_level);
- if (min_mic_level >= 0 && min_mic_level <= 255) {
- return min_mic_level;
- } else {
- RTC_LOG(LS_WARNING) << "[agc] Invalid parameter for "
- << kMinMicLevelFieldTrial << ", ignored.";
- return absl::nullopt;
- }
-}
+// Minimum mic level override. If specified, the override replaces
+// `kMinMicLevel` and it is applied even when clipping is detected.
+#if defined(WEBRTC_MAC)
+constexpr absl::optional<int> kMinMicLevelOverride = 20;
+#else
+constexpr absl::optional<int> kMinMicLevelOverride = absl::nullopt;
+#endif
int ClampLevel(int mic_level, int min_mic_level) {
return rtc::SafeClamp(mic_level, min_mic_level, kMaxMicLevel);
@@ -470,7 +454,7 @@
float clipped_ratio_threshold,
int clipped_wait_frames,
const ClippingPredictorConfig& clipping_config)
- : min_mic_level_override_(GetMinMicLevelOverride()),
+ : min_mic_level_override_(kMinMicLevelOverride),
data_dumper_(new ApmDataDumper(instance_counter_.fetch_add(1) + 1)),
use_min_channel_level_(!UseMaxAnalogChannelLevel()),
num_capture_channels_(num_capture_channels),
@@ -722,6 +706,8 @@
}
}
}
+ // 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()) {
stream_analog_level_ =
std::max(stream_analog_level_, *min_mic_level_override_);
diff --git a/modules/audio_processing/agc/agc_manager_direct_unittest.cc b/modules/audio_processing/agc/agc_manager_direct_unittest.cc
index 5c9b383..dce6aaa 100644
--- a/modules/audio_processing/agc/agc_manager_direct_unittest.cc
+++ b/modules/audio_processing/agc/agc_manager_direct_unittest.cc
@@ -35,7 +35,6 @@
constexpr int kInitialVolume = 128;
constexpr int kClippedMin = 165; // Arbitrary, but different from the default.
constexpr float kAboveClippedThreshold = 0.2f;
-constexpr int kMinMicLevel = 12;
constexpr int kClippedLevelStep = 15;
constexpr float kClippedRatioThreshold = 0.1f;
constexpr int kClippedWaitFrames = 300;
@@ -128,18 +127,6 @@
}
}
-std::string GetAgcMinMicLevelExperimentFieldTrial(
- int enabled_value,
- const std::string& suffix = "") {
- RTC_DCHECK_GE(enabled_value, 0);
- RTC_DCHECK_LE(enabled_value, 255);
- char field_trial_buffer[64];
- rtc::SimpleStringBuilder builder(field_trial_buffer);
- builder << "WebRTC-Audio-AgcMinMicLevelExperiment/Enabled-" << enabled_value
- << suffix << "/";
- return builder.str();
-}
-
// (Over)writes `samples_value` for the samples in `audio_buffer`.
// When `clipped_ratio`, a value in [0, 1], is greater than 0, the corresponding
// fraction of the frame is set to a full scale value to simulate clipping.
@@ -164,15 +151,6 @@
}
}
-void CallPreProcessAndProcess(int num_calls,
- const AudioBuffer& audio_buffer,
- AgcManagerDirect& manager) {
- for (int n = 0; n < num_calls; ++n) {
- manager.AnalyzePreProcess(&audio_buffer);
- manager.Process(&audio_buffer);
- }
-}
-
} // namespace
class AgcManagerDirectTest : public ::testing::Test {
@@ -391,13 +369,21 @@
EXPECT_CALL(*agc_, GetRmsErrorDb(_))
.WillOnce(DoAll(SetArgPointee<0>(-40), Return(true)));
CallProcess(1);
+#if defined(WEBRTC_MAC)
+ EXPECT_EQ(20, manager_.stream_analog_level());
+#else
EXPECT_EQ(18, manager_.stream_analog_level());
+#endif
// Won't go lower than the minimum.
EXPECT_CALL(*agc_, GetRmsErrorDb(_))
.WillOnce(DoAll(SetArgPointee<0>(-40), Return(true)));
CallProcess(1);
+#if defined(WEBRTC_MAC)
+ EXPECT_EQ(20, manager_.stream_analog_level());
+#else
EXPECT_EQ(12, manager_.stream_analog_level());
+#endif
}
TEST_F(AgcManagerDirectTest, CompressorStepsTowardsTarget) {
@@ -549,7 +535,11 @@
ExpectCheckVolumeAndReset(11);
EXPECT_CALL(*agc_, GetRmsErrorDb(_)).WillOnce(Return(false));
CallProcess(1);
+#if defined(WEBRTC_MAC)
+ EXPECT_EQ(20, manager_.stream_analog_level());
+#else
EXPECT_EQ(12, manager_.stream_analog_level());
+#endif
}
TEST_F(AgcManagerDirectTest, ManualLevelChangeResultsInNoSetMicCall) {
@@ -624,23 +614,39 @@
manager_.set_stream_analog_level(1);
EXPECT_CALL(*agc_, Reset()).Times(AtLeast(1));
CallProcess(1);
+#if defined(WEBRTC_MAC)
+ EXPECT_EQ(20, manager_.stream_analog_level());
+#else
EXPECT_EQ(1, manager_.stream_analog_level());
+#endif
// Continues working as usual afterwards.
EXPECT_CALL(*agc_, GetRmsErrorDb(_))
.WillOnce(DoAll(SetArgPointee<0>(11), Return(true)));
CallProcess(1);
+#if defined(WEBRTC_MAC)
+ EXPECT_EQ(20, manager_.stream_analog_level());
+#else
EXPECT_EQ(2, manager_.stream_analog_level());
+#endif
EXPECT_CALL(*agc_, GetRmsErrorDb(_))
.WillOnce(DoAll(SetArgPointee<0>(30), Return(true)));
CallProcess(1);
+#if defined(WEBRTC_MAC)
+ EXPECT_EQ(20, manager_.stream_analog_level());
+#else
EXPECT_EQ(11, manager_.stream_analog_level());
+#endif
EXPECT_CALL(*agc_, GetRmsErrorDb(_))
.WillOnce(DoAll(SetArgPointee<0>(20), Return(true)));
CallProcess(1);
+#if defined(WEBRTC_MAC)
+ EXPECT_EQ(20, manager_.stream_analog_level());
+#else
EXPECT_EQ(18, manager_.stream_analog_level());
+#endif
}
TEST_F(AgcManagerDirectTest, NoClippingHasNoImpact) {
@@ -832,6 +838,19 @@
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();
@@ -841,6 +860,7 @@
CallProcess(10);
EXPECT_EQ(0, manager_.stream_analog_level());
}
+#endif
TEST_F(AgcManagerDirectTest, ClippingDetectionLowersVolume) {
SetVolumeAndProcess(255);
@@ -876,175 +896,6 @@
manager->SetupDigitalGainControl(&gctrl);
}
-TEST(AgcManagerDirectStandaloneTest, AgcMinMicLevelExperiment) {
- std::unique_ptr<AgcManagerDirect> manager =
- CreateAgcManagerDirect(kInitialVolume, kClippedLevelStep,
- kClippedRatioThreshold, kClippedWaitFrames);
- EXPECT_EQ(manager->channel_agcs_[0]->min_mic_level(), kMinMicLevel);
- EXPECT_EQ(manager->channel_agcs_[0]->startup_min_level(), kInitialVolume);
-}
-
-TEST(AgcManagerDirectStandaloneTest, AgcMinMicLevelExperimentDisabled) {
- for (const std::string& field_trial_suffix : {"", "_20220210"}) {
- test::ScopedFieldTrials field_trial(
- "WebRTC-Audio-AgcMinMicLevelExperiment/Disabled" + field_trial_suffix +
- "/");
- std::unique_ptr<AgcManagerDirect> manager =
- CreateAgcManagerDirect(kInitialVolume, kClippedLevelStep,
- kClippedRatioThreshold, kClippedWaitFrames);
- EXPECT_EQ(manager->channel_agcs_[0]->min_mic_level(), kMinMicLevel);
- EXPECT_EQ(manager->channel_agcs_[0]->startup_min_level(), kInitialVolume);
- }
-}
-
-// Checks that a field-trial parameter outside of the valid range [0,255] is
-// ignored.
-TEST(AgcManagerDirectStandaloneTest, AgcMinMicLevelExperimentOutOfRangeAbove) {
- test::ScopedFieldTrials field_trial(
- "WebRTC-Audio-AgcMinMicLevelExperiment/Enabled-256/");
- std::unique_ptr<AgcManagerDirect> manager =
- CreateAgcManagerDirect(kInitialVolume, kClippedLevelStep,
- kClippedRatioThreshold, kClippedWaitFrames);
- EXPECT_EQ(manager->channel_agcs_[0]->min_mic_level(), kMinMicLevel);
- EXPECT_EQ(manager->channel_agcs_[0]->startup_min_level(), kInitialVolume);
-}
-
-// Checks that a field-trial parameter outside of the valid range [0,255] is
-// ignored.
-TEST(AgcManagerDirectStandaloneTest, AgcMinMicLevelExperimentOutOfRangeBelow) {
- test::ScopedFieldTrials field_trial(
- "WebRTC-Audio-AgcMinMicLevelExperiment/Enabled--1/");
- std::unique_ptr<AgcManagerDirect> manager =
- CreateAgcManagerDirect(kInitialVolume, kClippedLevelStep,
- kClippedRatioThreshold, kClippedWaitFrames);
- EXPECT_EQ(manager->channel_agcs_[0]->min_mic_level(), kMinMicLevel);
- EXPECT_EQ(manager->channel_agcs_[0]->startup_min_level(), kInitialVolume);
-}
-
-// Verifies that a valid experiment changes the minimum microphone level. The
-// start volume is larger than the min level and should therefore not be
-// changed.
-TEST(AgcManagerDirectStandaloneTest, AgcMinMicLevelExperimentEnabled50) {
- constexpr int kMinMicLevelOverride = 50;
- for (const std::string& field_trial_suffix : {"", "_20220210"}) {
- SCOPED_TRACE(field_trial_suffix);
- test::ScopedFieldTrials field_trial(GetAgcMinMicLevelExperimentFieldTrial(
- kMinMicLevelOverride, field_trial_suffix));
- std::unique_ptr<AgcManagerDirect> manager =
- CreateAgcManagerDirect(kInitialVolume, kClippedLevelStep,
- kClippedRatioThreshold, kClippedWaitFrames);
- EXPECT_EQ(manager->channel_agcs_[0]->min_mic_level(), kMinMicLevelOverride);
- EXPECT_EQ(manager->channel_agcs_[0]->startup_min_level(), kInitialVolume);
- }
-}
-
-// Checks that, when the "WebRTC-Audio-AgcMinMicLevelExperiment" field trial is
-// specified with a valid value, the mic level never gets lowered beyond the
-// override value in the presence of clipping.
-TEST(AgcManagerDirectStandaloneTest,
- AgcMinMicLevelExperimentCheckMinLevelWithClipping) {
- constexpr int kMinMicLevelOverride = 250;
-
- // Create and initialize two AGCs by specifying and leaving unspecified the
- // relevant field trial.
- const auto factory = []() {
- std::unique_ptr<AgcManagerDirect> manager =
- CreateAgcManagerDirect(kInitialVolume, kClippedLevelStep,
- kClippedRatioThreshold, kClippedWaitFrames);
- manager->Initialize();
- manager->set_stream_analog_level(kInitialVolume);
- return manager;
- };
- std::unique_ptr<AgcManagerDirect> manager = factory();
- std::unique_ptr<AgcManagerDirect> manager_with_override;
- {
- test::ScopedFieldTrials field_trial(
- GetAgcMinMicLevelExperimentFieldTrial(kMinMicLevelOverride));
- manager_with_override = factory();
- }
-
- // Create a test input signal which containts 80% of clipped samples.
- AudioBuffer audio_buffer(kSampleRateHz, 1, kSampleRateHz, 1, kSampleRateHz,
- 1);
- WriteAudioBufferSamples(/*samples_value=*/4000.0f, /*clipped_ratio=*/0.8f,
- audio_buffer);
-
- // Simulate 4 seconds of clipping; it is expected to trigger a downward
- // adjustment of the analog gain.
- CallPreProcessAndProcess(/*num_calls=*/400, audio_buffer, *manager);
- CallPreProcessAndProcess(/*num_calls=*/400, audio_buffer,
- *manager_with_override);
-
- // Make sure that an adaptation occurred.
- ASSERT_GT(manager->stream_analog_level(), 0);
-
- // Check that the test signal triggers a larger downward adaptation for
- // `manager`, which is allowed to reach a lower gain.
- EXPECT_GT(manager_with_override->stream_analog_level(),
- manager->stream_analog_level());
- // Check that the gain selected by `manager_with_override` equals the minimum
- // value overridden via field trial.
- EXPECT_EQ(manager_with_override->stream_analog_level(), kMinMicLevelOverride);
-}
-
-// Checks that, when the "WebRTC-Audio-AgcMinMicLevelExperiment" field trial is
-// specified with a value lower than the `clipped_level_min`, the behavior of
-// the analog gain controller is the same as that obtained when the field trial
-// is not specified.
-TEST(AgcManagerDirectStandaloneTest,
- AgcMinMicLevelExperimentCompareMicLevelWithClipping) {
- // Create and initialize two AGCs by specifying and leaving unspecified the
- // relevant field trial.
- const auto factory = []() {
- // Use a large clipped level step to more quickly decrease the analog gain
- // with clipping.
- auto controller = std::make_unique<AgcManagerDirect>(
- /*num_capture_channels=*/1, kInitialVolume,
- kDefaultAnalogConfig.clipped_level_min,
- /*disable_digital_adaptive=*/true, /*clipped_level_step=*/64,
- kClippedRatioThreshold, kClippedWaitFrames,
- kDefaultAnalogConfig.clipping_predictor);
- controller->Initialize();
- controller->set_stream_analog_level(kInitialVolume);
- return controller;
- };
- std::unique_ptr<AgcManagerDirect> manager = factory();
- std::unique_ptr<AgcManagerDirect> manager_with_override;
- {
- constexpr int kMinMicLevelOverride = 20;
- static_assert(
- kDefaultAnalogConfig.clipped_level_min >= kMinMicLevelOverride,
- "Use a lower override value.");
- test::ScopedFieldTrials field_trial(
- GetAgcMinMicLevelExperimentFieldTrial(kMinMicLevelOverride));
- manager_with_override = factory();
- }
-
- // Create a test input signal which containts 80% of clipped samples.
- AudioBuffer audio_buffer(kSampleRateHz, 1, kSampleRateHz, 1, kSampleRateHz,
- 1);
- WriteAudioBufferSamples(/*samples_value=*/4000.0f, /*clipped_ratio=*/0.8f,
- audio_buffer);
-
- // Simulate 4 seconds of clipping; it is expected to trigger a downward
- // adjustment of the analog gain.
- CallPreProcessAndProcess(/*num_calls=*/400, audio_buffer, *manager);
- CallPreProcessAndProcess(/*num_calls=*/400, audio_buffer,
- *manager_with_override);
-
- // Make sure that an adaptation occurred.
- ASSERT_GT(manager->stream_analog_level(), 0);
-
- // Check that the selected analog gain is the same for both controllers and
- // that it equals the minimum level reached when clipping is handled. That is
- // expected because the minimum microphone level override is less than the
- // minimum level used when clipping is detected.
- EXPECT_EQ(manager->stream_analog_level(),
- manager_with_override->stream_analog_level());
- EXPECT_EQ(manager_with_override->stream_analog_level(),
- kDefaultAnalogConfig.clipped_level_min);
-}
-
// TODO(bugs.webrtc.org/12774): Test the bahavior of `clipped_level_step`.
// TODO(bugs.webrtc.org/12774): Test the bahavior of `clipped_ratio_threshold`.
// TODO(bugs.webrtc.org/12774): Test the bahavior of `clipped_wait_frames`.
