Lock scheme #8: Introduced the new locking scheme

webrtc/modules/audio_processing/echo_cancellation_impl.h

Index: webrtc/modules/audio_processing/echo_cancellation_impl.h
diff --git a/webrtc/modules/audio_processing/echo_cancellation_impl.h b/webrtc/modules/audio_processing/echo_cancellation_impl.h
index 6903ea639f47df91a2bd9b01a7fe25f575ecd63f..ae9de61f0f3dc2c6af378e0588b603b48ee17555 100644
--- a/webrtc/modules/audio_processing/echo_cancellation_impl.h
+++ b/webrtc/modules/audio_processing/echo_cancellation_impl.h
@@ -12,6 +12,7 @@
 #define WEBRTC_MODULES_AUDIO_PROCESSING_ECHO_CANCELLATION_IMPL_H_
 
 #include "webrtc/base/scoped_ptr.h"
+#include "webrtc/base/thread_annotations.h"

[hlundin-webrtc, 2015/11/05 16:11:22]

You're not using any of this, right?

[peah-webrtc, 2015/11/06 09:54:32]

Done.

 #include "webrtc/base/thread_checker.h"
 #include "webrtc/common_audio/swap_queue.h"
 #include "webrtc/modules/audio_processing/include/audio_processing.h"
@@ -38,34 +39,45 @@ class AecRenderQueueItemVerifier {
 }  // namespace anonymous
 
 class AudioBuffer;
-class CriticalSectionWrapper;
 
 class EchoCancellationImpl : public EchoCancellation,
                              public ProcessingComponent {
  public:
+  // Called holding both the capture and render locks.
   EchoCancellationImpl(const AudioProcessing* apm,
-                       CriticalSectionWrapper* crit,
+                       rtc::CriticalSection* crit_render,
+                       rtc::CriticalSection* crit_capture,
                        rtc::ThreadChecker* render_thread_checker);
+
   virtual ~EchoCancellationImpl();
 
-  int ProcessRenderAudio(const AudioBuffer* audio);
-  int ProcessCaptureAudio(AudioBuffer* audio);
+  int ProcessRenderAudio(
+      const AudioBuffer* audio);  // Called holding the render lock.

[hlundin-webrtc, 2015/11/05 16:11:22]

+1 for adding the comments.
-1 for the ugly line breaks. Please, add the comments to the line above each
method and restore the original format. Here and in all places below.

[peah-webrtc, 2015/11/06 09:54:32]

Sorry!!!
Done.

+  int ProcessCaptureAudio(
+      AudioBuffer* audio);  // Called holding the capture lock.
 
   // EchoCancellation implementation.
-  bool is_enabled() const override;
-  int stream_drift_samples() const override;
-  SuppressionLevel suppression_level() const override;
-  bool is_drift_compensation_enabled() const override;
+  bool is_enabled()
+      const override;  // Aquires both the render and capture locks.
+  int stream_drift_samples() const override;  // Aquires the capture lock.
+  SuppressionLevel suppression_level()
+      const override;  // Aquires the capture lock.
+  bool is_drift_compensation_enabled()
+      const override;  // Aquires the capture lock.
 
   // ProcessingComponent implementation.
-  int Initialize() override;
-  void SetExtraOptions(const Config& config) override;
+  int Initialize()
+      override;  // Called holding both the capture and render locks.
+  void SetExtraOptions(const Config& config)
+      override;  // Conditionally acquires the capture lock.
 
-  bool is_delay_agnostic_enabled() const;
-  bool is_extended_filter_enabled() const;
+  bool is_delay_agnostic_enabled()
+      const;  // Only called from APM. No lock required.
+  bool is_extended_filter_enabled()
+      const;  // Only called from APM. No lock required.
 
   // Reads render side data that has been queued on the render call.
-  void ReadQueuedRenderData();
+  void ReadQueuedRenderData();  // Called holding the capture lock.
 
  private:
   static const size_t kAllowedValuesOfSamplesPerFrame1 = 80;
@@ -75,34 +87,51 @@ class EchoCancellationImpl : public EchoCancellation,
   static const size_t kMaxNumFramesToBuffer = 100;
 
   // EchoCancellation implementation.
-  int Enable(bool enable) override;
-  int enable_drift_compensation(bool enable) override;
-  void set_stream_drift_samples(int drift) override;
-  int set_suppression_level(SuppressionLevel level) override;
-  int enable_metrics(bool enable) override;
-  bool are_metrics_enabled() const override;
-  bool stream_has_echo() const override;
-  int GetMetrics(Metrics* metrics) override;
-  int enable_delay_logging(bool enable) override;
-  bool is_delay_logging_enabled() const override;
-  int GetDelayMetrics(int* median, int* std) override;
+  int Enable(
+      bool enable) override;  // Aquires both the render and capture locks.
+  int enable_drift_compensation(
+      bool enable) override;  // Aquires the capture lock.
+  void set_stream_drift_samples(
+      int drift) override;  // Aquires the capture lock.
+  int set_suppression_level(
+      SuppressionLevel level) override;       // Aquires the capture lock.
+  int enable_metrics(bool enable) override;   // Aquires the capture lock.
+  bool are_metrics_enabled() const override;  // Aquires the capture lock.
+  bool stream_has_echo()
+      const override;  // Conditionally acquires the capture lock.
+  int GetMetrics(Metrics* metrics) override;       // Aquires the capture lock.
+  int enable_delay_logging(bool enable) override;  // Aquires the capture lock.
+  bool is_delay_logging_enabled()
+      const override;  // Only called from APM. No lock required.
   int GetDelayMetrics(int* median,
-                      int* std,
-                      float* fraction_poor_delays) override;
-  struct AecCore* aec_core() const override;
+                      int* std) override;  // Aquires the capture lock.
+  int GetDelayMetrics(
+      int* median,
+      int* std,
+      float* fraction_poor_delays) override;  // Aquires the capture lock.
+  struct AecCore* aec_core()
+      const override;  // Only called from APM. No lock required.
 
   // ProcessingComponent implementation.
-  void* CreateHandle() const override;
-  int InitializeHandle(void* handle) const override;
-  int ConfigureHandle(void* handle) const override;
-  void DestroyHandle(void* handle) const override;
-  int num_handles_required() const override;
-  int GetHandleError(void* handle) const override;
-
-  void AllocateRenderQueue();
+  void* CreateHandle()
+      const override;  // Called holding both the render and capture locks.
+  int InitializeHandle(void* handle)
+      const override;  // Called holding both the render and capture locks.
+  int ConfigureHandle(void* handle)
+      const override;  // Called holding both the render and capture locks.
+  void DestroyHandle(void* handle)
+      const override;  // Called holding both the render and capture locks.
+  int num_handles_required()
+      const override;  // Called holding both the render and capture locks.
+  int GetHandleError(void* handle)
+      const override;  // Called holding both the render and capture locks.
+
+  void
+  AllocateRenderQueue();  // Called holding both the render and capture locks.
 
   const AudioProcessing* apm_;
-  CriticalSectionWrapper* crit_;
+  rtc::CriticalSection* const crit_render_;
+  rtc::CriticalSection* const crit_capture_;
   const rtc::ThreadChecker* const render_thread_checker_;
 
   bool drift_compensation_enabled_;
@@ -118,8 +147,8 @@ class EchoCancellationImpl : public EchoCancellation,
   size_t render_queue_element_max_size_;
   std::vector<float> render_queue_buffer_;
   std::vector<float> capture_queue_buffer_;
-  rtc::scoped_ptr<SwapQueue<std::vector<float>, AecRenderQueueItemVerifier>>
-      render_signal_queue_;
+  rtc::scoped_ptr<SwapQueue<std::vector<float>, AecRenderQueueItemVerifier> >
+      render_signal_queue_;  // Lock protection not needed.
 };
 
 }  // namespace webrtc