commit 4b27648d8b3279415d7249a7046eca74b684dcdc
author Oskar Sundbom <ossu@webrtc.org> Thu May 23 12:28:00 2019
committer Commit Bot <commit-bot@chromium.org> Fri May 24 13:24:27 2019
tree 69bf6d44ac96a14f95067c2b07fef0fcf58dca05
parent a0e9943ca69a826a2b0997361773cb96655e2bd7

Avoid the render lock in AudioProcessingImpl::ProcessStream

It seems unnecessary to lock it if not actually reinitializing.

Bug: webrtc:10205
Change-Id: Ib3292e1d640a92a7df77400aebe9583cf877f824
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/115460
Commit-Queue: Oskar Sundbom <ossu@webrtc.org>
Reviewed-by: Per Åhgren <peah@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#28060}

modules/audio_processing/audio_processing_impl.cc

diff --git a/modules/audio_processing/audio_processing_impl.cc b/modules/audio_processing/audio_processing_impl.cc
index c2ff7f0..a700038 100644
--- a/modules/audio_processing/audio_processing_impl.cc
+++ b/modules/audio_processing/audio_processing_impl.cc
@@ -486,22 +486,8 @@
 
 int AudioProcessingImpl::MaybeInitializeRender(
     const ProcessingConfig& processing_config) {
-  return MaybeInitialize(processing_config, false);
-}
-
-int AudioProcessingImpl::MaybeInitializeCapture(
-    const ProcessingConfig& processing_config,
-    bool force_initialization) {
-  return MaybeInitialize(processing_config, force_initialization);
-}
-
-// Calls InitializeLocked() if any of the audio parameters have changed from
-// their current values (needs to be called while holding the crit_render_lock).
-int AudioProcessingImpl::MaybeInitialize(
-    const ProcessingConfig& processing_config,
-    bool force_initialization) {
   // Called from both threads. Thread check is therefore not possible.
-  if (processing_config == formats_.api_format && !force_initialization) {
+  if (processing_config == formats_.api_format) {
     return kNoError;
   }
 
@@ -938,15 +924,23 @@
     reinitialization_required = UpdateActiveSubmoduleStates();
   }
 
-  processing_config.input_stream() = input_config;
-  processing_config.output_stream() = output_config;
-
-  {
-    // Do conditional reinitialization.
-    rtc::CritScope cs_render(&crit_render_);
-    RETURN_ON_ERR(
-        MaybeInitializeCapture(processing_config, reinitialization_required));
+  if (processing_config.input_stream() != input_config) {
+    processing_config.input_stream() = input_config;
+    reinitialization_required = true;
   }
+
+  if (processing_config.output_stream() != output_config) {
+    processing_config.output_stream() = output_config;
+    reinitialization_required = true;
+  }
+
+  if (reinitialization_required) {
+    // Reinitialize.
+    rtc::CritScope cs_render(&crit_render_);
+    rtc::CritScope cs_capture(&crit_capture_);
+    RETURN_ON_ERR(InitializeLocked(processing_config));
+  }
+
   rtc::CritScope cs_capture(&crit_capture_);
   RTC_DCHECK_EQ(processing_config.input_stream().num_frames(),
                 formats_.api_format.input_stream().num_frames());
@@ -1216,17 +1210,29 @@
 
     reinitialization_required = UpdateActiveSubmoduleStates();
   }
-  processing_config.input_stream().set_sample_rate_hz(frame->sample_rate_hz_);
-  processing_config.input_stream().set_num_channels(frame->num_channels_);
-  processing_config.output_stream().set_sample_rate_hz(frame->sample_rate_hz_);
-  processing_config.output_stream().set_num_channels(frame->num_channels_);
 
-  {
-    // Do conditional reinitialization.
+  reinitialization_required =
+      reinitialization_required ||
+      processing_config.input_stream().sample_rate_hz() !=
+          frame->sample_rate_hz_ ||
+      processing_config.input_stream().num_channels() != frame->num_channels_ ||
+      processing_config.output_stream().sample_rate_hz() !=
+          frame->sample_rate_hz_ ||
+      processing_config.output_stream().num_channels() != frame->num_channels_;
+
+  if (reinitialization_required) {
+    processing_config.input_stream().set_sample_rate_hz(frame->sample_rate_hz_);
+    processing_config.input_stream().set_num_channels(frame->num_channels_);
+    processing_config.output_stream().set_sample_rate_hz(
+        frame->sample_rate_hz_);
+    processing_config.output_stream().set_num_channels(frame->num_channels_);
+
+    // Reinitialize.
     rtc::CritScope cs_render(&crit_render_);
-    RETURN_ON_ERR(
-        MaybeInitializeCapture(processing_config, reinitialization_required));
+    rtc::CritScope cs_capture(&crit_capture_);
+    RETURN_ON_ERR(InitializeLocked(processing_config));
   }
+
   rtc::CritScope cs_capture(&crit_capture_);
   if (frame->samples_per_channel_ !=
       formats_.api_format.input_stream().num_frames()) {

modules/audio_processing/audio_processing_impl.h

diff --git a/modules/audio_processing/audio_processing_impl.h b/modules/audio_processing/audio_processing_impl.h
index a2b023a..05dbb50 100644
--- a/modules/audio_processing/audio_processing_impl.h
+++ b/modules/audio_processing/audio_processing_impl.h
@@ -221,16 +221,9 @@
   // that the capture thread blocks the render thread.
   // The struct is modified in a single-threaded manner by holding both the
   // render and capture locks.
-  int MaybeInitialize(const ProcessingConfig& config, bool force_initialization)
-      RTC_EXCLUSIVE_LOCKS_REQUIRED(crit_render_);
-
   int MaybeInitializeRender(const ProcessingConfig& processing_config)
       RTC_EXCLUSIVE_LOCKS_REQUIRED(crit_render_);
 
-  int MaybeInitializeCapture(const ProcessingConfig& processing_config,
-                             bool force_initialization)
-      RTC_EXCLUSIVE_LOCKS_REQUIRED(crit_render_);
-
   // Method for updating the state keeping track of the active submodules.
   // Returns a bool indicating whether the state has changed.
   bool UpdateActiveSubmoduleStates()