From 308bc646e01944a8d2c2077373c7d08f8289b6d3 Mon Sep 17 00:00:00 2001
From: Sam Zackrisson <saza@webrtc.org>
Date: Mon, 23 Dec 2019 10:22:08 +0100
Subject: [PATCH] Remove one acquisition of capture lock in APM AudioFrame API
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

This brings the two ProcessStream functions closer in implementation.
Additionally, the error checking that is currently done in the period of not holding the lock seems cheaper than releasing and reacquiring the capture lock.

Bug: webrtc:11235
Change-Id: Ib4afc68afb419fcabbb8cf08a3a2e61d2c12acda
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/163021
Reviewed-by: Per Åhgren <peah@webrtc.org>
Commit-Queue: Sam Zackrisson <saza@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#30140}
---
 .../audio_processing/audio_processing_impl.cc | 55 ++++++++++---------
 1 file changed, 28 insertions(+), 27 deletions(-)

diff --git a/modules/audio_processing/audio_processing_impl.cc b/modules/audio_processing/audio_processing_impl.cc
index 81959127a1..f22db20759 100644
--- a/modules/audio_processing/audio_processing_impl.cc
+++ b/modules/audio_processing/audio_processing_impl.cc
@@ -843,9 +843,13 @@ int AudioProcessingImpl::ProcessStream(const float* const* src,
   ProcessingConfig processing_config;
   bool reinitialization_required = false;
   {
-    // Acquire the capture lock in order to safely call the function
-    // that retrieves the render side data. This function accesses apm
-    // getters that need the capture lock held when being called.
+    // Acquire the capture lock in order to:
+    // - Safely call the function that retrieves the render side data. This
+    //   function accesses APM getters that need the capture lock held when
+    //   being called.
+    // - Access api_format. The lock is released immediately due to the
+    //   conditional reinitialization.
+
     rtc::CritScope cs_capture(&crit_capture_);
     EmptyQueuedRenderAudio();
 
@@ -1101,32 +1105,30 @@ void AudioProcessingImpl::EmptyQueuedRenderAudio() {
 
 int AudioProcessingImpl::ProcessStream(AudioFrame* frame) {
   TRACE_EVENT0("webrtc", "AudioProcessing::ProcessStream_AudioFrame");
-  {
-    // Acquire the capture lock in order to safely call the function
-    // that retrieves the render side data. This function accesses APM
-    // getters that need the capture lock held when being called.
-    rtc::CritScope cs_capture(&crit_capture_);
-    EmptyQueuedRenderAudio();
-  }
-
-  if (!frame) {
-    return kNullPointerError;
-  }
-  // Must be a native rate.
-  if (frame->sample_rate_hz_ != kSampleRate8kHz &&
-      frame->sample_rate_hz_ != kSampleRate16kHz &&
-      frame->sample_rate_hz_ != kSampleRate32kHz &&
-      frame->sample_rate_hz_ != kSampleRate48kHz) {
-    return kBadSampleRateError;
-  }
 
   ProcessingConfig processing_config;
   bool reinitialization_required = false;
   {
-    // Aquire lock for the access of api_format.
-    // The lock is released immediately due to the conditional
-    // reinitialization.
+    // Acquire the capture lock in order to:
+    // - Safely call the function that retrieves the render side data. This
+    //   function accesses APM getters that need the capture lock held when
+    //   being called.
+    // - Access api_format. The lock is released immediately due to the
+    //   conditional reinitialization.
     rtc::CritScope cs_capture(&crit_capture_);
+    EmptyQueuedRenderAudio();
+
+    if (!frame) {
+      return kNullPointerError;
+    }
+    // Must be a native rate.
+    if (frame->sample_rate_hz_ != kSampleRate8kHz &&
+        frame->sample_rate_hz_ != kSampleRate16kHz &&
+        frame->sample_rate_hz_ != kSampleRate32kHz &&
+        frame->sample_rate_hz_ != kSampleRate48kHz) {
+      return kBadSampleRateError;
+    }
+
     // TODO(ajm): The input and output rates and channels are currently
     // constrained to be identical in the int16 interface.
     processing_config = formats_.api_format;
@@ -1198,9 +1200,8 @@ int AudioProcessingImpl::ProcessCaptureStreamLocked() {
   // Ensure that not both the AEC and AECM are active at the same time.
   // TODO(peah): Simplify once the public API Enable functions for these
   // are moved to APM.
-  RTC_DCHECK_LE(!!submodules_.echo_controller +
-                    !!submodules_.echo_control_mobile,
-                1);
+  RTC_DCHECK_LE(
+      !!submodules_.echo_controller + !!submodules_.echo_control_mobile, 1);
 
   AudioBuffer* capture_buffer = capture_.capture_audio.get();  // For brevity.
   AudioBuffer* linear_aec_buffer = capture_.linear_aec_output.get();