AecDump implementation.

webrtc/modules/audio_processing/aec_dump/aec_dump.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/checks.h"
+#include "webrtc/base/event.h"
+#include "webrtc/base/ignore_wundef.h"
+#include "webrtc/base/protobuf_utils.h"
+#include "webrtc/modules/audio_processing/aec_dump/write_to_file_task.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 {
 
 InternalAPMConfig::InternalAPMConfig() = default;
 InternalAPMConfig::InternalAPMConfig(const InternalAPMConfig&) = default;
 InternalAPMConfig::InternalAPMConfig(InternalAPMConfig&&) = default;
 
-AecDump::AecDump(int64_t max_log_size_bytes, rtc::TaskQueue* worker_queue) {}
+AecDump::AecDump(int64_t max_log_size_bytes, rtc::TaskQueue* worker_queue)
+    : debug_file_(FileWrapper::Create()),
+      num_bytes_left_for_log_(max_log_size_bytes),
+      worker_queue_(worker_queue) {}
 
 AecDump::AecDump(rtc::PlatformFile file,
                  int64_t max_log_size_bytes,
-                 rtc::TaskQueue* worker_queue) {}
+                 rtc::TaskQueue* worker_queue)
+    : AecDump(max_log_size_bytes, worker_queue) {
+  FILE* handle = rtc::FdopenPlatformFileForWriting(file);
+  RTC_DCHECK(handle);
+  debug_file_->OpenFromFileHandle(handle);
+}
 
 AecDump::AecDump(std::string file_name,
                  int64_t max_log_size_bytes,
-                 rtc::TaskQueue* worker_queue) {}
+                 rtc::TaskQueue* worker_queue)
+    : AecDump(max_log_size_bytes, worker_queue) {
+  RTC_DCHECK(debug_file_);
+  debug_file_->OpenFile(file_name.c_str(), false);
+}
 
 AecDump::AecDump(FILE* handle,
                  int64_t max_log_size_bytes,
-                 rtc::TaskQueue* worker_queue) {}
+                 rtc::TaskQueue* worker_queue)
+    : AecDump(max_log_size_bytes, worker_queue) {
+  RTC_DCHECK(debug_file_);
+  debug_file_->OpenFromFileHandle(handle);
+}
 
-AecDump::~AecDump() {}
-
-AecDump::CaptureStreamInfo::CaptureStreamInfo() = default;
-AecDump::CaptureStreamInfo::~CaptureStreamInfo() = default;
-
-void AecDump::CaptureStreamInfo::AddInput(FloatAudioFrame src) {}
-void AecDump::CaptureStreamInfo::AddOutput(FloatAudioFrame src) {}
-
-void AecDump::CaptureStreamInfo::AddInput(const AudioFrame& frame) {}
-void AecDump::CaptureStreamInfo::AddOutput(const AudioFrame& frame) {}
-
-void AecDump::CaptureStreamInfo::set_delay(int delay) {}
-void AecDump::CaptureStreamInfo::set_drift(int drift) {}
-void AecDump::CaptureStreamInfo::set_level(int level) {}
-void AecDump::CaptureStreamInfo::set_keypress(bool keypress) {}
-
-std::unique_ptr<audioproc::Event> GetEventMsg();
+AecDump::~AecDump() {
+  // Block until all tasks have finished running.
+  rtc::Event thread_sync_event(false /* manual_reset */, false);
+  worker_queue_->PostTask([&thread_sync_event] { thread_sync_event.Set(); });
+  thread_sync_event.Wait(rtc::Event::kForever);

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

What does this do? Can this wait forever in the destructor?

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

It pauses execution until 'thread_sync_event.Set()' in the lambda function on
line 67 has been run. This is to ensure the task queue has pointers to an
AecDump object from inside pending tasks when the d-tor has finished.

It shouldn't wait forever.

The same pattern is used in AudioSendStream, VideoSendStream and rtc event
logging.

I've added a comment.

+}
 
 void AecDump::WriteInitMessage(const InternalAPMStreamsConfig& streams_config) {
+  auto event = std::unique_ptr<audioproc::Event>(new audioproc::Event());
+  event->set_type(audioproc::Event::INIT);
+  audioproc::Init* msg = event->mutable_init();
+
+  msg->set_sample_rate(streams_config.input_sample_rate);
+  msg->set_output_sample_rate(streams_config.output_sample_rate);
+  msg->set_reverse_sample_rate(streams_config.render_input_sample_rate);
+  msg->set_reverse_output_sample_rate(streams_config.render_output_sample_rate);
+
+  msg->set_num_input_channels(
+      static_cast<int32_t>(streams_config.input_num_channels));
+  msg->set_num_output_channels(
+      static_cast<int32_t>(streams_config.output_num_channels));
+  msg->set_num_reverse_channels(
+      static_cast<int32_t>(streams_config.render_input_num_channels));
+  msg->set_num_reverse_output_channels(
+      streams_config.render_output_num_channels);
+
+  PostTask(std::move(event));
 }
 
-void AecDump::WriteRenderStreamMessage(const AudioFrame& frame) {}
+void AecDump::WriteRenderStreamMessage(const AudioFrame& frame) {
+  auto event = std::unique_ptr<audioproc::Event>(new audioproc::Event());
 
-void AecDump::WriteRenderStreamMessage(FloatAudioFrame src) {}
+  event->set_type(audioproc::Event::REVERSE_STREAM);
+  audioproc::ReverseStream* msg = event->mutable_reverse_stream();
+  const size_t data_size =
+      sizeof(int16_t) * frame.samples_per_channel_ * frame.num_channels_;
+  msg->set_data(frame.data_, data_size);
+
+  PostTask(std::move(event));
+}
+
+void AecDump::WriteRenderStreamMessage(FloatAudioFrame src) {

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

Instead of passing a FloatAudioFrame as
aec_dump_->WriteRenderStreamMessage(FloatAudioFrame(src, num_channels,
channel_size));
have you considered changing the call to
aec_dump_->WriteRenderStreamMessage(src, num_channels, channel_size);

That would eliminate the need for FloatAudioFrame.
The for loop would then instead be
for (size_t i = 0; i < num_channels; ++i) {
  const auto& channel_view = src.channel(i);
  msg->add_channel(&src[i], sizeof(float) * channel_size);
}

WDYT?

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

I have a mild preference for leaving FloatAudioFrame. If we were to change the
interface to (float** data, num_channels, channel_size), there would be three
arguments for what is conceptually the same thing.

FloadAudioFrame helps to use the interface correctly by showing how the data
will be used through the public method channel(idx). It also helps in the
implementation of an AecDump subclass by showing how the data is supposed to be
used.

+  auto event = std::unique_ptr<audioproc::Event>(new audioproc::Event());
+  event->set_type(audioproc::Event::REVERSE_STREAM);
+
+  audioproc::ReverseStream* msg = event->mutable_reverse_stream();
+
+  for (size_t i = 0; i < src.num_channels(); ++i) {
+    const auto& channel_view = src.channel(i);
+    msg->add_channel(channel_view.begin(), sizeof(float) * channel_view.size());
+  }
+
+  PostTask(std::move(event));
+}
 
 void AecDump::WriteCaptureStreamMessage(
-    CaptureStreamInfo* capture_stream_info) {}
+    CaptureStreamInfo* capture_stream_info) {
+  auto event_ptr = capture_stream_info->GetEventMsg();
+  if (event_ptr) {
+    PostTask(std::move(event_ptr));
+  }
+}
 
-void AecDump::WriteConfig(const InternalAPMConfig& config, bool forced) {}
+void CopyFromConfigToEvent(const webrtc::InternalAPMConfig& config,
+                           webrtc::audioproc::Config* pb_cfg) {
+  pb_cfg->set_aec_enabled(config.aec_enabled);
+  pb_cfg->set_aec_delay_agnostic_enabled(config.aec_delay_agnostic_enabled);
+  pb_cfg->set_aec_drift_compensation_enabled(
+      config.aec_drift_compensation_enabled);
+  pb_cfg->set_aec_extended_filter_enabled(config.aec_extended_filter_enabled);
+  pb_cfg->set_aec_suppression_level(config.aec_suppression_level);
 
-void AecDump::PostTask(std::unique_ptr<audioproc::Event> event) {}
+  pb_cfg->set_aecm_enabled(config.aecm_enabled);
+  pb_cfg->set_aecm_comfort_noise_enabled(config.aecm_comfort_noise_enabled);
+  pb_cfg->set_aecm_routing_mode(config.aecm_routing_mode);
+
+  pb_cfg->set_agc_enabled(config.agc_enabled);
+  pb_cfg->set_agc_mode(config.agc_mode);
+  pb_cfg->set_agc_limiter_enabled(config.agc_limiter_enabled);
+  pb_cfg->set_noise_robust_agc_enabled(config.noise_robust_agc_enabled);
+
+  pb_cfg->set_hpf_enabled(config.hpf_enabled);
+
+  pb_cfg->set_ns_enabled(config.ns_enabled);
+  pb_cfg->set_ns_level(config.ns_level);
+
+  pb_cfg->set_transient_suppression_enabled(
+      config.transient_suppression_enabled);
+  pb_cfg->set_intelligibility_enhancer_enabled(
+      config.intelligibility_enhancer_enabled);
+
+  pb_cfg->set_experiments_description(config.experiments_description);
+}
+
+void AecDump::WriteConfig(const InternalAPMConfig& config, bool forced) {
+  auto event = std::unique_ptr<audioproc::Event>(new audioproc::Event());

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

I'd prefer to make this part of the WriteToFileTask. As it is now, it is
injected into WriteToFileTask but due to that construction it needs to be
created separately.

Would it be possible to do this as the following instead.

std::unique_ptr<rtc::QueuedTask> task(new WriteToFileTask(debug_file_.get(),
&num_bytes_left_for_log_)));
audioproc::Event* = task.GetEvent();
....
[update the fields of Event]
....
worker_queue_->PostTask(std::move(task));

That approach would eliminate the need for a separate newing up  of Event().
Furthermore, the postTask method would not be needed.

WDYT?

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

That makes everything a little simpler. I like it!

+  event->set_type(audioproc::Event::CONFIG);
+  CopyFromConfigToEvent(config, event->mutable_config());
+
+  ProtoString serialized_config = event->mutable_config()->SerializeAsString();
+  {
+    rtc::CritScope cs(&config_string_lock_);
+    if (!forced && serialized_config == last_serialized_capture_config_) {
+      return;
+    }
+    last_serialized_capture_config_ = serialized_config;
+  }
+
+  PostTask(std::move(event));
+}
+
+void AecDump::PostTask(std::unique_ptr<audioproc::Event> event) {
+  RTC_DCHECK(event);
+  worker_queue_->PostTask(std::unique_ptr<rtc::QueuedTask>(new WriteToFileTask(
+      debug_file_.get(), std::move(event), &num_bytes_left_for_log_)));
+}
 }  // namespace webrtc