From 533e4612288247b2ba52f010dfeea7d0af382e9d Mon Sep 17 00:00:00 2001 From: Alessio Bazzica Date: Wed, 7 Sep 2022 16:58:33 +0200 Subject: [PATCH] APM: make `recommended_stream_analog_level()` a trivial getter MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 Reviewed-by: Per Åhgren Commit-Queue: Alessio Bazzica Cr-Commit-Position: refs/heads/main@{#38055} --- .../audio_processing/audio_processing_impl.cc | 46 ++++++++++++++----- .../audio_processing/audio_processing_impl.h | 6 ++- 2 files changed, 40 insertions(+), 12 deletions(-) diff --git a/modules/audio_processing/audio_processing_impl.cc b/modules/audio_processing/audio_processing_impl.cc index b839f91fd0..0857b2801d 100644 --- a/modules/audio_processing/audio_processing_impl.cc +++ b/modules/audio_processing/audio_processing_impl.cc @@ -1359,6 +1359,8 @@ int AudioProcessingImpl::ProcessCaptureStreamLocked() { // 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 @@ int AudioProcessingImpl::ProcessCaptureStreamLocked() { // 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 @@ int AudioProcessingImpl::ProcessCaptureStreamLocked() { 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 @@ void AudioProcessingImpl::set_stream_analog_level_locked(int level) { *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 @@ void AudioProcessingImpl::set_stream_analog_level_locked(int level) { 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 e28d1f6ed9..0f1ba519c1 100644 --- a/modules/audio_processing/audio_processing_impl.h +++ b/modules/audio_processing/audio_processing_impl.h @@ -163,7 +163,7 @@ class AudioProcessingImpl : public AudioProcessing { 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 @@ class AudioProcessingImpl : public AudioProcessing { // acquired. Unspecified when unknown. absl::optional 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 recommended_input_volume; } capture_ RTC_GUARDED_BY(mutex_capture_); struct ApmCaptureNonLockedState {