APM: make `recommended_stream_analog_level()` a trivial getter
The current design of the modified getter is error-prone since the
returned value changes meaning based on when (which point in the code)
the getter is called - namely, before `ProcessStream()` is called the
getter returns the stream analog level, after it returns the
recommended level.
Plus, the new implementation, which essentially returns a local
member, removes the risks that the non-trivial implementation
is computationally expensive.
Bug: webrtc:7494, b/241923537
Change-Id: I6714444df27bcc055ae693974ecd1f77f79e6ec0
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/271580
Reviewed-by: Hanna Silen <silen@webrtc.org>
Reviewed-by: Per Åhgren <peah@webrtc.org>
Commit-Queue: Alessio Bazzica <alessiob@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#38055}
diff --git a/modules/audio_processing/audio_processing_impl.cc b/modules/audio_processing/audio_processing_impl.cc
index b839f91..0857b28 100644
--- a/modules/audio_processing/audio_processing_impl.cc
+++ b/modules/audio_processing/audio_processing_impl.cc
@@ -1359,6 +1359,8 @@
// Pass stats for reporting.
stats_reporter_.UpdateStatistics(capture_.stats);
+ UpdateRecommendedInputVolumeLocked();
+
if (submodules_.capture_levels_adjuster) {
submodules_.capture_levels_adjuster->ApplyPostLevelAdjustment(
*capture_buffer);
@@ -1367,8 +1369,9 @@
// If the input volume emulation is used, retrieve the recommended input
// volume and set that to emulate the input volume on the next processed
// audio frame.
+ RTC_DCHECK(capture_.recommended_input_volume.has_value());
submodules_.capture_levels_adjuster->SetAnalogMicGainLevel(
- recommended_stream_analog_level_locked());
+ *capture_.recommended_input_volume);
}
}
@@ -1388,7 +1391,9 @@
capture_.was_stream_delay_set = false;
- // TODO(bugs.webrtc.org/7494): Dump recommended input volume.
+ data_dumper_->DumpRaw("recommended_input_volume",
+ capture_.recommended_input_volume.value_or(
+ kUnspecifiedDataDumpInputVolume));
return kNoError;
}
@@ -1612,6 +1617,10 @@
*capture_.applied_input_volume != level;
capture_.applied_input_volume = level;
+ // Invalidate any previously recommended input volume which will be updated by
+ // `ProcessStream()`.
+ capture_.recommended_input_volume = absl::nullopt;
+
if (submodules_.agc_manager) {
submodules_.agc_manager->set_stream_analog_level(level);
return;
@@ -1626,25 +1635,40 @@
int AudioProcessingImpl::recommended_stream_analog_level() const {
MutexLock lock_capture(&mutex_capture_);
- return recommended_stream_analog_level_locked();
-}
-
-int AudioProcessingImpl::recommended_stream_analog_level_locked() const {
if (!capture_.applied_input_volume.has_value()) {
RTC_LOG(LS_ERROR) << "set_stream_analog_level has not been called";
}
+ // Input volume to recommend when `set_stream_analog_level()` is not called.
+ constexpr int kFallBackInputVolume = 255;
+ // When APM has no input volume to recommend, return the latest applied input
+ // volume that has been observed in order to possibly produce no input volume
+ // change. If no applied input volume has been observed, return a fall-back
+ // value.
+ return capture_.recommended_input_volume.value_or(
+ capture_.applied_input_volume.value_or(kFallBackInputVolume));
+}
+
+void AudioProcessingImpl::UpdateRecommendedInputVolumeLocked() {
+ if (!capture_.applied_input_volume.has_value()) {
+ // When `set_stream_analog_level()` is not called, no input level can be
+ // recommended.
+ capture_.recommended_input_volume = absl::nullopt;
+ return;
+ }
if (submodules_.agc_manager) {
- return submodules_.agc_manager->recommended_analog_level();
+ capture_.recommended_input_volume =
+ submodules_.agc_manager->recommended_analog_level();
+ return;
}
if (submodules_.gain_control) {
- return submodules_.gain_control->stream_analog_level();
+ capture_.recommended_input_volume =
+ submodules_.gain_control->stream_analog_level();
+ return;
}
- // Input volume to recommend when `set_stream_analog_level()` is not called.
- constexpr int kFallBackInputVolume = 255;
- return capture_.applied_input_volume.value_or(kFallBackInputVolume);
+ capture_.recommended_input_volume = capture_.applied_input_volume;
}
bool AudioProcessingImpl::CreateAndAttachAecDump(absl::string_view file_name,
diff --git a/modules/audio_processing/audio_processing_impl.h b/modules/audio_processing/audio_processing_impl.h
index e28d1f6..0f1ba51 100644
--- a/modules/audio_processing/audio_processing_impl.h
+++ b/modules/audio_processing/audio_processing_impl.h
@@ -163,7 +163,7 @@
void set_stream_analog_level_locked(int level)
RTC_EXCLUSIVE_LOCKS_REQUIRED(mutex_capture_);
- int recommended_stream_analog_level_locked() const
+ void UpdateRecommendedInputVolumeLocked()
RTC_EXCLUSIVE_LOCKS_REQUIRED(mutex_capture_);
void OverrideSubmoduleCreationForTesting(
@@ -475,6 +475,10 @@
// acquired. Unspecified when unknown.
absl::optional<int> applied_input_volume;
bool applied_input_volume_changed;
+ // Recommended input volume to apply on the audio input device the next time
+ // that audio is acquired. Unspecified when no input volume can be
+ // recommended.
+ absl::optional<int> recommended_input_volume;
} capture_ RTC_GUARDED_BY(mutex_capture_);
struct ApmCaptureNonLockedState {
