Major AEC3 render pipeline changes

webrtc/modules/audio_processing/aec3/block_processor.cc

Index: webrtc/modules/audio_processing/aec3/block_processor.cc
diff --git a/webrtc/modules/audio_processing/aec3/block_processor.cc b/webrtc/modules/audio_processing/aec3/block_processor.cc
index 5055b3f77deedcb7805989cf3f381f8f1a552ec3..83957731a2e50a84c04ce6373967068cc4cdb417 100644
--- a/webrtc/modules/audio_processing/aec3/block_processor.cc
+++ b/webrtc/modules/audio_processing/aec3/block_processor.cc
@@ -20,6 +20,8 @@
 namespace webrtc {
 namespace {
 
+enum class BlockProcessorApiCall { kCapture, kRender };
+
 class BlockProcessorImpl final : public BlockProcessor {
  public:
   BlockProcessorImpl(int sample_rate_hz,
@@ -33,24 +35,25 @@ class BlockProcessorImpl final : public BlockProcessor {
                       bool capture_signal_saturation,
                       std::vector<std::vector<float>>* capture_block) override;
 
-  bool BufferRender(std::vector<std::vector<float>>* block) override;
+  void BufferRender(std::vector<std::vector<float>>* block) override;
 
   void UpdateEchoLeakageStatus(bool leakage_detected) override;
 
  private:
   static int instance_count_;
+  bool no_capture_data_received_ = true;
+  bool no_render_data_received_ = true;
   std::unique_ptr<ApmDataDumper> data_dumper_;
   const size_t sample_rate_hz_;
   std::unique_ptr<RenderDelayBuffer> render_buffer_;
   std::unique_ptr<RenderDelayController> delay_controller_;
   std::unique_ptr<EchoRemover> echo_remover_;
   BlockProcessorMetrics metrics_;
+  bool render_buffer_overrun_occurred_ = false;
   RTC_DISALLOW_IMPLICIT_CONSTRUCTORS(BlockProcessorImpl);
 };
 
-constexpr size_t kRenderBufferSize = 250;
 int BlockProcessorImpl::instance_count_ = 0;
-constexpr size_t kMaxApiJitter = 30;
 
 BlockProcessorImpl::BlockProcessorImpl(
     int sample_rate_hz,
@@ -75,40 +78,88 @@ void BlockProcessorImpl::ProcessCapture(
   RTC_DCHECK(capture_block);
   RTC_DCHECK_EQ(NumBandsForRate(sample_rate_hz_), capture_block->size());
   RTC_DCHECK_EQ(kBlockSize, (*capture_block)[0].size());
-
-  const size_t delay = delay_controller_->GetDelay((*capture_block)[0]);
-  const bool render_delay_change = delay != render_buffer_->Delay();
-
-  if (render_delay_change) {
-    render_buffer_->SetDelay(delay);
+  data_dumper_->DumpRaw("aec3_processblock_call_order",
+                        static_cast<int>(BlockProcessorApiCall::kCapture));
+  data_dumper_->DumpWav("aec3_processblock_capture_input", kBlockSize,
+                        &(*capture_block)[0][0],
+                        LowestBandRate(sample_rate_hz_), 1);
+
+  // Do not start processing until render data has been buffered as that will
+  // cause the buffers to be wrongly aligned.
+  no_capture_data_received_ = false;
+  if (no_render_data_received_) {
+    return;
   }
 
-  if (render_buffer_->IsBlockAvailable()) {
-    auto& render_block = render_buffer_->GetNext();
-    echo_remover_->ProcessBlock(
-        delay_controller_->AlignmentHeadroomSamples(),
-        EchoPathVariability(echo_path_gain_change, render_delay_change),
-        capture_signal_saturation, render_block, capture_block);
-    metrics_.UpdateCapture(false);
+  data_dumper_->DumpWav("aec3_processblock_capture_input2", kBlockSize,
+                        &(*capture_block)[0][0],
+                        LowestBandRate(sample_rate_hz_), 1);
+
+  bool render_buffer_underrun = false;
+  if (render_buffer_overrun_occurred_) {
+    // Reset the render buffers and the alignment functionality when there has
+    // been a render buffer overrun as the buffer alignment may be noncausal.
+    delay_controller_->Reset();
+    render_buffer_->Reset();
   } else {
-    metrics_.UpdateCapture(true);
+    // Update the render buffers with new render data, filling the buffers with
+    // empty blocks when there is no render data available.
+    render_buffer_underrun = !render_buffer_->UpdateBuffers();
+
+    // Compute and and apply the render delay required to achieve proper signal
+    // alignment.
+    const size_t old_delay = render_buffer_->Delay();
+    const size_t new_delay = delay_controller_->GetDelay(
+        render_buffer_->GetDownsampledRenderBuffer(), (*capture_block)[0]);
+    render_buffer_->SetDelay(new_delay);
+    const size_t achieved_delay = render_buffer_->Delay();
+
+    // Inform the delay controller of the actually set delay to allow it to
+    // properly react to a non-feasible delay.
+    delay_controller_->SetDelay(achieved_delay);
+
+    // Remove the echo from the capture signal.
+    echo_remover_->ProcessCapture(
+        delay_controller_->AlignmentHeadroomSamples(),
+        EchoPathVariability(echo_path_gain_change,
+                            old_delay != achieved_delay ||
+                                old_delay != new_delay ||

[ivoc, 2017/03/31 13:58:31]

The formatting looks a bit funny here, is this really what 'git cl format' does?

[peah-webrtc, 2017/04/03 08:02:32]

Agree, that looks superstrange! I'll do a reformat on that using cl format.

[ivoc, 2017/04/03 16:09:06]

Looks like it didn't change, so maybe this is correct after all.

+                                render_buffer_overrun_occurred_),
+        capture_signal_saturation, render_buffer_->GetRenderBuffer(),
+        capture_block);
   }
+
+  // Update the metrics.
+  metrics_.UpdateCapture(render_buffer_underrun);
+
+  render_buffer_overrun_occurred_ = false;

[ivoc, 2017/03/31 13:58:31]

I think it's a bit more clear if this is moved into the if
(render_buffer_overrun_occurred) block.

[peah-webrtc, 2017/04/03 08:02:32]

I agree. There is, however, an issue with this code that is fixed in an upcoming
CL. The else, statement should always be run, regardless of whether the
if-statement passed.

That means that the render_buffer_overrun_occurred_ variable will be used in the
ProcessCapture call. Therefore, to avoid the risk of this being reset to early,
I'd prefer to keep it at the end of the method.

WDYT?

[ivoc, 2017/04/03 16:09:07]

Ok, that sounds reasonable.

 }
 
-bool BlockProcessorImpl::BufferRender(std::vector<std::vector<float>>* block) {
+void BlockProcessorImpl::BufferRender(std::vector<std::vector<float>>* block) {
   RTC_DCHECK(block);
   RTC_DCHECK_EQ(NumBandsForRate(sample_rate_hz_), block->size());
   RTC_DCHECK_EQ(kBlockSize, (*block)[0].size());
+  data_dumper_->DumpRaw("aec3_processblock_call_order",
+                        static_cast<int>(BlockProcessorApiCall::kRender));
+  data_dumper_->DumpWav("aec3_processblock_render_input", kBlockSize,
+                        &(*block)[0][0], LowestBandRate(sample_rate_hz_), 1);
+
+  no_render_data_received_ = false;
 
-  const bool delay_controller_overrun =
-      !delay_controller_->AnalyzeRender((*block)[0]);
-  const bool render_buffer_overrun = !render_buffer_->Insert(block);
-  if (delay_controller_overrun || render_buffer_overrun) {
-    metrics_.UpdateRender(true);
-    return false;
+  // Do not start buffer render data until capture data has been received as
+  // that data may give a false alignment.
+  if (no_capture_data_received_) {
+    return;
   }
-  metrics_.UpdateRender(false);
-  return true;
+
+  data_dumper_->DumpWav("aec3_processblock_render_input2", kBlockSize,
+                        &(*block)[0][0], LowestBandRate(sample_rate_hz_), 1);
+
+  // Buffer the render data.
+  render_buffer_overrun_occurred_ = !render_buffer_->Insert(block);
+
+  // Update the metrics.
+  metrics_.UpdateRender(render_buffer_overrun_occurred_);
 }
 
 void BlockProcessorImpl::UpdateEchoLeakageStatus(bool leakage_detected) {
@@ -118,10 +169,10 @@ void BlockProcessorImpl::UpdateEchoLeakageStatus(bool leakage_detected) {
 }  // namespace
 
 BlockProcessor* BlockProcessor::Create(int sample_rate_hz) {
-  std::unique_ptr<RenderDelayBuffer> render_buffer(RenderDelayBuffer::Create(
-      kRenderBufferSize, NumBandsForRate(sample_rate_hz), kMaxApiJitter));
+  std::unique_ptr<RenderDelayBuffer> render_buffer(
+      RenderDelayBuffer::Create(NumBandsForRate(sample_rate_hz)));
   std::unique_ptr<RenderDelayController> delay_controller(
-      RenderDelayController::Create(sample_rate_hz, *render_buffer));
+      RenderDelayController::Create(sample_rate_hz));
   std::unique_ptr<EchoRemover> echo_remover(
       EchoRemover::Create(sample_rate_hz));
   return Create(sample_rate_hz, std::move(render_buffer),
@@ -132,7 +183,7 @@ BlockProcessor* BlockProcessor::Create(
     int sample_rate_hz,
     std::unique_ptr<RenderDelayBuffer> render_buffer) {
   std::unique_ptr<RenderDelayController> delay_controller(
-      RenderDelayController::Create(sample_rate_hz, *render_buffer));
+      RenderDelayController::Create(sample_rate_hz));
   std::unique_ptr<EchoRemover> echo_remover(
       EchoRemover::Create(sample_rate_hz));
   return Create(sample_rate_hz, std::move(render_buffer),