From 02ba1d252e21daae9ebe80866c7e3d6f63174a64 Mon Sep 17 00:00:00 2001 From: Markus Handell Date: Thu, 14 May 2020 14:31:18 +0200 Subject: [PATCH] AudioProcessingImpl: remove lock recursions. This change removes lock recursions and adds thread annotations. Bug: webrtc:11567 Change-Id: Ibefb49bb5b865cb0bb33e4580d34d9837fb41bff Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/175121 Reviewed-by: Karl Wiberg Commit-Queue: Markus Handell Cr-Commit-Position: refs/heads/master@{#31260} --- .../audio_processing/audio_processing_impl.cc | 18 +++++++++++++----- .../audio_processing/audio_processing_impl.h | 10 ++++++++-- 2 files changed, 21 insertions(+), 7 deletions(-) diff --git a/modules/audio_processing/audio_processing_impl.cc b/modules/audio_processing/audio_processing_impl.cc index 6abebd2612..7dd6b7d467 100644 --- a/modules/audio_processing/audio_processing_impl.cc +++ b/modules/audio_processing/audio_processing_impl.cc @@ -992,6 +992,10 @@ void AudioProcessingImpl::AllocateRenderQueue() { void AudioProcessingImpl::EmptyQueuedRenderAudio() { rtc::CritScope cs_capture(&crit_capture_); + EmptyQueuedRenderAudioLocked(); +} + +void AudioProcessingImpl::EmptyQueuedRenderAudioLocked() { if (submodules_.echo_control_mobile) { RTC_DCHECK(aecm_render_signal_queue_); while (aecm_render_signal_queue_->Remove(&aecm_capture_queue_buffer_)) { @@ -1047,7 +1051,7 @@ int AudioProcessingImpl::ProcessStream(const int16_t* const src, } int AudioProcessingImpl::ProcessCaptureStreamLocked() { - EmptyQueuedRenderAudio(); + EmptyQueuedRenderAudioLocked(); HandleCaptureRuntimeSettings(); // Ensure that not both the AEC and AECM are active at the same time. @@ -1087,7 +1091,7 @@ int AudioProcessingImpl::ProcessCaptureStreamLocked() { if (submodules_.echo_controller) { // Detect and flag any change in the analog gain. - int analog_mic_level = recommended_stream_analog_level(); + int analog_mic_level = recommended_stream_analog_level_locked(); capture_.echo_path_gain_change = capture_.prev_analog_mic_level != analog_mic_level && capture_.prev_analog_mic_level != -1; @@ -1256,7 +1260,7 @@ int AudioProcessingImpl::ProcessCaptureStreamLocked() { if (submodules_.gain_controller2) { submodules_.gain_controller2->NotifyAnalogLevel( - recommended_stream_analog_level()); + recommended_stream_analog_level_locked()); submodules_.gain_controller2->Process(capture_buffer); } @@ -1284,7 +1288,7 @@ int AudioProcessingImpl::ProcessCaptureStreamLocked() { } if (submodules_.agc_manager) { - int level = recommended_stream_analog_level(); + int level = recommended_stream_analog_level_locked(); data_dumper_->DumpRaw("experimental_gain_control_stream_analog_level", 1, &level); } @@ -1524,6 +1528,10 @@ void AudioProcessingImpl::set_stream_analog_level(int level) { int AudioProcessingImpl::recommended_stream_analog_level() const { rtc::CritScope cs_capture(&crit_capture_); + return recommended_stream_analog_level_locked(); +} + +int AudioProcessingImpl::recommended_stream_analog_level_locked() const { if (submodules_.agc_manager) { return submodules_.agc_manager->stream_analog_level(); } else if (submodules_.gain_control) { @@ -2006,7 +2014,7 @@ void AudioProcessingImpl::RecordAudioProcessingState() { AecDump::AudioProcessingState audio_proc_state; audio_proc_state.delay = capture_nonlocked_.stream_delay_ms; audio_proc_state.drift = 0; - audio_proc_state.level = recommended_stream_analog_level(); + audio_proc_state.level = recommended_stream_analog_level_locked(); audio_proc_state.keypress = capture_.key_pressed; aec_dump_->AddAudioProcessingState(audio_proc_state); } diff --git a/modules/audio_processing/audio_processing_impl.h b/modules/audio_processing/audio_processing_impl.h index 3aa86ac5a1..676e3cde9d 100644 --- a/modules/audio_processing/audio_processing_impl.h +++ b/modules/audio_processing/audio_processing_impl.h @@ -100,7 +100,8 @@ class AudioProcessingImpl : public AudioProcessing { int set_stream_delay_ms(int delay) override; void set_stream_key_pressed(bool key_pressed) override; void set_stream_analog_level(int level) override; - int recommended_stream_analog_level() const override; + int recommended_stream_analog_level() const + RTC_LOCKS_EXCLUDED(crit_capture_) override; // Render-side exclusive methods possibly running APM in a // multi-threaded manner. Acquire the render lock. @@ -155,6 +156,9 @@ class AudioProcessingImpl : public AudioProcessing { FRIEND_TEST_ALL_PREFIXES(ApmWithSubmodulesExcludedTest, BitexactWithDisabledModules); + int recommended_stream_analog_level_locked() const + RTC_EXCLUSIVE_LOCKS_REQUIRED(crit_capture_); + void OverrideSubmoduleCreationForTesting( const ApmSubmoduleCreationOverrides& overrides); @@ -281,7 +285,9 @@ class AudioProcessingImpl : public AudioProcessing { RTC_EXCLUSIVE_LOCKS_REQUIRED(crit_capture_); void HandleRenderRuntimeSettings() RTC_EXCLUSIVE_LOCKS_REQUIRED(crit_render_); - void EmptyQueuedRenderAudio(); + void EmptyQueuedRenderAudio() RTC_LOCKS_EXCLUDED(crit_capture_); + void EmptyQueuedRenderAudioLocked() + RTC_EXCLUSIVE_LOCKS_REQUIRED(crit_capture_); void AllocateRenderQueue() RTC_EXCLUSIVE_LOCKS_REQUIRED(crit_render_, crit_capture_); void QueueBandedRenderAudio(AudioBuffer* audio)