Corrected the behavior in AEC3 during buffer overruns and underruns

webrtc/modules/audio_processing/aec3/render_delay_buffer.cc

 /*
  *  Copyright (c) 2017 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.
  */
 
@@ skipping 79 matching lines @@
 
  private:
   const Aec3Optimization optimization_;
   std::array<std::vector<std::vector<float>>, kRenderDelayBufferSize> buffer_;
   size_t delay_ = 0;
   size_t last_insert_index_ = 0;
   RenderBuffer fft_buffer_;
   DownsampledRenderBuffer downsampled_render_buffer_;
   DecimatorBy4 render_decimator_;
   ApiCallJitterBuffer api_call_jitter_buffer_;
+  const std::vector<std::vector<float>> zero_block_;
   RTC_DISALLOW_IMPLICIT_CONSTRUCTORS(RenderDelayBufferImpl);
 };
 
 RenderDelayBufferImpl::RenderDelayBufferImpl(size_t num_bands)
     : optimization_(DetectOptimization()),
       fft_buffer_(
           optimization_,
           num_bands,
           std::max(kResidualEchoPowerRenderWindowSize, kAdaptiveFilterLength),
           std::vector<size_t>(1, kAdaptiveFilterLength)),
-      api_call_jitter_buffer_(num_bands) {
+      api_call_jitter_buffer_(num_bands),
+      zero_block_(num_bands, std::vector<float>(kBlockSize, 0.f)) {
   buffer_.fill(std::vector<std::vector<float>>(
       num_bands, std::vector<float>(kBlockSize, 0.f)));
 
   RTC_DCHECK_LT(buffer_.size(), downsampled_render_buffer_.buffer.size());
 }
 
 RenderDelayBufferImpl::~RenderDelayBufferImpl() = default;
 
 void RenderDelayBufferImpl::Reset() {
   // Empty all data in the buffers.
@@ skipping 19 matching lines @@
     // Report buffer overrun and let the caller handle the overrun.
     return false;
   }
   api_call_jitter_buffer_.Insert(block);
 
   return true;
 }
 
 bool RenderDelayBufferImpl::UpdateBuffers() {
   bool underrun = true;
-  // Update the buffers with a new block if such is available, otherwise repeat
-  // the previous block.
+  // Update the buffers with a new block if such is available, otherwise insert
+  // a block of silence.
   if (api_call_jitter_buffer_.Size() > 0) {
     last_insert_index_ = (last_insert_index_ + 1) % buffer_.size();
     api_call_jitter_buffer_.Remove(&buffer_[last_insert_index_]);
-
     underrun = false;
   }
 
   downsampled_render_buffer_.position =
       (downsampled_render_buffer_.position - kSubBlockSize +
        downsampled_render_buffer_.buffer.size()) %
       downsampled_render_buffer_.buffer.size();
 
   std::array<float, kSubBlockSize> render_downsampled;
-  render_decimator_.Decimate(buffer_[last_insert_index_][0],
-                             render_downsampled);
+  if (!underrun) {

[aleloi, 2017/05/03 11:23:39]

I think if/else statements should check for the positive case, 'if (underrun) {}
else {}', unless there is a reason not to. Then the reader has to do one mental
step less. In this case, I don't see a reason to have the negative case first.

[peah-webrtc, 2017/05/03 11:31:23]

I agree, good point! I prefer to have the most normal case in the if-section and
the exception in the else-section. But with the underrun flag it becomes weird.
Will change.

Done.

+    render_decimator_.Decimate(buffer_[last_insert_index_][0],
+                               render_downsampled);
+  } else {
+    render_decimator_.Decimate(zero_block_[0], render_downsampled);
+  }
   std::copy(render_downsampled.rbegin(), render_downsampled.rend(),
             downsampled_render_buffer_.buffer.begin() +
                 downsampled_render_buffer_.position);
 
-  fft_buffer_.Insert(
-      buffer_[(last_insert_index_ - delay_ + buffer_.size()) % buffer_.size()]);
+  if (!underrun) {

[aleloi, 2017/05/03 11:23:39]

Same here.

[peah-webrtc, 2017/05/03 11:31:22]

Done.

+    fft_buffer_.Insert(buffer_[(last_insert_index_ - delay_ + buffer_.size()) %
+                               buffer_.size()]);
+  } else {
+    fft_buffer_.Insert(zero_block_);
+  }
   return !underrun;
 }
 
 void RenderDelayBufferImpl::SetDelay(size_t delay) {
   if (delay_ == delay) {
     return;
   }
 
   // If there is a new delay set, clear the fft buffer.
   fft_buffer_.Clear();
@@ skipping 16 matching lines @@
   }
 }
 
 }  // namespace
 
 RenderDelayBuffer* RenderDelayBuffer::Create(size_t num_bands) {
   return new RenderDelayBufferImpl(num_bands);
 }
 
 }  // namespace webrtc