Major AEC3 render pipeline changes

webrtc/modules/audio_processing/aec3/render_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.
  */
 
-#include "webrtc/modules/audio_processing/aec3/fft_buffer.h"
+#include "webrtc/modules/audio_processing/aec3/render_buffer.h"
 
 #include <algorithm>
 
 #include "webrtc/base/checks.h"
 #include "webrtc/modules/audio_processing/aec3/aec3_common.h"
 
 namespace webrtc {
 
-FftBuffer::FftBuffer(Aec3Optimization optimization,
-                     size_t num_partitions,
-                     const std::vector<size_t> num_ffts_for_spectral_sums)
+RenderBuffer::RenderBuffer(Aec3Optimization optimization,
+                           size_t num_bands,
+                           size_t num_partitions,
+                           const std::vector<size_t> num_ffts_for_spectral_sums)
     : optimization_(optimization),
       fft_buffer_(num_partitions),
       spectrum_buffer_(num_partitions, std::array<float, kFftLengthBy2Plus1>()),
       spectral_sums_(num_ffts_for_spectral_sums.size(),
-                     std::array<float, kFftLengthBy2Plus1>()) {
+                     std::array<float, kFftLengthBy2Plus1>()),
+      last_block_(num_bands, std::vector<float>(kBlockSize, 0.f)) {
   // Current implementation only allows a maximum of one spectral sum lengths.
   RTC_DCHECK_EQ(1, num_ffts_for_spectral_sums.size());
   spectral_sums_length_ = num_ffts_for_spectral_sums[0];
   RTC_DCHECK_GE(fft_buffer_.size(), spectral_sums_length_);
 
+  Clear();
+}
+
+RenderBuffer::~RenderBuffer() = default;
+
+void RenderBuffer::Clear() {
+  position_ = 0;
   for (auto& sum : spectral_sums_) {
     sum.fill(0.f);
   }
 
   for (auto& spectrum : spectrum_buffer_) {
     spectrum.fill(0.f);
   }
 
   for (auto& fft : fft_buffer_) {
     fft.Clear();
   }
+
+  for (auto& b : last_block_) {
+    std::fill(b.begin(), b.end(), 0.f);
+  }
 }
 
-FftBuffer::~FftBuffer() = default;
+void RenderBuffer::Insert(const std::vector<std::vector<float>>& block) {
+  // Compute the FFT of the data in the lowest band.

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

A DCHECK to verify the size of block would be useful.

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

Done.

+  FftData X;
+  fft_.PaddedFft(block[0], last_block_[0], &X);
 
-void FftBuffer::Insert(const FftData& fft) {
-  // Insert the fft into the buffer.
+  // Copy the last render frame.
+  for (size_t k = 0; k < block.size(); ++k) {
+    std::copy(block[k].begin(), block[k].end(), last_block_[k].begin());

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

A DHECK for the size of block[k] would be good.

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

Done.

+  }
+
+  // Insert X into the buffer.
   position_ = (position_ - 1 + fft_buffer_.size()) % fft_buffer_.size();
-  fft_buffer_[position_].Assign(fft);
+  fft_buffer_[position_].Assign(X);
 
   // Compute and insert the spectrum for the FFT into the spectrum buffer.
-  fft.Spectrum(optimization_, &spectrum_buffer_[position_]);
+  X.Spectrum(optimization_, &spectrum_buffer_[position_]);
 
   // Pre-compute and cachec the spectral sums.

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

cache

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

Done.

   std::copy(spectrum_buffer_[position_].begin(),
             spectrum_buffer_[position_].end(), spectral_sums_[0].begin());
   size_t position = (position_ + 1) % fft_buffer_.size();
   for (size_t j = 1; j < spectral_sums_length_; ++j) {
     const std::array<float, kFftLengthBy2Plus1>& spectrum =
         spectrum_buffer_[position];
 
     for (size_t k = 0; k < spectral_sums_[0].size(); ++k) {
       spectral_sums_[0][k] += spectrum[k];
     }
 
     position = position < (fft_buffer_.size() - 1) ? position + 1 : 0;

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

I think it would be nicer to update position in a similar way to the previous,
so position = (position_ + 1) % fft_buffer_.size();

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

I agree that the code becomes nicer. However, the % adds a division, which in
this case would mean that 12 (spectral_sums_length_==12) divisions are added per
block if we do that. Therefore, I think that this construct is better, even
though it definitely is less readable.

The difference to what is done on line 70, is that it is not done inside a loop
so the total cost is less. I think, however, that it is not worth the
computational cost even then so I'll change that one to a conditional as well.

WDYT?

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

Ok, let's keep it this way then.

   }
 }
 
 }  // namespace webrtc