Added first layer of the echo canceller 3 functionality

webrtc/modules/audio_processing/aec3/echo_canceller3.cc

 /*
  *  Copyright (c) 2016 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/echo_canceller3.h"
 
 #include "webrtc/base/atomicops.h"
-#include "webrtc/system_wrappers/include/logging.h"
+#include "webrtc/modules/audio_processing/logging/apm_data_dumper.h"
 
 namespace webrtc {
 
-int EchoCanceller3::instance_count_ = 0;
-
-EchoCanceller3::EchoCanceller3(int sample_rate_hz, bool use_anti_hum_filter) {
-  int band_sample_rate_hz = (sample_rate_hz == 8000 ? sample_rate_hz : 16000);
-  frame_length_ = rtc::CheckedDivExact(band_sample_rate_hz, 100);
-
-  LOG(LS_INFO) << "AEC3 created : "
-               << "{ instance_count: " << instance_count_ << "}";
-  instance_count_ = rtc::AtomicOps::Increment(&instance_count_);
-}
-
-EchoCanceller3::~EchoCanceller3() = default;
-
-bool EchoCanceller3::AnalyzeRender(AudioBuffer* render) {
+namespace {
+
+bool DetectSaturation(rtc::ArrayView<const float> y) {
+  for (auto y_k : y) {
+    if (y_k > 32767.0f || y_k < -32768.0f) {

[ivoc, 2016/12/19 11:30:22]

How about if the signals are equal to the limits? 

[peah-webrtc, 2016/12/20 10:10:25]

Good find!
Done.

+      return true;
+    }
+  }
+  return false;
+}
+
+void ProcessCaptureFrameContent(AudioBuffer* capture,
+                                bool known_echo_path_change,
+                                bool saturated_microphone_signal,
+                                size_t subframe_index,
+                                FrameBlocker* capture_blocker,
+                                BlockFramer* output_framer,
+                                BlockProcessor* block_processor) {
+  float capture_block[kMaxNumBands * kBlockSize];
+  capture_blocker->InsertFrameAndExtractBlock(
+      subframe_index, capture->split_bands_f(0), capture_block);
+  block_processor->ProcessCapture(known_echo_path_change,
+                                  saturated_microphone_signal, capture_block);
+  output_framer->InsertBlock(capture_block);
+  output_framer->ExtractFrame(subframe_index, capture->split_bands_f(0));
+}
+
+void ProcessRemainingCaptureFrameContent(bool known_echo_path_change,
+                                         bool saturated_microphone_signal,
+                                         FrameBlocker* capture_blocker,
+                                         BlockFramer* output_framer,
+                                         BlockProcessor* block_processor) {
+  if (capture_blocker->IsBlockAvailable()) {

[hlundin-webrtc, 2016/12/16 10:04:47]

Nit: I prefer early return.
if (!capture_blocker->IsBlockAvailable()) {
  return;
}
...

[peah-webrtc, 2016/12/20 10:10:25]

Done.

+    float capture_block[kMaxNumBands * kBlockSize];
+    capture_blocker->ExtractBlockIfAvailable(capture_block);

[hlundin-webrtc, 2016/12/16 10:04:47]

...IfAvailable? I thought we established that it was available.

[peah-webrtc, 2016/12/20 10:10:25]

True, I changed the implementation of FrameBlocker accordingly.

Done.

+    block_processor->ProcessCapture(known_echo_path_change,
+                                    saturated_microphone_signal, capture_block);
+    output_framer->InsertBlock(capture_block);
+  }
+}
+
+bool BufferRenderFrameContent(rtc::ArrayView<const float> render_frame,
+                              size_t subframe_index,
+                              FrameBlocker* render_blocker,
+                              BlockProcessor* block_processor) {
+  return block_processor->BufferRender(
+      [render_blocker, render_frame,
+       subframe_index](rtc::ArrayView<float> block) mutable {
+        render_blocker->InsertFrameAndExtractBlock(subframe_index, render_frame,
+                                                   block);
+      });

[ivoc, 2016/12/19 11:30:22]

Can you explain what this is supposed to do? The way the code looks now doesn't
make much sense to me. For example, this lambda is passed to the block processor
(why?), and is never executed there. I guess some steps will be added later?

[peah-webrtc, 2016/12/20 10:10:25]

Yes, the reason for this construct is to provide a cheap way of producing a
render block from a frame and passing it to the BufferRender method. The reason
for why it is not used in the block_processor is that that functionality is to
be added in the next CL.

However, with the code changes in the new patch, this lambda is no longer used
and I instead use the vector-swap functionality for efficiently passing the
blocks.

PTAL

[ivoc, 2016/12/21 13:04:04]

It looks easier to understand now.

[peah-webrtc, 2016/12/21 23:13:48]

Acknowledged.

+}
+
+bool BufferRemainingRenderFrameContent(FrameBlocker* render_blocker,
+                                       BlockProcessor* block_processor) {
+  if (render_blocker->IsBlockAvailable()) {
+    return block_processor->BufferRender(
+        [render_blocker](rtc::ArrayView<float> block) mutable {
+          render_blocker->ExtractBlockIfAvailable(block);
+        });
+  }
+  return false;
+}
+
+void CopyAudioBufferIntoFrame(AudioBuffer* buffer,

[hlundin-webrtc, 2016/12/16 10:04:47]

I assume this cannot be made a const argument because of magic happening inside
the buffer.

[peah-webrtc, 2016/12/20 10:10:25]

Afaics, that is the case.

+                              size_t num_bands,
+                              size_t frame_length,
+                              rtc::ArrayView<float> frame) {
+  RTC_DCHECK_EQ(num_bands * frame_length, frame.size());
+  for (size_t i = 0; i < num_bands; ++i) {
+    rtc::ArrayView<float> buffer_view(&buffer->split_bands_f(0)[i][0],
+                                      frame_length);
+    std::copy(buffer_view.begin(), buffer_view.end(),
+              frame.begin() + i * frame_length);
+  }
+}
+
+// [B,A] = butter(2,100/4000,'high')
+const CascadedBiQuadFilter::BiQuadCoefficients
+    kHighPassFilterCoefficients_8kHz = {
+        {0.945976856002790, -1.891953712005580, 0.945976856002790},
+        {-1.889033079394525, 0.894874344616636}};
+const int kNumberOfHighPassBiQuads_8kHz = 1;
+
+// [B,A] = butter(2,100/8000,'high')
+const CascadedBiQuadFilter::BiQuadCoefficients
+    kHighPassFilterCoefficients_16kHz = {
+        {0.972613898499844, -1.945227796999688, 0.972613898499844},
+        {-1.944477657767094, 0.945977936232282}};
+const int kNumberOfHighPassBiQuads_16kHz = 1;
+
+}  // namespace
+
+EchoCanceller3::RenderWriterState::RenderWriterState(
+    ApmDataDumper* data_dumper,
+    RenderTransferBufferWriter* transfer_buffer_writer,
+    std::unique_ptr<CascadedBiQuadFilter> render_highpass_filter,
+    int sample_rate_hz,
+    int frame_length,
+    int num_bands)
+    : data_dumper_(data_dumper),

[hlundin-webrtc, 2016/12/16 10:04:47]

Is nullptr valid? Otherwise, add a DCHECK.

[peah-webrtc, 2016/12/20 10:10:25]

Done.

+      sample_rate_hz_(sample_rate_hz),
+      frame_length_(frame_length),
+      num_bands_(num_bands),
+      transfer_buffer_writer_(transfer_buffer_writer),
+      render_highpass_filter_(std::move(render_highpass_filter)),
+      render_queue_input_frame_(num_bands_ * frame_length_, 0.f) {}
+
+EchoCanceller3::RenderWriterState::~RenderWriterState() = default;
+
+bool EchoCanceller3::RenderWriterState::Insert(AudioBuffer* render) {

[hlundin-webrtc, 2016/12/16 10:04:46]

"render" is a dubious name here. I think "input" would me more descriptive.

[peah-webrtc, 2016/12/20 10:10:25]

Done.

   RTC_DCHECK_EQ(1u, render->num_channels());

[hlundin-webrtc, 2016/12/16 10:04:47]

Nit: I think you no longer need the 'u' suffix, as a result of
https://codereview.webrtc.org/2535593002.

[peah-webrtc, 2016/12/20 10:10:25]

Done.

   RTC_DCHECK_EQ(frame_length_, render->num_frames_per_band());
-  return true;
-}
-
-void EchoCanceller3::AnalyzeCapture(AudioBuffer* capture) {}
+  data_dumper_->DumpWav("aec3_render_input", frame_length_,
+                        &render->split_bands_f(0)[0][0],
+                        LowestBandRate(sample_rate_hz_), 1);
+
+  CopyAudioBufferIntoFrame(render, num_bands_, frame_length_,
+                           render_queue_input_frame_);
+
+  if (render_highpass_filter_) {
+    render_highpass_filter_->Process(
+        rtc::ArrayView<float>(&render_queue_input_frame_[0], frame_length_));
+  }
+
+  return transfer_buffer_writer_->Insert(&render_queue_input_frame_);
+}
+
+int EchoCanceller3::instance_count_ = 0;
+
+EchoCanceller3::EchoCanceller3(int sample_rate_hz, bool use_highpass_filter)
+    : data_dumper_(new ApmDataDumper(instance_count_)),
+      sample_rate_hz_(sample_rate_hz),
+      num_bands_(sample_rate_hz_ == 8000 ? 1 : sample_rate_hz / 16000),

[hlundin-webrtc, 2016/12/16 10:04:47]

You have this expression defined as NumBandsForRate(sample_rate_hz) in
aec3_constants.h.

[peah-webrtc, 2016/12/20 10:10:25]

Done.

+      frame_length_(rtc::CheckedDivExact(LowestBandRate(sample_rate_hz_), 100)),
+      output_framer_(num_bands_, frame_length_),
+      capture_blocker_(num_bands_, frame_length_),
+      render_blocker_(num_bands_, frame_length_),
+      render_transfer_buffer_(num_bands_, frame_length_),
+      block_processor_(data_dumper_.get(), sample_rate_hz, num_bands_),
+      render_queue_output_frame_(num_bands_ * frame_length_, 0.f) {
+  std::unique_ptr<CascadedBiQuadFilter> render_highpass_filter;
+  if (use_highpass_filter) {
+    render_highpass_filter.reset(new CascadedBiQuadFilter(
+        sample_rate_hz_ == 8000 ? kHighPassFilterCoefficients_8kHz
+                                : kHighPassFilterCoefficients_16kHz,
+        sample_rate_hz_ == 8000 ? kNumberOfHighPassBiQuads_8kHz
+                                : kNumberOfHighPassBiQuads_16kHz));
+    capture_highpass_filter_.reset(new CascadedBiQuadFilter(
+        sample_rate_hz_ == 8000 ? kHighPassFilterCoefficients_8kHz
+                                : kHighPassFilterCoefficients_16kHz,
+        sample_rate_hz_ == 8000 ? kNumberOfHighPassBiQuads_8kHz
+                                : kNumberOfHighPassBiQuads_16kHz));
+  }
+
+  render_writer_.reset(
+      new RenderWriterState(data_dumper_.get(), &render_transfer_buffer_,
+                            std::move(render_highpass_filter), sample_rate_hz_,

[hlundin-webrtc, 2016/12/16 10:04:47]

What happens when you move from an (explicitly) uninitialized unique_ptr? Are
you sure you get a nullptr as expected? From
http://en.cppreference.com/w/cpp/memory/unique_ptr/unique_ptr, it seems that the
default ctor value-initializes the pointer, which I assume sets it to nullptr.

[peah-webrtc, 2016/12/20 10:10:25]

Good point! I cannot find whether it actually initializes it to null though. I
added that specifically.

PTAL 

+                            frame_length_, num_bands_));
+
+  RTC_DCHECK_EQ(num_bands_, std::max(sample_rate_hz_, 16000) / 16000);
+  RTC_DCHECK_GE(kMaxNumBands, num_bands_);
+  instance_count_ = rtc::AtomicOps::Increment(&instance_count_);
+}
+
+EchoCanceller3::~EchoCanceller3() = default;
+
+bool EchoCanceller3::AnalyzeRender(AudioBuffer* render) {
+  return render_writer_->Insert(render);
+}
+
+bool EchoCanceller3::EmptyRenderQueue() {

[hlundin-webrtc, 2016/12/16 10:04:47]

Check the order of the methods in this file, and make sure it is the same as in
the h-file.

[peah-webrtc, 2016/12/20 10:10:25]

Done.

+  bool render_buffer_overrun = false;
+  bool frame_to_buffer =
+      render_transfer_buffer_.Remove(&render_queue_output_frame_);
+  while (frame_to_buffer) {
+    render_buffer_overrun |= BufferRenderFrameContent(
+        render_queue_output_frame_, 0, &render_blocker_, &block_processor_);
+
+    if (sample_rate_hz_ != 8000) {
+      render_buffer_overrun |= BufferRenderFrameContent(
+          render_queue_output_frame_, 1, &render_blocker_, &block_processor_);
+    }
+
+    render_buffer_overrun |=
+        BufferRemainingRenderFrameContent(&render_blocker_, &block_processor_);
+
+    frame_to_buffer =
+        render_transfer_buffer_.Remove(&render_queue_output_frame_);
+  }
+  return render_buffer_overrun;
+}
+
+void EchoCanceller3::AnalyzeCapture(AudioBuffer* capture) {
+  data_dumper_->DumpWav("aec3_capture_analyze_input", frame_length_,
+                        capture->channels_f()[0], sample_rate_hz_, 1);
+
+  saturated_microphone_signal_ = false;

[hlundin-webrtc, 2016/12/16 10:04:47]

You reset saturated_microphone_signal_ both here and at the end of
ProcessCapture. Is this intentional?

[peah-webrtc, 2016/12/20 10:10:25]

Done.

+  for (size_t k = 0; k < capture->num_channels(); ++k) {
+    saturated_microphone_signal_ |=

[hlundin-webrtc, 2016/12/16 10:04:47]

I assume you will expand the work of this for loop later one. Otherwise, you can
do early return on finding the first saturation.
for () {
  if (DetectSaturation(blah) {
    saturated_microphone_signal_ = true;
    return;
  }
}

[peah-webrtc, 2016/12/20 10:10:25]

There will be no more work on this loop. I added early return.

Done.

+        DetectSaturation(rtc::ArrayView<const float>(capture->channels_f()[k],
+                                                     capture->num_frames()));
+  }
+}
 
 void EchoCanceller3::ProcessCapture(AudioBuffer* capture,
                                     bool known_echo_path_change) {
   RTC_DCHECK_EQ(1u, capture->num_channels());
   RTC_DCHECK_EQ(frame_length_, capture->num_frames_per_band());
+
+  rtc::ArrayView<float> capture_lower_band =
+      rtc::ArrayView<float>(&capture->split_bands_f(0)[0][0], frame_length_);
+
+  data_dumper_->DumpWav("aec3_capture_input", capture_lower_band,
+                        LowestBandRate(sample_rate_hz_), 1);
+
+  bool render_buffer_overrun = EmptyRenderQueue();
+  RTC_DCHECK(!render_buffer_overrun);
+
+  if (capture_highpass_filter_) {
+    capture_highpass_filter_->Process(capture_lower_band);
+  }
+
+  ProcessCaptureFrameContent(capture, known_echo_path_change,
+                             saturated_microphone_signal_, 0, &capture_blocker_,
+                             &output_framer_, &block_processor_);
+
+  if (sample_rate_hz_ != 8000) {
+    ProcessCaptureFrameContent(
+        capture, known_echo_path_change, saturated_microphone_signal_, 1,
+        &capture_blocker_, &output_framer_, &block_processor_);
+  }
+
+  ProcessRemainingCaptureFrameContent(
+      known_echo_path_change, saturated_microphone_signal_, &capture_blocker_,
+      &output_framer_, &block_processor_);
+
+  data_dumper_->DumpWav("aec3_capture_output", frame_length_,
+                        &capture->split_bands_f(0)[0][0],
+                        LowestBandRate(sample_rate_hz_), 1);
+
+  saturated_microphone_signal_ = false;
 }
 
 std::string EchoCanceller3::ToString(
     const AudioProcessing::Config::EchoCanceller3& config) {
   std::stringstream ss;
   ss << "{"
      << "enabled: " << (config.enabled ? "true" : "false") << "}";
   return ss.str();
 }
 
 bool EchoCanceller3::Validate(
     const AudioProcessing::Config::EchoCanceller3& config) {
   return true;
 }
 
 }  // namespace webrtc