AecDump implementation.

webrtc/modules/audio_processing/aec_dump/capture_stream_info_impl.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/aec_dump/aec_dump.h"
+
+#include "webrtc/base/ignore_wundef.h"
+// Files generated at build-time by the protobuf compiler.
+RTC_PUSH_IGNORING_WUNDEF()
+#ifdef WEBRTC_ANDROID_PLATFORM_BUILD
+#include "external/webrtc/webrtc/modules/audio_processing/debug.pb.h"
+#else
+#include "webrtc/modules/audio_processing/debug.pb.h"
+#endif
+RTC_POP_IGNORING_WUNDEF()
+
+namespace webrtc {
+AecDump::CaptureStreamInfo::CaptureStreamInfo()
+    : event_(new audioproc::Event()) {

[peah-webrtc, 2017/05/09 07:14:57]

I think the separation of the capture stream information writing into a separate
class is a bit unfortunate. Sorry for not flagging for this before but it was
not clear to me when reviewing the code for the generic aec_dump interface.

My concerns are the following:
-The separation of this functionality from AecDump is the only thing that
enforces creation of separate Events. In the current implementation, the events
for render and capture are cached in order to avoid having to create new events
for each frame. If creation of new Event objects for each frame is not needed, I
think it should not be introduced in this CL (which has the main purpose of
reducing the cost of writing AECdumps). 

-As it is now, all the writing of the capture information is abstracted to this
class while the writing of the render information is still in the aec_dump.cc.

-The usage of CaptureStreamInfo requires creation of a new CaptureStreamInfo
object for each capture frame, right? (as GetEventMessage moves the ownership of
event_)

[aleloi, 2017/05/12 13:07:56]

Changed after offline discussion.

To avoid creating new CaptureStreamInfos, AecDump holds a *single*
CaptureStreamInfoImpl. A CaptureStreamInfoImpl holds a WriteToFileTask, which
has an audioproc::Event. APM gets a pointer (with CaptureStreamInfo interface).
Then APM calls aec_dump_->LogCaptureStream() or
aec_dump_->LogCaptureStream(CaptureStreamInfo*). Exact interface is TBD.

This calls AecDumpImpl to send the WriteToFileTask owned by its
CaptureStreamInfoImpl instance to the task queue. A new WriteToFileTask owned by
CaptureStreamInfoImpl is created.

CaptureStreamInfoImpl holds the state needed to group all the capture stream
data 
that is logged in one protobuf message.

+  RTC_DCHECK(event_);
+  event_->set_type(audioproc::Event::STREAM);
+}
+
+AecDump::CaptureStreamInfo::~CaptureStreamInfo() {
+  // |event_| can't be in a unique_ptr, because of the no-protobuf
+  // implementation of AecDump::CaptureStreamInfo. In the no-protobuf
+  // case, a forward-declared audioproc::Event would have to be
+  // destroyed.
+  if (event_) {
+    delete event_;
+  }
+}
+
+void AecDump::CaptureStreamInfo::AddInput(FloatAudioFrame src) {
+  auto* stream = event_->mutable_stream();
+
+  for (size_t i = 0; i < src.num_channels(); ++i) {
+    const auto& channel_view = src.channel(i);
+    stream->add_input_channel(channel_view.begin(),
+                              sizeof(float) * channel_view.size());
+  }
+}
+
+void AecDump::CaptureStreamInfo::AddOutput(FloatAudioFrame src) {
+  auto* stream = event_->mutable_stream();
+
+  for (size_t i = 0; i < src.num_channels(); ++i) {
+    const auto& channel_view = src.channel(i);
+    stream->add_output_channel(channel_view.begin(),
+                               sizeof(float) * channel_view.size());
+  }
+}
+
+void AecDump::CaptureStreamInfo::AddInput(const AudioFrame& frame) {
+  audioproc::Stream* stream = event_->mutable_stream();
+  const size_t data_size =
+      sizeof(int16_t) * frame.samples_per_channel_ * frame.num_channels_;
+  stream->set_input_data(frame.data_, data_size);
+}
+
+void AecDump::CaptureStreamInfo::AddOutput(const AudioFrame& frame) {
+  audioproc::Stream* stream = event_->mutable_stream();
+  const size_t data_size =
+      sizeof(int16_t) * frame.samples_per_channel_ * frame.num_channels_;
+  stream->set_output_data(frame.data_, data_size);
+}
+
+void AecDump::CaptureStreamInfo::set_delay(int delay) {
+  event_->mutable_stream()->set_delay(delay);
+}
+void AecDump::CaptureStreamInfo::set_drift(int drift) {
+  event_->mutable_stream()->set_drift(drift);
+}
+void AecDump::CaptureStreamInfo::set_level(int level) {
+  event_->mutable_stream()->set_level(level);
+}
+void AecDump::CaptureStreamInfo::set_keypress(bool keypress) {
+  event_->mutable_stream()->set_keypress(keypress);
+}
+
+std::unique_ptr<audioproc::Event> AecDump::CaptureStreamInfo::GetEventMsg() {

[peah-webrtc, 2017/05/09 07:14:57]

This part of the class is a bit error prone. It means that none of methods in
the class can be used after the GetEventMsg() method has been called.

[aleloi, 2017/05/12 13:07:56]

The code is rather different now after the CaptureStreamInfo changes.

+  auto result = std::unique_ptr<audioproc::Event>(event_);
+  event_ = nullptr;
+  return result;
+}
+}  // namespace webrtc