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}
This commit is contained in:
Alessio Bazzica
2022-07-05 16:47:26 +02:00
committed by WebRTC LUCI CQ
parent c9cad23274
commit e76daab8b3
3 changed files with 20 additions and 19 deletions

View File

@ -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_);
}

View File

@ -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_;

View File

@ -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);