Lock scheme #8: Introduced the new locking scheme

webrtc/modules/audio_processing/gain_control_impl.cc

Index: webrtc/modules/audio_processing/gain_control_impl.cc
diff --git a/webrtc/modules/audio_processing/gain_control_impl.cc b/webrtc/modules/audio_processing/gain_control_impl.cc
index 0eacd28686382c2bd8cffb17b340495915d47eb8..c8175dc01f22685246af7255c3ddafe917db5b84 100644
--- a/webrtc/modules/audio_processing/gain_control_impl.cc
+++ b/webrtc/modules/audio_processing/gain_control_impl.cc
@@ -14,7 +14,6 @@
 
 #include "webrtc/modules/audio_processing/audio_buffer.h"
 #include "webrtc/modules/audio_processing/agc/legacy/gain_control.h"
-#include "webrtc/system_wrappers/include/critical_section_wrapper.h"
 
 namespace webrtc {
 
@@ -44,10 +43,12 @@ static const size_t kMaxNumFramesToBuffer = 100;
 }  // namespace
 
 GainControlImpl::GainControlImpl(const AudioProcessing* apm,
-                                 CriticalSectionWrapper* crit)
+                                 rtc::CriticalSection* crit_render,
+                                 rtc::CriticalSection* crit_capture)
     : ProcessingComponent(),
       apm_(apm),
-      crit_(crit),
+      crit_render_(crit_render),
+      crit_capture_(crit_capture),
       mode_(kAdaptiveAnalog),
       minimum_capture_level_(0),
       maximum_capture_level_(255),
@@ -57,13 +58,18 @@ GainControlImpl::GainControlImpl(const AudioProcessing* apm,
       analog_capture_level_(0),
       was_analog_level_set_(false),
       stream_is_saturated_(false),
-      render_queue_element_max_size_(0) {}
+      render_queue_element_max_size_(0) {
+  RTC_DCHECK(apm);
+  RTC_DCHECK(crit_render);
+  RTC_DCHECK(crit_capture);
+}
 
 GainControlImpl::~GainControlImpl() {}
 
 int GainControlImpl::ProcessRenderAudio(AudioBuffer* audio) {
+  rtc::CritScope cs(crit_render_);
   if (!is_component_enabled()) {
-    return apm_->kNoError;
+    return AudioProcessing::kNoError;
   }
 
   assert(audio->num_frames_per_band() <= 160);
@@ -74,7 +80,7 @@ int GainControlImpl::ProcessRenderAudio(AudioBuffer* audio) {
     int err =
         WebRtcAgc_GetAddFarendError(my_handle, audio->num_frames_per_band());
 
-    if (err != apm_->kNoError)
+    if (err != AudioProcessing::kNoError)
       return GetHandleError(my_handle);
 
     // Buffer the samples in the render queue.
@@ -85,18 +91,21 @@ int GainControlImpl::ProcessRenderAudio(AudioBuffer* audio) {
 
   // Insert the samples into the queue.
   if (!render_signal_queue_->Insert(&render_queue_buffer_)) {
+    // The data queue is full and needs to be emptied.
     ReadQueuedRenderData();
 
     // Retry the insert (should always work).
     RTC_DCHECK_EQ(render_signal_queue_->Insert(&render_queue_buffer_), true);
   }
 
-  return apm_->kNoError;
+  return AudioProcessing::kNoError;
 }
 
 // Read chunks of data that were received and queued on the render side from
 // a queue. All the data chunks are buffered into the farend signal of the AGC.
 void GainControlImpl::ReadQueuedRenderData() {
+  rtc::CritScope cs(crit_capture_);
+
   if (!is_component_enabled()) {
     return;
   }
@@ -116,14 +125,16 @@ void GainControlImpl::ReadQueuedRenderData() {
 }
 
 int GainControlImpl::AnalyzeCaptureAudio(AudioBuffer* audio) {
+  rtc::CritScope cs(crit_capture_);
+
   if (!is_component_enabled()) {
-    return apm_->kNoError;
+    return AudioProcessing::kNoError;
   }
 
   assert(audio->num_frames_per_band() <= 160);
   assert(audio->num_channels() == num_handles());
 
-  int err = apm_->kNoError;
+  int err = AudioProcessing::kNoError;
 
   if (mode_ == kAdaptiveAnalog) {
     capture_levels_.assign(num_handles(), analog_capture_level_);
@@ -135,7 +146,7 @@ int GainControlImpl::AnalyzeCaptureAudio(AudioBuffer* audio) {
           audio->num_bands(),
           audio->num_frames_per_band());
 
-      if (err != apm_->kNoError) {
+      if (err != AudioProcessing::kNoError) {
         return GetHandleError(my_handle);
       }
     }
@@ -155,23 +166,25 @@ int GainControlImpl::AnalyzeCaptureAudio(AudioBuffer* audio) {
 
       capture_levels_[i] = capture_level_out;
 
-      if (err != apm_->kNoError) {
+      if (err != AudioProcessing::kNoError) {
         return GetHandleError(my_handle);
       }
 
     }
   }
 
-  return apm_->kNoError;
+  return AudioProcessing::kNoError;
 }
 
 int GainControlImpl::ProcessCaptureAudio(AudioBuffer* audio) {
+  rtc::CritScope cs(crit_capture_);
+
   if (!is_component_enabled()) {
-    return apm_->kNoError;
+    return AudioProcessing::kNoError;
   }
 
   if (mode_ == kAdaptiveAnalog && !was_analog_level_set_) {
-    return apm_->kStreamParameterNotSetError;
+    return AudioProcessing::kStreamParameterNotSetError;
   }
 
   assert(audio->num_frames_per_band() <= 160);
@@ -183,6 +196,8 @@ int GainControlImpl::ProcessCaptureAudio(AudioBuffer* audio) {
     int32_t capture_level_out = 0;
     uint8_t saturation_warning = 0;
 
+    // The call to stream_has_echo() is ok from a deadlock perspective
+    // as the capture lock is allready held.
     int err = WebRtcAgc_Process(
         my_handle,
         audio->split_bands_const(i),
@@ -194,7 +209,7 @@ int GainControlImpl::ProcessCaptureAudio(AudioBuffer* audio) {
         apm_->echo_cancellation()->stream_has_echo(),
         &saturation_warning);
 
-    if (err != apm_->kNoError) {
+    if (err != AudioProcessing::kNoError) {
       return GetHandleError(my_handle);
     }
 
@@ -215,22 +230,24 @@ int GainControlImpl::ProcessCaptureAudio(AudioBuffer* audio) {
   }
 
   was_analog_level_set_ = false;
-  return apm_->kNoError;
+  return AudioProcessing::kNoError;
 }
 
 // TODO(ajm): ensure this is called under kAdaptiveAnalog.
 int GainControlImpl::set_stream_analog_level(int level) {
-  CriticalSectionScoped crit_scoped(crit_);
+  rtc::CritScope cs(crit_capture_);
+
   was_analog_level_set_ = true;
   if (level < minimum_capture_level_ || level > maximum_capture_level_) {
-    return apm_->kBadParameterError;
+    return AudioProcessing::kBadParameterError;
   }
   analog_capture_level_ = level;
 
-  return apm_->kNoError;
+  return AudioProcessing::kNoError;
 }
 
 int GainControlImpl::stream_analog_level() {
+  rtc::CritScope cs(crit_capture_);
   // TODO(ajm): enable this assertion?
   //assert(mode_ == kAdaptiveAnalog);
 
@@ -238,18 +255,21 @@ int GainControlImpl::stream_analog_level() {
 }
 
 int GainControlImpl::Enable(bool enable) {
-  CriticalSectionScoped crit_scoped(crit_);
+  rtc::CritScope cs_render(crit_render_);
+  rtc::CritScope cs_capture(crit_capture_);
   return EnableComponent(enable);
 }
 
 bool GainControlImpl::is_enabled() const {
+  rtc::CritScope cs(crit_capture_);
   return is_component_enabled();
 }
 
 int GainControlImpl::set_mode(Mode mode) {
-  CriticalSectionScoped crit_scoped(crit_);
+  rtc::CritScope cs_render(crit_render_);
+  rtc::CritScope cs_capture(crit_capture_);
   if (MapSetting(mode) == -1) {
-    return apm_->kBadParameterError;
+    return AudioProcessing::kBadParameterError;
   }
 
   mode_ = mode;
@@ -257,22 +277,23 @@ int GainControlImpl::set_mode(Mode mode) {
 }
 
 GainControl::Mode GainControlImpl::mode() const {
+  rtc::CritScope cs(crit_capture_);
   return mode_;
 }
 
 int GainControlImpl::set_analog_level_limits(int minimum,
                                              int maximum) {
-  CriticalSectionScoped crit_scoped(crit_);
+  rtc::CritScope cs(crit_capture_);
   if (minimum < 0) {
-    return apm_->kBadParameterError;
+    return AudioProcessing::kBadParameterError;
   }
 
   if (maximum > 65535) {
-    return apm_->kBadParameterError;
+    return AudioProcessing::kBadParameterError;
   }
 
   if (maximum < minimum) {
-    return apm_->kBadParameterError;
+    return AudioProcessing::kBadParameterError;
   }
 
   minimum_capture_level_ = minimum;
@@ -282,21 +303,24 @@ int GainControlImpl::set_analog_level_limits(int minimum,
 }
 
 int GainControlImpl::analog_level_minimum() const {
+  rtc::CritScope cs(crit_capture_);
   return minimum_capture_level_;
 }
 
 int GainControlImpl::analog_level_maximum() const {
+  rtc::CritScope cs(crit_capture_);
   return maximum_capture_level_;
 }
 
 bool GainControlImpl::stream_is_saturated() const {
+  rtc::CritScope cs(crit_capture_);
   return stream_is_saturated_;
 }
 
 int GainControlImpl::set_target_level_dbfs(int level) {
-  CriticalSectionScoped crit_scoped(crit_);
+  rtc::CritScope cs(crit_capture_);
   if (level > 31 || level < 0) {
-    return apm_->kBadParameterError;
+    return AudioProcessing::kBadParameterError;
   }
 
   target_level_dbfs_ = level;
@@ -304,13 +328,14 @@ int GainControlImpl::set_target_level_dbfs(int level) {
 }
 
 int GainControlImpl::target_level_dbfs() const {
+  rtc::CritScope cs(crit_capture_);
   return target_level_dbfs_;
 }
 
 int GainControlImpl::set_compression_gain_db(int gain) {
-  CriticalSectionScoped crit_scoped(crit_);
+  rtc::CritScope cs(crit_capture_);
   if (gain < 0 || gain > 90) {
-    return apm_->kBadParameterError;
+    return AudioProcessing::kBadParameterError;
   }
 
   compression_gain_db_ = gain;
@@ -318,31 +343,35 @@ int GainControlImpl::set_compression_gain_db(int gain) {
 }
 
 int GainControlImpl::compression_gain_db() const {
+  rtc::CritScope cs(crit_capture_);
   return compression_gain_db_;
 }
 
 int GainControlImpl::enable_limiter(bool enable) {
-  CriticalSectionScoped crit_scoped(crit_);
+  rtc::CritScope cs(crit_capture_);
   limiter_enabled_ = enable;
   return Configure();
 }
 
 bool GainControlImpl::is_limiter_enabled() const {
+  rtc::CritScope cs(crit_capture_);
   return limiter_enabled_;
 }
 
 int GainControlImpl::Initialize() {
   int err = ProcessingComponent::Initialize();
-  if (err != apm_->kNoError || !is_component_enabled()) {
+  if (err != AudioProcessing::kNoError || !is_component_enabled()) {
     return err;
   }
 
   AllocateRenderQueue();
 
+  rtc::CritScope cs_capture(crit_capture_);
   const int n = num_handles();
   RTC_CHECK_GE(n, 0) << "Bad number of handles: " << n;
+
   capture_levels_.assign(n, analog_capture_level_);
-  return apm_->kNoError;
+  return AudioProcessing::kNoError;
 }
 
 void GainControlImpl::AllocateRenderQueue() {
@@ -350,6 +379,9 @@ void GainControlImpl::AllocateRenderQueue() {
       std::max<size_t>(static_cast<size_t>(1),
                        kMaxAllowedValuesOfSamplesPerFrame * num_handles());
 
+  rtc::CritScope cs_render(crit_render_);
+  rtc::CritScope cs_capture(crit_capture_);
+
   if (render_queue_element_max_size_ < new_render_queue_element_max_size) {
     render_queue_element_max_size_ = new_render_queue_element_max_size;
     std::vector<int16_t> template_queue_element(render_queue_element_max_size_);
@@ -375,6 +407,9 @@ void GainControlImpl::DestroyHandle(void* handle) const {
 }
 
 int GainControlImpl::InitializeHandle(void* handle) const {
+  rtc::CritScope cs_render(crit_render_);
+  rtc::CritScope cs_capture(crit_capture_);
+
   return WebRtcAgc_Init(static_cast<Handle*>(handle),
                           minimum_capture_level_,
                           maximum_capture_level_,
@@ -383,6 +418,8 @@ int GainControlImpl::InitializeHandle(void* handle) const {
 }
 
 int GainControlImpl::ConfigureHandle(void* handle) const {
+  rtc::CritScope cs_render(crit_render_);
+  rtc::CritScope cs_capture(crit_capture_);
   WebRtcAgcConfig config;
   // TODO(ajm): Flip the sign here (since AGC expects a positive value) if we
   //            change the interface.
@@ -397,6 +434,7 @@ int GainControlImpl::ConfigureHandle(void* handle) const {
 }
 
 int GainControlImpl::num_handles_required() const {
+  // Not locked as it only relies on APM public API which is threadsafe.
   return apm_->num_output_channels();
 }
 
@@ -404,6 +442,6 @@ int GainControlImpl::GetHandleError(void* handle) const {
   // The AGC has no get_error() function.
   // (Despite listing errors in its interface...)
   assert(handle != NULL);
-  return apm_->kUnspecifiedError;
+  return AudioProcessing::kUnspecifiedError;
 }
 }  // namespace webrtc