Lock scheme #4: Introduced the render sample queue for the aec and aecm

webrtc/modules/audio_processing/echo_cancellation_impl.h

 /*
  *  Copyright (c) 2012 The WebRTC project authors. All Rights Reserved.
  *
  *  Use of this source code is governed by a BSD-style license
  *  that can be found in the LICENSE file in the root of the source
  *  tree. An additional intellectual property rights grant can be found
  *  in the file PATENTS.  All contributing project authors may
  *  be found in the AUTHORS file in the root of the source tree.
  */
 
 #ifndef WEBRTC_MODULES_AUDIO_PROCESSING_ECHO_CANCELLATION_IMPL_H_
 #define WEBRTC_MODULES_AUDIO_PROCESSING_ECHO_CANCELLATION_IMPL_H_
 
+#include "webrtc/base/scoped_ptr.h"
+#include "webrtc/common_audio/swap_queue.h"
 #include "webrtc/modules/audio_processing/include/audio_processing.h"
 #include "webrtc/modules/audio_processing/processing_component.h"
 
 namespace webrtc {
 
+namespace {
+// Functor to use when supplying a verifier function for the queue item
+// verifcation.
+class AecRenderQueueItemVerifier {
+ public:
+  explicit AecRenderQueueItemVerifier(size_t allowed_size_1,
+                                      size_t allowed_size_2)
+      : allowed_size_1_(allowed_size_1), allowed_size_2_(allowed_size_2) {}
+
+  bool operator()(const std::vector<float>& v) const {
+    return (((v.size() == allowed_size_1_) || (v.size() == allowed_size_2_)) &&
+            (v.capacity() >= std::max(allowed_size_1_, allowed_size_2_)));

[kwiberg-webrtc, 2015/10/27 10:43:07]

Why test size in addition to capacity? The latter is sufficient to ensure there
will be no reallocations.

[peah-webrtc, 2015/10/29 11:36:56]

Done.

+  }
+
+ private:
+  size_t allowed_size_1_;
+  size_t allowed_size_2_;
+};
+
+}  // namespace anonymous
+
 class AudioBuffer;
 class CriticalSectionWrapper;
 
 class EchoCancellationImpl : public EchoCancellation,
                              public ProcessingComponent {
  public:
   EchoCancellationImpl(const AudioProcessing* apm,
                        CriticalSectionWrapper* crit);
   virtual ~EchoCancellationImpl();
 
   int ProcessRenderAudio(const AudioBuffer* audio);
   int ProcessCaptureAudio(AudioBuffer* audio);
 
   // 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;
 
   // ProcessingComponent implementation.
   int Initialize() override;
   void SetExtraOptions(const Config& config) override;
 
   bool is_delay_agnostic_enabled() const;
   bool is_extended_filter_enabled() const;
 
+  // Reads render side data that has been queued on the render call.
+  void ReadQueuedRenderData();
+
  private:
+  static const size_t kAllowedValuesOfSamplesPerFrame1 = 80;
+  static const size_t kAllowedValuesOfSamplesPerFrame2 = 160;
+  // TODO(peah): Decrease this once we properly handle hugely unbalanced
+  // reverse and forward call numbers.
+  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 GetDelayMetrics(int* median,
                       int* std,
                       float* fraction_poor_delays) override;
   struct AecCore* aec_core() const override;
 
   // 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();
+
   const AudioProcessing* apm_;
   CriticalSectionWrapper* crit_;
   bool drift_compensation_enabled_;
   bool metrics_enabled_;
   SuppressionLevel suppression_level_;
   int stream_drift_samples_;
   bool was_stream_drift_set_;
   bool stream_has_echo_;
   bool delay_logging_enabled_;
   bool extended_filter_enabled_;
   bool delay_agnostic_enabled_;
+
+  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_;

[kwiberg-webrtc, 2015/10/27 10:43:07]

As of C++11, you can write >> instead of > >.

Also, why make an indirection with a scoped_ptr instead of holding the SwapQueue
directly?

[peah-webrtc, 2015/10/29 11:36:56]

The thinking was that since the queue needs to be reconstructed during runtime,
it is better to do it this way as it is cheaper to new it up dynamically, than
to create it and assign to a queue state variable.

Would that make sense, or is there another way to do this?

[kwiberg-webrtc, 2015/10/29 12:30:22]

Well... Why do you need to recreate it? Isn't this what the Clean
method is for?

[peah-webrtc, 2015/10/29 13:07:36]

I need to recreate it because the maximum item size has changed, and there is
currently no functionality to enforce a new container size on the queue. 

This cannot be known in advance since the number of channels are only known
during runtime and can change during reinitialization. Therefore queue
reallocation may be needed.

I realized that the reinitialization should be conditional though, so I added
that in the latest update.

[kwiberg-webrtc, 2015/10/29 15:04:50]

OK. So it isn't just a performance thing; you actually *need* the indirection,
since SwapQueue doesn't support assignment.

Then I'd argue that SwapQueue should support assignment. Move assignment should
even be more efficient than the indirection you have now.

[peah-webrtc, 2015/11/05 11:45:10]

According to discussions in another CL, I found the conclusion to be that until
move assignment is allowed, the current implementation simplifies the use of the
SwapQueue while having the same complexity as if a copy assignment/construction
is implemented.

Therefore I keep the current implementation. Please say if you don't agree!

 };
 
 }  // namespace webrtc
 
 #endif  // WEBRTC_MODULES_AUDIO_PROCESSING_ECHO_CANCELLATION_IMPL_H_