zqz/platform-external-webrtc
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Reland "AgcManagerDirect: stop enforcing min mic level override with 0 level"

Browse Source 
This reverts commit d0a6fd239cef0d9fd5fdd5a41df389a696bff017.

Reason for revert: reland the bug fix

Original change's description:
> Revert "`AgcManagerDirect`: stop enforcing min mic level override with 0 level"
>
> This reverts commit e76daab8b36f8c2a16d59a116425a3a2f98022f6.
>
> Reason for revert: revert required to revert the parent CL
>
> Original change's description:
> > `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}
>
> Bug: chromium:1275566
> Change-Id: I6d22d8f3fafdc7da3814827b9b69146a506595db
> Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/268468
> Bot-Commit: rubber-stamper@appspot.gserviceaccount.com <rubber-stamper@appspot.gserviceaccount.com>
> Commit-Queue: Alessio Bazzica <alessiob@webrtc.org>
> Cr-Commit-Position: refs/heads/main@{#37515}

Bug: chromium:1275566
Change-Id: I7198587dec2a153270e8beb714e9dacccdaae806
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/268544
Reviewed-by: Hanna Silen <silen@webrtc.org>
Commit-Queue: Alessio Bazzica <alessiob@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#37530}
This commit is contained in:
Alessio Bazzica
2022-07-15 10:11:42 +02:00
committed by WebRTC LUCI CQ
parent 591ea38cdc
commit 08480a599d
3 changed files with 32 additions and 19 deletions
Download Patch File Download Diff File Expand all files Collapse all files

11
modules/audio_processing/agc/agc_manager_direct.cc
View File

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

2
modules/audio_processing/agc/agc_manager_direct.h
View File

@ -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,

38
modules/audio_processing/agc/agc_manager_direct_unittest.cc
View File

@ -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<float> 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<AgcManagerDirect> 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<AgcManagerDirect> 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<AgcManagerDirect> 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<AgcManagerDirect> 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<AgcManagerDirect> manager =
CreateAgcManagerDirect(kInitialVolume, kClippedLevelStep,
kClippedRatioThreshold, kClippedWaitFrames);
@ -959,7 +973,7 @@ TEST(AgcManagerDirectStandaloneTest,
std::unique_ptr<AgcManagerDirect> 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();
}