AecDump implementation.

webrtc/modules/audio_processing/aec_dump/aec_dump_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 <utility>
+
+#include "webrtc/modules/audio_processing/aec_dump/aec_dump_impl.h"
+
+#include "webrtc/base/checks.h"
+#include "webrtc/base/event.h"
+#include "webrtc/base/ptr_util.h"
+#include "webrtc/modules/audio_processing/aec_dump/aec_dump_factory.h"
+
+namespace webrtc {
+
+AecDumpImpl::AecDumpImpl(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),
+      capture_stream_info_(CreateWriteToFileTask()) {}
+
+AecDumpImpl::AecDumpImpl(rtc::PlatformFile file,
+                         int64_t max_log_size_bytes,
+                         rtc::TaskQueue* worker_queue)
+    : AecDumpImpl(max_log_size_bytes, worker_queue) {
+  FILE* handle = rtc::FdopenPlatformFileForWriting(file);
+  RTC_DCHECK(handle);
+  debug_file_->OpenFromFileHandle(handle);
+}
+
+AecDumpImpl::AecDumpImpl(std::string file_name,
+                         int64_t max_log_size_bytes,
+                         rtc::TaskQueue* worker_queue)
+    : AecDumpImpl(max_log_size_bytes, worker_queue) {
+  RTC_DCHECK(debug_file_);
+  debug_file_->OpenFile(file_name.c_str(), false);
+}
+
+AecDumpImpl::AecDumpImpl(FILE* handle,
+                         int64_t max_log_size_bytes,
+                         rtc::TaskQueue* worker_queue)
+    : AecDumpImpl(max_log_size_bytes, worker_queue) {
+  RTC_DCHECK(debug_file_);
+  debug_file_->OpenFromFileHandle(handle);
+}
+
+AecDumpImpl::~AecDumpImpl() {
+  // 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(); });
+  // Wait until the event has been signaled with .Set(). By then all
+  // pending tasks will have finished.
+  thread_sync_event.Wait(rtc::Event::kForever);
+}
+
+void AecDumpImpl::AddCaptureStreamInput(const FloatAudioFrame& src) {

[peah-webrtc, 2017/05/16 06:30:37]

You could probably put these (63-79) in the header file if you want.

[aleloi, 2017/05/16 20:10:16]

The chromium-style plugin disagrees: virtual methods with non-empty bodies
shouldn't be declared inline.

+  capture_stream_info_.AddInput(src);
+}
+
+void AecDumpImpl::AddCaptureStreamOutput(const FloatAudioFrame& src) {
+  capture_stream_info_.AddOutput(src);
+}
+
+void AecDumpImpl::AddCaptureStreamInput(const AudioFrame& frame) {
+  capture_stream_info_.AddInput(frame);
+}
+
+void AecDumpImpl::AddCaptureStreamOutput(const AudioFrame& frame) {
+  capture_stream_info_.AddOutput(frame);
+}
+
+void AecDumpImpl::AddAudioProcessingState(const AudioProcessingState& state) {
+  capture_stream_info_.AddAudioProcessingState(state);
+}
+
+void AecDumpImpl::WriteInitMessage(
+    const InternalAPMStreamsConfig& streams_config) {
+  auto task = CreateWriteToFileTask();
+  auto* event = task->GetEvent();
+  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);
+
+  worker_queue_->PostTask(std::unique_ptr<rtc::QueuedTask>(std::move(task)));
+}
+
+void AecDumpImpl::WriteRenderStreamMessage(const AudioFrame& frame) {
+  auto task = CreateWriteToFileTask();
+  auto* event = task->GetEvent();
+
+  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);
+
+  worker_queue_->PostTask(std::unique_ptr<rtc::QueuedTask>(std::move(task)));
+}
+
+void AecDumpImpl::WriteRenderStreamMessage(const FloatAudioFrame& src) {
+  auto task = CreateWriteToFileTask();
+  auto* event = task->GetEvent();
+
+  // auto event = std::unique_ptr<audioproc::Event>(new audioproc::Event());

[peah-webrtc, 2017/05/16 06:30:37]

Is line 124 intended to be commented out? I guess it should be deleted, right?

[aleloi, 2017/05/16 20:10:16]

Done.

+  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());
+  }
+
+  worker_queue_->PostTask(std::unique_ptr<rtc::QueuedTask>(std::move(task)));
+}
+
+void AecDumpImpl::WriteCaptureStreamMessage() {
+  auto task = capture_stream_info_.GetTask();
+  RTC_DCHECK(task);
+  worker_queue_->PostTask(std::unique_ptr<rtc::QueuedTask>(std::move(task)));
+  capture_stream_info_.SetTask(CreateWriteToFileTask());
+}
+
+void CopyFromConfigToEvent(const webrtc::InternalAPMConfig& config,

[peah-webrtc, 2017/05/16 06:30:37]

Can this method be put in the anonymous namespace?

[aleloi, 2017/05/16 20:10:16]

Done.

+                           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);
+
+  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 AecDumpImpl::WriteConfig(const InternalAPMConfig& config, bool forced) {
+  auto task = CreateWriteToFileTask();
+  auto* event = task->GetEvent();
+  event->set_type(audioproc::Event::CONFIG);
+  CopyFromConfigToEvent(config, event->mutable_config());
+
+  ProtoString serialized_config = event->mutable_config()->SerializeAsString();
+  {
+    rtc::CritScope cs(&config_string_lock_);

[peah-webrtc, 2017/05/16 06:30:38]

I don't see that locking is needed for the config_string. It is only accessed in
WriteConfig, right? And WriteConfig is in the usage of AecDump only called one
at a time. 

This is a tricky part when having an interface that can be accessed
concurrently, but in a way such that the state used by the different thread
accesses is separated so that no locks are needed.

In cases like this I like the approach of separating the members of the class
into nested classes, where each contain only the data that it can operate on.
That makes it easier to see that the code is threadsafe by examining the code.
But in this case the separation is so trivial so it probably does not make
sense, at least for the render part.

Another thing thing to do would be to add thread sanitizers/checkers. Are there
any such that you can use instead?

[aleloi, 2017/05/16 20:10:16]

There was something detected by the tsan bots that led me to add the locks. I
can't think of why they would be needed now; maybe the threading model has
changed. I've replaced the lock with a race checker. I'll run the new patch on
the bots.

+    if (!forced && serialized_config == last_serialized_capture_config_) {
+      return;
+    }
+    last_serialized_capture_config_ = serialized_config;
+  }
+
+  worker_queue_->PostTask(std::unique_ptr<rtc::QueuedTask>(std::move(task)));
+}
+
+std::unique_ptr<WriteToFileTask> AecDumpImpl::CreateWriteToFileTask() {
+  return rtc::MakeUnique<WriteToFileTask>(debug_file_.get(),
+                                          &num_bytes_left_for_log_);
+}
+
+std::unique_ptr<AecDump> AecDumpFactory::Create(rtc::PlatformFile file,
+                                                int64_t max_log_size_bytes,
+                                                rtc::TaskQueue* worker_queue) {
+  return std::unique_ptr<AecDumpImpl>(
+      new AecDumpImpl(file, max_log_size_bytes, worker_queue));
+}
+
+std::unique_ptr<AecDump> AecDumpFactory::Create(std::string file_name,
+                                                int64_t max_log_size_bytes,
+                                                rtc::TaskQueue* worker_queue) {
+  return std::unique_ptr<AecDumpImpl>(
+      new AecDumpImpl(file_name, max_log_size_bytes, worker_queue));
+}
+
+std::unique_ptr<AecDump> AecDumpFactory::Create(FILE* handle,
+                                                int64_t max_log_size_bytes,
+                                                rtc::TaskQueue* worker_queue) {
+  return std::unique_ptr<AecDumpImpl>(
+      new AecDumpImpl(handle, max_log_size_bytes, worker_queue));
+}
+}  // namespace webrtc