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 97709a5186c7e663d1ca6c0ba1231adeead10b2c..a1bd56e517c58a01afc1a4a5b9cfd9d42b587018 100644
--- a/webrtc/modules/audio_processing/echo_cancellation_impl.h
+++ b/webrtc/modules/audio_processing/echo_cancellation_impl.h
@@ -11,6 +11,7 @@
 #ifndef WEBRTC_MODULES_AUDIO_PROCESSING_ECHO_CANCELLATION_IMPL_H_
 #define WEBRTC_MODULES_AUDIO_PROCESSING_ECHO_CANCELLATION_IMPL_H_
 
+#include "webrtc/base/criticalsection.h"
 #include "webrtc/base/scoped_ptr.h"
 #include "webrtc/base/thread_checker.h"
 #include "webrtc/common_audio/swap_queue.h"
@@ -20,65 +21,96 @@
 namespace webrtc {
 
 class AudioBuffer;
-class CriticalSectionWrapper;
 
 class EchoCancellationImpl : public EchoCancellation,
                              public ProcessingComponent {
  public:
   EchoCancellationImpl(const AudioProcessing* apm,
-                       CriticalSectionWrapper* crit,
+                       rtc::CriticalSection* crit_render,
+                       rtc::CriticalSection* crit_capture,
                        const rtc::ThreadChecker* render_thread_checker);
   virtual ~EchoCancellationImpl();
 
+  // Called holding the render lock.

[the sun, 2015/11/23 21:36:05]

These comments just add noise. Remove. Document your assumptions with
RTC_DCHECKs.

[peah-webrtc, 2015/11/24 21:42:24]

After today's discussion, I think this work will merit one more CL chained to
this one. So if ok with you, I will fix this in that CL instead.

   int ProcessRenderAudio(const AudioBuffer* audio);
+  // Called holding the capture lock.
   int ProcessCaptureAudio(AudioBuffer* audio);
 
   // EchoCancellation implementation.
+  // Aquires both the render and capture locks.
   bool is_enabled() const override;
+  // Aquires the capture loc.k

[kwiberg-webrtc, 2015/11/23 22:15:11]

Swap the last two characters.

[peah-webrtc, 2015/11/24 21:42:24]

Done.

   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;
 
   // ProcessingComponent implementation.
+  // Called holding both the capture and render locks.
   int Initialize() override;
+  // Conditionally acquires the capture lock.
   void SetExtraOptions(const Config& config) override;
-
+  // Only called from APM. No lock required.
   bool is_delay_agnostic_enabled() const;
+  // Only called from APM. No lock required.
   bool is_extended_filter_enabled() const;
 
   // Reads render side data that has been queued on the render call.
+  // Called holding the capture lock.
   void ReadQueuedRenderData();
 
  private:
   // EchoCancellation implementation.
+  // Aquires both the render and capture locks.
   int Enable(bool enable) override;
+  // Aquires the capture lock.
   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;
+  // Conditionally acquires the capture lock.
   bool stream_has_echo() const override;
+  // Aquires the capture lock.
   int GetMetrics(Metrics* metrics) override;
+  // Aquires the capture lock.
   int enable_delay_logging(bool enable) override;
+  // Only called from APM. No lock required.
   bool is_delay_logging_enabled() const override;
+  // Aquires the capture lock.
   int GetDelayMetrics(int* median, int* std) override;
+  // Aquires the capture lock.
   int GetDelayMetrics(int* median,
                       int* std,
                       float* fraction_poor_delays) override;
+  // Only called from APM. No lock required.
   struct AecCore* aec_core() const override;
 
   // ProcessingComponent implementation.
+  // Called holding both the render and capture locks.
   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();
 
   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_;
@@ -94,6 +126,8 @@ class EchoCancellationImpl : public EchoCancellation,
   size_t render_queue_element_max_size_;
   std::vector<float> render_queue_buffer_;
   std::vector<float> capture_queue_buffer_;
+
+  // Lock protection not needed.
   rtc::scoped_ptr<SwapQueue<std::vector<float>, RenderQueueItemVerifier<float>>>
       render_signal_queue_;
 };