Add aecdump support to audioproc_f.

webrtc/modules/audio_processing/test/audio_file_processor.cc

Index: webrtc/modules/audio_processing/test/audio_file_processor.cc
diff --git a/webrtc/modules/audio_processing/test/audio_file_processor.cc b/webrtc/modules/audio_processing/test/audio_file_processor.cc
new file mode 100644
index 0000000000000000000000000000000000000000..9cc71c4a3bc2ee98e5cc9a74922af372e2c1c616
--- /dev/null
+++ b/webrtc/modules/audio_processing/test/audio_file_processor.cc
@@ -0,0 +1,187 @@
+/*
+ *  Copyright (c) 2015 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/test/audio_file_processor.h"
+
+#include <algorithm>
+
+#include "webrtc/base/checks.h"
+#include "webrtc/modules/audio_processing/test/protobuf_utils.h"
+
+using rtc::scoped_ptr;
+using rtc::CheckedDivExact;
+using std::vector;
+using webrtc::audioproc::Event;
+using webrtc::audioproc::Init;
+using webrtc::audioproc::ReverseStream;
+using webrtc::audioproc::Stream;
+
+namespace webrtc {
+namespace {
+
+// Returns a StreamConfig corresponding to file.
+StreamConfig GetStreamConfig(const WavFile& file) {
+  return StreamConfig(file.sample_rate(), file.num_channels());
+}
+
+// Returns a ChannelBuffer corresponding to file.
+ChannelBuffer<float> GetChannelBuffer(const WavFile& file) {
+  return ChannelBuffer<float>(
+      CheckedDivExact(file.sample_rate(), AudioFileProcessor::kChunksPerSecond),

[peah-webrtc, 2015/10/20 21:17:25]

It would be good to be able to process wav files of any length, most likely by
padding to an even number of chunks before processing and then shortening the
output to be of the same length as the original input.

If the requirement of a length of an even number of chunks is imposed this means
that preprocessing is required in order to be able to apply the program to any
type of wav file.

[Andrew MacDonald, 2015/10/21 00:29:28]

This line is just about computing the buffer size, which the AudioProcessing
interface constrains to 10 ms anyway.

aecdumps may only contain full buffers, so we don't have to worry there.

For WAV input, this continues processing until we don't have a full buffer. So
it will process non-10ms-multiple files, but drop the last < 10 ms chunk, which
I think is reasonable behavior.

[peah-webrtc, 2015/10/21 08:10:04]

A, sorry about that. Now I see! Great!

The behavior you described for non-10ms-multiple files is super!

+      file.num_channels());
+}
+
+}  // namespace
+
+WavFileProcessor::WavFileProcessor(scoped_ptr<AudioProcessing> ap,
+                                   scoped_ptr<WavReader> in_file,
+                                   scoped_ptr<WavWriter> out_file)
+    : ap_(ap.Pass()),
+      in_file_(in_file.Pass()),
+      in_buf_(GetChannelBuffer(*in_file_)),
+      out_buf_(GetChannelBuffer(*out_file)),
+      in_interleaved_(in_buf_.size()),
+      input_config_(GetStreamConfig(*in_file_)),
+      output_config_(GetStreamConfig(*out_file)),
+      buffer_writer_(out_file.Pass()) {}
+
+bool WavFileProcessor::ProcessChunk() {
+  if (in_file_->ReadSamples(in_interleaved_.size(), &in_interleaved_[0]) !=
+      in_interleaved_.size()) {
+    return false;
+  }
+
+  FloatS16ToFloat(&in_interleaved_[0], in_interleaved_.size(),
+                  &in_interleaved_[0]);
+  Deinterleave(&in_interleaved_[0], in_buf_.num_frames(),
+               in_buf_.num_channels(), in_buf_.channels());
+
+  {
+    const auto st = ScopedTimer(processing_time());
+    RTC_CHECK_EQ(kNoErr,
+                 ap_->ProcessStream(in_buf_.channels(), input_config_,
+                                    output_config_, out_buf_.channels()));
+  }
+
+  buffer_writer_.Write(out_buf_);

[aluebs-webrtc, 2015/10/24 00:53:34]

It would be awesome to have a ChannelBufferWavReader as well. This
implementation would be much more straight forward.

[Andrew MacDonald, 2015/10/29 00:44:50]

I didn't bother because it's only used in one place (unlike CBWW which is used
twice). But I think the symmetry is nice actually; done.

[aluebs-webrtc, 2015/10/29 01:03:19]

Thank you for adding that! :)

+  return true;
+}
+
+AecDumpFileProcessor::AecDumpFileProcessor(scoped_ptr<AudioProcessing> ap,
+                                           FILE* dump_file,
+                                           scoped_ptr<WavWriter> out_file)
+    : ap_(ap.Pass()),
+      dump_file_(dump_file),
+      out_buf_(GetChannelBuffer(*out_file)),
+      output_config_(GetStreamConfig(*out_file)),
+      buffer_writer_(out_file.Pass()) {
+  RTC_CHECK(dump_file_);

[aluebs-webrtc, 2015/10/24 00:53:34]

RTC_DCHECK? And in the other places. Although this is only a test feature.

[Andrew MacDonald, 2015/10/29 00:44:50]

No, I want it to crash in release as well.

[aluebs-webrtc, 2015/10/29 01:03:19]

Agreed.

+}
+
+AecDumpFileProcessor::~AecDumpFileProcessor() {
+  fclose(dump_file_);
+}
+
+bool AecDumpFileProcessor::ProcessChunk() {
+  Event event_msg;
+
+  // Continue until we process our first Stream message.
+  do {
+    if (!ReadMessageFromFile(dump_file_, &event_msg)) {
+      return false;
+    }
+
+    if (event_msg.type() == Event::INIT) {
+      RTC_CHECK(event_msg.has_init());
+      HandleMessage(event_msg.init());
+
+    } else if (event_msg.type() == Event::STREAM) {
+      RTC_CHECK(event_msg.has_stream());
+      HandleMessage(event_msg.stream());
+      break;

[aluebs-webrtc, 2015/10/24 00:53:34]

This is not necessary, it will leave the dowhile loop anyway, right?

[Andrew MacDonald, 2015/10/29 00:44:50]

True, changed. I was using a different structure earlier that required this.

+
+    } else if (event_msg.type() == Event::REVERSE_STREAM) {
+      RTC_CHECK(event_msg.has_reverse_stream());
+      HandleMessage(event_msg.reverse_stream());
+    }
+  } while (event_msg.type() != Event::STREAM);
+
+  return true;
+}
+
+void AecDumpFileProcessor::HandleMessage(const Init& msg) {
+  RTC_CHECK(msg.has_sample_rate());
+  RTC_CHECK(msg.has_num_input_channels());
+  RTC_CHECK(msg.has_num_reverse_channels());
+
+  in_buf_.reset(new ChannelBuffer<float>(
+      CheckedDivExact(msg.sample_rate(), kChunksPerSecond),
+      msg.num_input_channels()));
+  const int reverse_sample_rate = msg.has_reverse_sample_rate()
+                                      ? msg.reverse_sample_rate()
+                                      : msg.sample_rate();
+  reverse_buf_.reset(new ChannelBuffer<float>(
+      CheckedDivExact(reverse_sample_rate, kChunksPerSecond),
+      msg.num_reverse_channels()));
+  input_config_ = StreamConfig(msg.sample_rate(), msg.num_input_channels());
+  reverse_config_ =
+      StreamConfig(msg.reverse_sample_rate(), msg.num_reverse_channels());

[aluebs-webrtc, 2015/10/24 00:53:34]

reverse_sample_rate

[Andrew MacDonald, 2015/10/29 00:44:50]

Did you intend to post this? Not sure what you mean.

[aluebs-webrtc, 2015/10/29 01:03:19]

I did actually intend to post this. I meant was, shouldn't it be:
msg.reverse_sample_rate() ---> reverse_sample_rate
For the case where !msg.has_reverse_sample_rate().

[Andrew MacDonald, 2015/10/29 01:14:33]

Aha, yes it should. Will fix.

[Andrew MacDonald, 2015/10/30 00:21:16]

Done.

+
+  const ProcessingConfig config = {
+      {input_config_, output_config_, reverse_config_, reverse_config_}};
+  RTC_CHECK_EQ(kNoErr, ap_->Initialize(config));
+}
+
+void AecDumpFileProcessor::HandleMessage(const Stream& msg) {
+  RTC_CHECK(!msg.has_input_data());
+  RTC_CHECK_EQ(in_buf_->num_channels(), msg.input_channel_size());

[aluebs-webrtc, 2015/10/24 00:53:34]

The APM has support to change this dynamically. Why not take advantage of this?
And for the reverse stream as well?

[Andrew MacDonald, 2015/10/29 00:44:50]

We do take advantage of it. Whenever the stream format changes APM will record
an initialization event *before* the stream event. We recreate the buffers while
handling the initialization event and therefore expect the channels to always
match here.

[aluebs-webrtc, 2015/10/29 01:03:19]

Oh, I was not aware that the INIT msg was required then.

[Andrew MacDonald, 2015/10/29 01:14:33]

It's a little confusing, because an explicit user init call is not required to
cause an init message. It will be recorded when APM initializes for any reason.

+
+  for (int i = 0; i < msg.input_channel_size(); ++i) {
+    RTC_CHECK_EQ(in_buf_->num_frames() * sizeof(*in_buf_->channels()[i]),
+                 msg.input_channel(i).size());
+    std::memcpy(in_buf_->channels()[i], msg.input_channel(i).data(),

[aluebs-webrtc, 2015/10/24 00:53:34]

Check the output of memcpy? And for the reverse stream as well?

[Andrew MacDonald, 2015/10/29 00:44:50]

http://en.cppreference.com/w/cpp/string/byte/memcpy

The return value is just |dest|. memcpy provides no safety.

[aluebs-webrtc, 2015/10/29 01:03:19]

My whole life was a lie! :O

[Andrew MacDonald, 2015/10/29 01:14:33]

:-)

+                msg.input_channel(i).size());
+  }
+  {
+    const auto st = ScopedTimer(processing_time());

[aluebs-webrtc, 2015/10/24 00:53:34]

Shouldn't this scope be much more tight around ProcessStream to reflect the real
time? And for the reverse stream as well?

[Andrew MacDonald, 2015/10/29 00:44:50]

I think it's fair to include the other API calls to APM in the execution time.

[aluebs-webrtc, 2015/10/29 01:03:19]

I guess it depends on what you want to measure and in practice I don't think it
would affect. So you can leave it as is.

+    RTC_CHECK_EQ(kNoErr, ap_->set_stream_delay_ms(msg.delay()));
+    ap_->echo_cancellation()->set_stream_drift_samples(msg.drift());
+    if (msg.has_keypress()) {
+      ap_->set_stream_key_pressed(msg.keypress());
+    }
+    RTC_CHECK_EQ(kNoErr,
+                 ap_->ProcessStream(in_buf_->channels(), input_config_,
+                                    output_config_, out_buf_.channels()));
+  }
+
+  buffer_writer_.Write(out_buf_);
+}
+
+void AecDumpFileProcessor::HandleMessage(const ReverseStream& msg) {
+  RTC_CHECK(!msg.has_data());
+  RTC_CHECK_EQ(reverse_buf_->num_channels(), msg.channel_size());
+
+  for (int i = 0; i < msg.channel_size(); ++i) {
+    RTC_CHECK_EQ(reverse_buf_->num_frames() * sizeof(*in_buf_->channels()[i]),
+                 msg.channel(i).size());
+    std::memcpy(reverse_buf_->channels()[i], msg.channel(i).data(),
+                msg.channel(i).size());
+  }
+  {
+    const auto st = ScopedTimer(processing_time());

[aluebs-webrtc, 2015/10/24 00:53:34]

This will take into account in the same time capture and render times? Do we
want to have 2 different timers?

[Andrew MacDonald, 2015/10/29 00:44:50]

We didn't before. That might be interesting, but I think it would be better to
run a manual profile if you want that kind of granularity.

[aluebs-webrtc, 2015/10/29 01:03:19]

Makes sense. And if wanted, it would be better to do another CL anyway :)

+    // TODO(ajm): This currently discards the processed output, which is needed
+    // for e.g. intelligibility enhancement.
+    RTC_CHECK_EQ(kNoErr, ap_->ProcessReverseStream(
+                             reverse_buf_->channels(), reverse_config_,
+                             reverse_config_, reverse_buf_->channels()));
+  }
+}
+
+}  // namespace webrtc