Reset level estimator when analog gain changes.
In AgcManagerDirect::UpdateGain(), Agc::GetRmsErrorDb() is
called. Depending on the result of that call, the analog gain may be
changed. After an analog gain change, the Agc should be reset, because
it's memory contains now invalid loudness levels.
The Agc in modules/audio_processing/agc/agc.cc resets itself at every
successful Agc::GetRmsErrorDb call. The AdaptiveModeLevelEstimatorAgc
does not. This change makes sure all Agcs are reset from
AgcManagerDirect.
It will cause some Agcs to be reset twice. This is fine, because
Agc::Reset() is cheap and idempotent.
Bug: webrtc:7494
Change-Id: Iee7495d699cbdb9d69b2ae0cb07034c6e2761e22
Reviewed-on: https://webrtc-review.googlesource.com/89040
Commit-Queue: Alex Loiko <aleloi@webrtc.org>
Reviewed-by: Alessio Bazzica <alessiob@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#24054}diff --git a/modules/audio_processing/agc/agc_manager_direct.cc b/modules/audio_processing/agc/agc_manager_direct.cc
index 132cf82..9386837 100644
--- a/modules/audio_processing/agc/agc_manager_direct.cc
+++ b/modules/audio_processing/agc/agc_manager_direct.cc
@@ -115,13 +115,15 @@
int clipped_level_min,
bool use_agc2_level_estimation,
bool use_agc2_digital_adaptive)
- : AgcManagerDirect(new Agc(),
+ : AgcManagerDirect(use_agc2_level_estimation ? nullptr : new Agc(),
gctrl,
volume_callbacks,
startup_min_level,
clipped_level_min,
use_agc2_level_estimation,
- use_agc2_digital_adaptive) {}
+ use_agc2_digital_adaptive) {
+ RTC_DCHECK(agc_);
+}
AgcManagerDirect::AgcManagerDirect(Agc* agc,
GainControl* gctrl,
@@ -134,7 +136,9 @@
startup_min_level,
clipped_level_min,
false,
- false) {}
+ false) {
+ RTC_DCHECK(agc_);
+}
AgcManagerDirect::AgcManagerDirect(Agc* agc,
GainControl* gctrl,
@@ -437,6 +441,8 @@
// level_ was updated by SetLevel; log the new value.
RTC_HISTOGRAM_COUNTS_LINEAR("WebRTC.Audio.AgcSetLevel", level_, 1,
kMaxMicLevel, 50);
+ // Reset the AGC since the level has changed.
+ agc_->Reset();
}
}
diff --git a/modules/audio_processing/agc/agc_manager_direct_unittest.cc b/modules/audio_processing/agc/agc_manager_direct_unittest.cc
index 1a03402..dae0769 100644
--- a/modules/audio_processing/agc/agc_manager_direct_unittest.cc
+++ b/modules/audio_processing/agc/agc_manager_direct_unittest.cc
@@ -17,6 +17,7 @@
#include "test/gtest.h"
using ::testing::_;
+using ::testing::AtLeast;
using ::testing::DoAll;
using ::testing::Return;
using ::testing::SetArgPointee;
@@ -53,7 +54,7 @@
}
void FirstProcess() {
- EXPECT_CALL(*agc_, Reset());
+ EXPECT_CALL(*agc_, Reset()).Times(AtLeast(1));
EXPECT_CALL(*agc_, GetRmsErrorDb(_)).WillOnce(Return(false));
CallProcess(1);
}
@@ -366,11 +367,14 @@
// to SetMicVolume.
EXPECT_CALL(*agc_, GetRmsErrorDb(_))
.WillOnce(DoAll(SetArgPointee<0>(11), Return(true)));
+
+ // When the analog volume changes, the gain controller is reset.
+ EXPECT_CALL(*agc_, Reset()).Times(AtLeast(1));
+
// GetMicVolume returns a value outside of the quantization slack, indicating
// a manual volume change.
+ ASSERT_NE(volume_.GetMicVolume(), 154);
volume_.SetMicVolume(154);
- // SetMicVolume should not be called.
- EXPECT_CALL(*agc_, Reset()).Times(1);
CallProcess(1);
EXPECT_EQ(154, volume_.GetMicVolume());
@@ -378,7 +382,7 @@
EXPECT_CALL(*agc_, GetRmsErrorDb(_))
.WillOnce(DoAll(SetArgPointee<0>(-1), Return(true)));
volume_.SetMicVolume(100);
- EXPECT_CALL(*agc_, Reset()).Times(1);
+ EXPECT_CALL(*agc_, Reset()).Times(AtLeast(1));
CallProcess(1);
EXPECT_EQ(100, volume_.GetMicVolume());
@@ -407,7 +411,7 @@
EXPECT_CALL(*agc_, GetRmsErrorDb(_))
.WillOnce(DoAll(SetArgPointee<0>(-1), Return(true)));
volume_.SetMicVolume(50);
- EXPECT_CALL(*agc_, Reset()).Times(1);
+ EXPECT_CALL(*agc_, Reset()).Times(AtLeast(1));
CallProcess(1);
EXPECT_EQ(50, volume_.GetMicVolume());
@@ -426,7 +430,7 @@
.WillOnce(DoAll(SetArgPointee<0>(-1), Return(true)));
// Don't set to zero, which will cause AGC to take no action.
volume_.SetMicVolume(1);
- EXPECT_CALL(*agc_, Reset()).Times(1);
+ EXPECT_CALL(*agc_, Reset()).Times(AtLeast(1));
CallProcess(1);
EXPECT_EQ(1, volume_.GetMicVolume());
@@ -467,7 +471,7 @@
SetVolumeAndProcess(255);
EXPECT_CALL(*agc_, AnalyzePreproc(_, _)).WillOnce(Return(0.101));
- EXPECT_CALL(*agc_, Reset()).Times(1);
+ EXPECT_CALL(*agc_, Reset()).Times(AtLeast(1));
CallPreProc(1);
EXPECT_EQ(240, volume_.GetMicVolume());
}
@@ -477,7 +481,7 @@
EXPECT_CALL(*agc_, AnalyzePreproc(_, _))
.WillOnce(Return(kAboveClippedThreshold));
- EXPECT_CALL(*agc_, Reset()).Times(1);
+ EXPECT_CALL(*agc_, Reset()).Times(AtLeast(1));
CallPreProc(1);
EXPECT_EQ(240, volume_.GetMicVolume());
@@ -489,7 +493,7 @@
EXPECT_CALL(*agc_, AnalyzePreproc(_, _))
.WillOnce(Return(kAboveClippedThreshold));
- EXPECT_CALL(*agc_, Reset()).Times(1);
+ EXPECT_CALL(*agc_, Reset()).Times(AtLeast(1));
CallPreProc(1);
EXPECT_EQ(225, volume_.GetMicVolume());
}
@@ -499,7 +503,7 @@
EXPECT_CALL(*agc_, AnalyzePreproc(_, _))
.WillOnce(Return(kAboveClippedThreshold));
- EXPECT_CALL(*agc_, Reset()).Times(1);
+ EXPECT_CALL(*agc_, Reset()).Times(AtLeast(1));
CallPreProc(1);
EXPECT_EQ(kClippedMin, volume_.GetMicVolume());
@@ -515,7 +519,7 @@
EXPECT_CALL(*agc_, AnalyzePreproc(_, _))
.WillOnce(Return(kAboveClippedThreshold));
- EXPECT_CALL(*agc_, Reset()).Times(1);
+ EXPECT_CALL(*agc_, Reset()).Times(AtLeast(1));
CallPreProc(1);
EXPECT_EQ(240, volume_.GetMicVolume());
@@ -530,7 +534,7 @@
EXPECT_CALL(*agc_, AnalyzePreproc(_, _))
.WillOnce(Return(kAboveClippedThreshold));
- EXPECT_CALL(*agc_, Reset()).Times(1);
+ EXPECT_CALL(*agc_, Reset()).Times(AtLeast(1));
CallPreProc(1);
EXPECT_EQ(185, volume_.GetMicVolume());
@@ -547,7 +551,7 @@
EXPECT_CALL(*agc_, AnalyzePreproc(_, _))
.WillOnce(Return(kAboveClippedThreshold));
- EXPECT_CALL(*agc_, Reset()).Times(1);
+ EXPECT_CALL(*agc_, Reset()).Times(AtLeast(1));
CallPreProc(1);
EXPECT_EQ(195, volume_.GetMicVolume());
@@ -576,14 +580,14 @@
CallPreProc(300);
EXPECT_CALL(*agc_, AnalyzePreproc(_, _))
.WillOnce(Return(kAboveClippedThreshold));
- EXPECT_CALL(*agc_, Reset()).Times(1);
+ EXPECT_CALL(*agc_, Reset()).Times(AtLeast(1));
CallPreProc(1);
EXPECT_EQ(180, volume_.GetMicVolume());
CallPreProc(300);
EXPECT_CALL(*agc_, AnalyzePreproc(_, _))
.WillOnce(Return(kAboveClippedThreshold));
- EXPECT_CALL(*agc_, Reset()).Times(1);
+ EXPECT_CALL(*agc_, Reset()).Times(AtLeast(1));
CallPreProc(1);
EXPECT_EQ(kClippedMin, volume_.GetMicVolume());
@@ -628,7 +632,7 @@
EXPECT_CALL(*agc_, AnalyzePreproc(_, _))
.WillOnce(Return(kAboveClippedThreshold));
- EXPECT_CALL(*agc_, Reset()).Times(1);
+ EXPECT_CALL(*agc_, Reset()).Times(AtLeast(1));
CallPreProc(1);
EXPECT_EQ(210, volume_.GetMicVolume());
@@ -637,7 +641,7 @@
.WillOnce(DoAll(SetArgPointee<0>(14), Return(true)));
// User changed the volume.
volume_.SetMicVolume(250);
- EXPECT_CALL(*agc_, Reset()).Times(1);
+ EXPECT_CALL(*agc_, Reset()).Times(AtLeast(1));
CallProcess(1);
EXPECT_EQ(250, volume_.GetMicVolume());
