diff --git a/modules/audio_processing/agc/agc_manager_direct.cc b/modules/audio_processing/agc/agc_manager_direct.cc index 8bc6de979f..7e209e8621 100644 --- a/modules/audio_processing/agc/agc_manager_direct.cc +++ b/modules/audio_processing/agc/agc_manager_direct.cc @@ -62,14 +62,14 @@ bool UseMaxAnalogChannelLevel() { return field_trial::IsEnabled("WebRTC-UseMaxAnalogAgcChannelLevel"); } -// If the "WebRTC-Audio-AgcMinMicLevelExperiment" field trial is specified, +// If the "WebRTC-Audio-2ndAgcMinMicLevelExperiment" 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. +// 'WebRTC-Audio-2ndAgcMinMicLevelExperiment/Enabled-80' => returns 80. absl::optional GetMinMicLevelOverride() { constexpr char kMinMicLevelFieldTrial[] = - "WebRTC-Audio-AgcMinMicLevelExperiment"; + "WebRTC-Audio-2ndAgcMinMicLevelExperiment"; if (!webrtc::field_trial::IsEnabled(kMinMicLevelFieldTrial)) { return absl::nullopt; } @@ -722,9 +722,8 @@ 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() && 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..268667a82c 100644 --- a/modules/audio_processing/agc/agc_manager_direct.h +++ b/modules/audio_processing/agc/agc_manager_direct.h @@ -91,7 +91,7 @@ class AgcManagerDirect final { FRIEND_TEST_ALL_PREFIXES(AgcManagerDirectStandaloneTest, DisableDigitalDisablesDigital); FRIEND_TEST_ALL_PREFIXES(AgcManagerDirectStandaloneTest, - AgcMinMicLevelExperiment); + AgcMinMicLevelExperimentDefault); FRIEND_TEST_ALL_PREFIXES(AgcManagerDirectStandaloneTest, AgcMinMicLevelExperimentDisabled); FRIEND_TEST_ALL_PREFIXES(AgcManagerDirectStandaloneTest, diff --git a/modules/audio_processing/agc/agc_manager_direct_unittest.cc b/modules/audio_processing/agc/agc_manager_direct_unittest.cc index 5c9b383f78..35e5d82241 100644 --- a/modules/audio_processing/agc/agc_manager_direct_unittest.cc +++ b/modules/audio_processing/agc/agc_manager_direct_unittest.cc @@ -128,15 +128,25 @@ void CallPreProcessAudioBuffer(int num_calls, } } -std::string GetAgcMinMicLevelExperimentFieldTrial( +constexpr char kMinMicLevelFieldTrial[] = + "WebRTC-Audio-2ndAgcMinMicLevelExperiment"; + +std::string GetAgcMinMicLevelExperimentFieldTrial(const std::string& value) { + char field_trial_buffer[64]; + rtc::SimpleStringBuilder builder(field_trial_buffer); + builder << kMinMicLevelFieldTrial << "/" << value << "/"; + return builder.str(); +} + +std::string GetAgcMinMicLevelExperimentFieldTrialEnabled( 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 << "/"; + builder << kMinMicLevelFieldTrial << "/Enabled-" << enabled_value << suffix + << "/"; return builder.str(); } @@ -283,6 +293,10 @@ class AgcManagerDirectTest : public ::testing::Test { std::vector audio_data; }; +// TODO(crbug.com/1275566): Switch to parametric test and run all the +// AgcManagerDirectTest tests enabling and disabling +// "WebRTC-Audio-2ndAgcMinMicLevelExperiment". + TEST_F(AgcManagerDirectTest, StartupMinVolumeConfigurationIsRespected) { FirstProcess(); EXPECT_EQ(kInitialVolume, manager_.stream_analog_level()); @@ -876,7 +890,7 @@ TEST(AgcManagerDirectStandaloneTest, DisableDigitalDisablesDigital) { manager->SetupDigitalGainControl(&gctrl); } -TEST(AgcManagerDirectStandaloneTest, AgcMinMicLevelExperiment) { +TEST(AgcManagerDirectStandaloneTest, AgcMinMicLevelExperimentDefault) { std::unique_ptr manager = CreateAgcManagerDirect(kInitialVolume, kClippedLevelStep, kClippedRatioThreshold, kClippedWaitFrames); @@ -887,8 +901,7 @@ TEST(AgcManagerDirectStandaloneTest, AgcMinMicLevelExperiment) { TEST(AgcManagerDirectStandaloneTest, AgcMinMicLevelExperimentDisabled) { for (const std::string& field_trial_suffix : {"", "_20220210"}) { test::ScopedFieldTrials field_trial( - "WebRTC-Audio-AgcMinMicLevelExperiment/Disabled" + field_trial_suffix + - "/"); + GetAgcMinMicLevelExperimentFieldTrial("Disabled" + field_trial_suffix)); std::unique_ptr manager = CreateAgcManagerDirect(kInitialVolume, kClippedLevelStep, kClippedRatioThreshold, kClippedWaitFrames); @@ -901,7 +914,7 @@ TEST(AgcManagerDirectStandaloneTest, AgcMinMicLevelExperimentDisabled) { // ignored. TEST(AgcManagerDirectStandaloneTest, AgcMinMicLevelExperimentOutOfRangeAbove) { test::ScopedFieldTrials field_trial( - "WebRTC-Audio-AgcMinMicLevelExperiment/Enabled-256/"); + GetAgcMinMicLevelExperimentFieldTrial("Enabled-256")); std::unique_ptr manager = CreateAgcManagerDirect(kInitialVolume, kClippedLevelStep, kClippedRatioThreshold, kClippedWaitFrames); @@ -913,7 +926,7 @@ TEST(AgcManagerDirectStandaloneTest, AgcMinMicLevelExperimentOutOfRangeAbove) { // ignored. TEST(AgcManagerDirectStandaloneTest, AgcMinMicLevelExperimentOutOfRangeBelow) { test::ScopedFieldTrials field_trial( - "WebRTC-Audio-AgcMinMicLevelExperiment/Enabled--1/"); + GetAgcMinMicLevelExperimentFieldTrial("Enabled--1")); std::unique_ptr manager = CreateAgcManagerDirect(kInitialVolume, kClippedLevelStep, kClippedRatioThreshold, kClippedWaitFrames); @@ -928,8 +941,9 @@ 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)); + test::ScopedFieldTrials field_trial( + GetAgcMinMicLevelExperimentFieldTrialEnabled(kMinMicLevelOverride, + field_trial_suffix)); std::unique_ptr manager = CreateAgcManagerDirect(kInitialVolume, kClippedLevelStep, kClippedRatioThreshold, kClippedWaitFrames); @@ -959,7 +973,7 @@ TEST(AgcManagerDirectStandaloneTest, std::unique_ptr manager_with_override; { test::ScopedFieldTrials field_trial( - GetAgcMinMicLevelExperimentFieldTrial(kMinMicLevelOverride)); + GetAgcMinMicLevelExperimentFieldTrialEnabled(kMinMicLevelOverride)); manager_with_override = factory(); } @@ -1016,7 +1030,7 @@ TEST(AgcManagerDirectStandaloneTest, kDefaultAnalogConfig.clipped_level_min >= kMinMicLevelOverride, "Use a lower override value."); test::ScopedFieldTrials field_trial( - GetAgcMinMicLevelExperimentFieldTrial(kMinMicLevelOverride)); + GetAgcMinMicLevelExperimentFieldTrialEnabled(kMinMicLevelOverride)); manager_with_override = factory(); }