Moved the AECM render sample queue into the audio processing module

webrtc/modules/audio_processing/audio_processing_impl.cc

Index: webrtc/modules/audio_processing/audio_processing_impl.cc
diff --git a/webrtc/modules/audio_processing/audio_processing_impl.cc b/webrtc/modules/audio_processing/audio_processing_impl.cc
index 93e43f7add126592ddf940480d53292b5280ce45..ace788efa02aff1efa2fd3c688c60d34c35b17ba 100644
--- a/webrtc/modules/audio_processing/audio_processing_impl.cc
+++ b/webrtc/modules/audio_processing/audio_processing_impl.cc
@@ -708,7 +708,6 @@ int AudioProcessingImpl::ProcessStream(const float* const* src,
     // getters that need the capture lock held when being called.
     rtc::CritScope cs_capture(&crit_capture_);
     EmptyQueuedRenderAudio();
-    public_submodules_->echo_control_mobile->ReadQueuedRenderData();
     public_submodules_->gain_control->ReadQueuedRenderData();
 
     if (!src || !dest) {
@@ -770,52 +769,103 @@ int AudioProcessingImpl::ProcessStream(const float* const* src,
 void AudioProcessingImpl::QueueRenderAudio(const AudioBuffer* audio) {
   EchoCancellationImpl::PackRenderAudioBuffer(audio, num_output_channels(),
                                               num_reverse_channels(),
-                                              &render_queue_buffer_);
+                                              &float_render_queue_buffer_);
 
   RTC_DCHECK_GE(160u, audio->num_frames_per_band());
 
   // Insert the samples into the queue.
-  if (!render_signal_queue_->Insert(&render_queue_buffer_)) {
+  if (!float_render_signal_queue_->Insert(&float_render_queue_buffer_)) {
     // The data queue is full and needs to be emptied.
     EmptyQueuedRenderAudio();
 
     // Retry the insert (should always work).
-    bool result = render_signal_queue_->Insert(&render_queue_buffer_);
+    bool result =
+        float_render_signal_queue_->Insert(&float_render_queue_buffer_);
+    RTC_DCHECK(result);
+  }
+
+  EchoControlMobileImpl::PackRenderAudioBuffer(audio, num_output_channels(),
+                                               num_reverse_channels(),
+                                               &int16_render_queue_buffer_);
+
+  // Insert the samples into the queue.
+  if (!int16_render_signal_queue_->Insert(&int16_render_queue_buffer_)) {
+    // The data queue is full and needs to be emptied.
+    EmptyQueuedRenderAudio();
+
+    // Retry the insert (should always work).
+    bool result =
+        int16_render_signal_queue_->Insert(&int16_render_queue_buffer_);
     RTC_DCHECK(result);
   }
 }
 
 void AudioProcessingImpl::AllocateRenderQueue() {
-  const size_t new_render_queue_element_max_size =
+  const size_t new_float_render_queue_element_max_size =
       std::max(static_cast<size_t>(1),
                kMaxAllowedValuesOfSamplesPerFrame *
                    EchoCancellationImpl::NumCancellersRequired(
                        num_output_channels(), num_reverse_channels()));
 
-  // Reallocate the queue if the queue item size is too small to fit the
-  // data to put in the queue.
-  if (render_queue_element_max_size_ < new_render_queue_element_max_size) {
-    render_queue_element_max_size_ = new_render_queue_element_max_size;
+  const size_t new_int16_render_queue_element_max_size =

[ivoc, 2016/10/25 08:57:00]

Would this ever be different from the size of the float buffer in practice?

[peah-webrtc, 2016/10/25 09:48:33]

In practice it should not. However, currently these lengths implicitly depend on
how the submodules are initialized. In practice, it should not be a problem
since they are not used at the same time but until we do further refactoring I'd
prefer to have these lengths separate.

WDYT?

[ivoc, 2016/10/25 10:39:46]

Ok, that sounds fine.

+      std::max(static_cast<size_t>(1),
+               kMaxAllowedValuesOfSamplesPerFrame *
+                   EchoControlMobileImpl::NumCancellersRequired(
+                       num_output_channels(), num_reverse_channels()));
+
+  // Reallocate the queues if the queue item sizes are too small to fit the
+  // data to put in the queues.
+  if (float_render_queue_element_max_size_ <
+      new_float_render_queue_element_max_size) {
+    float_render_queue_element_max_size_ =
+        new_float_render_queue_element_max_size;
 
-    std::vector<float> template_queue_element(render_queue_element_max_size_);
+    std::vector<float> template_queue_element(
+        float_render_queue_element_max_size_);
 
-    render_signal_queue_.reset(
+    float_render_signal_queue_.reset(
         new SwapQueue<std::vector<float>, RenderQueueItemVerifier<float>>(
             kMaxNumFramesToBuffer, template_queue_element,
-            RenderQueueItemVerifier<float>(render_queue_element_max_size_)));
+            RenderQueueItemVerifier<float>(
+                float_render_queue_element_max_size_)));
 
-    render_queue_buffer_.resize(render_queue_element_max_size_);
-    capture_queue_buffer_.resize(render_queue_element_max_size_);
+    float_render_queue_buffer_.resize(float_render_queue_element_max_size_);
+    float_capture_queue_buffer_.resize(float_render_queue_element_max_size_);
   } else {
-    render_signal_queue_->Clear();
+    float_render_signal_queue_->Clear();
+  }
+
+  if (int16_render_queue_element_max_size_ <
+      new_int16_render_queue_element_max_size) {
+    int16_render_queue_element_max_size_ =
+        new_int16_render_queue_element_max_size;
+
+    std::vector<int16_t> template_queue_element(
+        int16_render_queue_element_max_size_);
+
+    int16_render_signal_queue_.reset(
+        new SwapQueue<std::vector<int16_t>, RenderQueueItemVerifier<int16_t>>(
+            kMaxNumFramesToBuffer, template_queue_element,
+            RenderQueueItemVerifier<int16_t>(
+                int16_render_queue_element_max_size_)));
+
+    int16_render_queue_buffer_.resize(int16_render_queue_element_max_size_);
+    int16_capture_queue_buffer_.resize(int16_render_queue_element_max_size_);
+  } else {
+    int16_render_signal_queue_->Clear();
   }
 }
 
 void AudioProcessingImpl::EmptyQueuedRenderAudio() {
   rtc::CritScope cs_capture(&crit_capture_);
-  while (render_signal_queue_->Remove(&capture_queue_buffer_)) {
+  while (float_render_signal_queue_->Remove(&float_capture_queue_buffer_)) {
     public_submodules_->echo_cancellation->ProcessRenderAudio(
-        capture_queue_buffer_);
+        float_capture_queue_buffer_);
+  }
+
+  while (int16_render_signal_queue_->Remove(&int16_capture_queue_buffer_)) {

[ivoc, 2016/10/25 08:57:00]

Shouldn't this be split into separate functions? This function is called when
the float buffer is full or when the int16 buffer is full. If the sizes of these
buffers are potentially different (and it looks from the code that they could
be), then the buffers can be full at different times. Shouldn't we have a
separate EmptyQueuedFloatRenderAudio() and EmptyQueuedInt16RenderAudio()?

[peah-webrtc, 2016/10/25 09:48:33]

Yes, that could make sense.
This method is, however, called in two situations
1) Before a capture call to ensure that all render data has been processed.
2) When the render buffer becomes full to ensure that there is space in the
render buffer for more data.

Bullet 1) is part of the standard operation and for that I think that it is not
necessary to split this method.

Bullet 2) is very costly as it forces the APM to run single-threadedly. This we
want to do as seldom as possible and therefore I think it is good to take the
opportunity to empty all the render buffers to avoid that happening soon again.

Finally, note that all the render buffers store kMaxNumFramesToBuffer so they
should have the same lengths all the time.

WDYT? Can we keep it as it is?

[ivoc, 2016/10/25 10:39:46]

Right, that does make sense, let's keep it like this.

+    public_submodules_->echo_control_mobile->ProcessRenderAudio(
+        int16_capture_queue_buffer_);
   }
 }
 
@@ -830,7 +880,6 @@ int AudioProcessingImpl::ProcessStream(AudioFrame* frame) {
     // as well.
     rtc::CritScope cs_capture(&crit_capture_);
     EmptyQueuedRenderAudio();
-    public_submodules_->echo_control_mobile->ReadQueuedRenderData();
     public_submodules_->gain_control->ReadQueuedRenderData();
   }
 
@@ -1193,8 +1242,6 @@ int AudioProcessingImpl::ProcessRenderStreamLocked() {
 #endif
 
   QueueRenderAudio(render_buffer);
-  RETURN_ON_ERR(public_submodules_->echo_control_mobile->ProcessRenderAudio(
-      render_buffer));
   if (!constants_.use_experimental_agc) {
     RETURN_ON_ERR(
         public_submodules_->gain_control->ProcessRenderAudio(render_buffer));