AecDump interface

webrtc/modules/audio_processing/audio_processing_impl.cc

Index: webrtc/modules/audio_processing/audio_processing_impl.cc
diff --git a/webrtc/modules/audio_processing/audio_processing_impl.cc b/webrtc/modules/audio_processing/audio_processing_impl.cc
index 816210f34f8738eb7a042adee69f87f5ed1a99d8..8b16a23ceffc8906539ede001dd8dc7884feeb6a 100644
--- a/webrtc/modules/audio_processing/audio_processing_impl.cc
+++ b/webrtc/modules/audio_processing/audio_processing_impl.cc
@@ -152,6 +152,24 @@ class HighPassFilterImpl : public HighPassFilter {
   RTC_DISALLOW_IMPLICIT_CONSTRUCTORS(HighPassFilterImpl);
 };
 
+webrtc::InternalAPMStreamsConfig ToStreamsConfig(
+    const ProcessingConfig& api_format) {
+  webrtc::InternalAPMStreamsConfig result;
+  result.input_sample_rate = api_format.input_stream().sample_rate_hz();
+  result.input_num_channels = api_format.input_stream().num_channels();
+  result.output_num_channels = api_format.output_stream().num_channels();
+  result.render_input_num_channels =
+      api_format.reverse_input_stream().num_channels();
+  result.render_input_sample_rate =
+      api_format.reverse_input_stream().sample_rate_hz();
+  result.output_sample_rate = api_format.output_stream().sample_rate_hz();
+  result.render_output_sample_rate =
+      api_format.reverse_output_stream().sample_rate_hz();
+  result.render_output_num_channels =
+      api_format.reverse_output_stream().num_channels();
+  return result;
+}
+
 }  // namespace
 
 // Throughout webrtc, it's assumed that success is represented by zero.
@@ -526,7 +544,9 @@ int AudioProcessingImpl::InitializeLocked() {
     }
   }
 #endif
-
+  if (aec_dump_) {
+    aec_dump_->WriteInitMessage(ToStreamsConfig(formats_.api_format));
+  }
   return kNoError;
 }
 
@@ -824,7 +844,28 @@ int AudioProcessingImpl::ProcessStream(const float* const* src,
   }
 #endif
 
+  std::unique_ptr<AecDump::CaptureStreamInfo> stream_info;
+  if (aec_dump_) {

[peah-webrtc, 2017/04/19 12:30:29]

Why not merge the if-statements on 848 and 852?

[aleloi, 2017/04/20 15:26:23]

Oh, I thought I did that... Done! Although a bit on the verbose side.

+    stream_info = aec_dump_->GetCaptureStreamInfo();
+  }
+
+  if (aec_dump_) {
+    const size_t channel_size =
+        sizeof(float) * formats_.api_format.input_stream().num_frames();
+    std::vector<rtc::ArrayView<const float>> src_view;
+    for (size_t i = 0; i < formats_.api_format.input_stream().num_channels();
+         ++i) {
+      src_view.emplace_back(src[i], channel_size);
+    }
+    stream_info->AddInput(src_view);
+    stream_info->set_delay(capture_nonlocked_.stream_delay_ms);
+    stream_info->set_drift(
+        public_submodules_->echo_cancellation->stream_drift_samples());
+    stream_info->set_level(gain_control()->stream_analog_level());
+    stream_info->set_keypress(capture_.key_pressed);
+  }
   capture_.capture_audio->CopyFrom(src, formats_.api_format.input_stream());
+

[peah-webrtc, 2017/04/19 12:30:29]

It would probably make sense to have the empty line before 867 instead of after,
as 867 is related to 869 and 870.

[aleloi, 2017/04/20 15:26:23]

Done.

   RETURN_ON_ERR(ProcessCaptureStreamLocked());
   capture_.capture_audio->CopyTo(formats_.api_format.output_stream(), dest);
 
@@ -841,6 +882,18 @@ int AudioProcessingImpl::ProcessStream(const float* const* src,
                                           &crit_debug_, &debug_dump_.capture));
   }
 #endif
+  if (aec_dump_) {
+    const size_t channel_size =
+        sizeof(float) * formats_.api_format.output_stream().num_frames();
+    std::vector<rtc::ArrayView<const float>> dest_view;
+    for (size_t i = 0; i < formats_.api_format.output_stream().num_channels();
+         ++i) {
+      dest_view.emplace_back(dest[i], channel_size);

[peah-webrtc, 2017/04/19 12:30:29]

This code construct using a vector causes heap allocation, and possible vector
reallocation. That happens for each frame.

I'd rather prefer using an array
std::array<rtc::ArrayView<const float>, kMaxNumChannels> dest_view;
for (...) {
dest_view[i] = rtc::ArrayView<const float>(dest[i], channel_size);
}

WDYT?

[aleloi, 2017/04/20 15:26:23]

Done.

+    }
+    RTC_DCHECK(stream_info);
+    stream_info->AddOutput(dest_view);
+    aec_dump_->WriteCaptureStreamMessage(std::move(stream_info));
+  }
 
   return kNoError;
 }
@@ -1078,6 +1131,18 @@ int AudioProcessingImpl::ProcessStream(AudioFrame* frame) {
     return kBadDataLengthError;
   }
 
+  std::unique_ptr<AecDump::CaptureStreamInfo> stream_info;
+  if (aec_dump_) {
+    stream_info = aec_dump_->GetCaptureStreamInfo();
+    RTC_DCHECK(stream_info);
+    stream_info->AddInput(*frame);
+    stream_info->set_delay(capture_nonlocked_.stream_delay_ms);
+    stream_info->set_drift(
+        public_submodules_->echo_cancellation->stream_drift_samples());
+    stream_info->set_level(gain_control()->stream_analog_level());
+    stream_info->set_keypress(capture_.key_pressed);
+  }
+
 #ifdef WEBRTC_AUDIOPROC_DEBUG_DUMP
   if (debug_dump_.debug_file->is_open()) {
     RETURN_ON_ERR(WriteConfigMessage(false));
@@ -1091,10 +1156,15 @@ int AudioProcessingImpl::ProcessStream(AudioFrame* frame) {
 #endif
 
   capture_.capture_audio->DeinterleaveFrom(frame);
+

[peah-webrtc, 2017/04/19 12:30:29]

Remove empty line.

[aleloi, 2017/04/20 15:26:23]

Done.

   RETURN_ON_ERR(ProcessCaptureStreamLocked());
   capture_.capture_audio->InterleaveTo(
       frame, submodule_states_.CaptureMultiBandProcessingActive());
 
+  if (aec_dump_) {
+    stream_info->AddOutput(*frame);

[peah-webrtc, 2017/04/19 12:30:29]

DCHECK on stream_info?

[aleloi, 2017/04/20 15:26:23]

It's done just after stream_info is assigned to on 1136.

+    aec_dump_->WriteCaptureStreamMessage(std::move(stream_info));
+  }
 #ifdef WEBRTC_AUDIOPROC_DEBUG_DUMP
   if (debug_dump_.debug_file->is_open()) {
     audioproc::Stream* msg = debug_dump_.capture.event_msg->mutable_stream();
@@ -1376,7 +1446,17 @@ int AudioProcessingImpl::AnalyzeReverseStreamLocked(
                                           &crit_debug_, &debug_dump_.render));
   }
 #endif
+  if (aec_dump_) {
+    std::vector<rtc::ArrayView<const float>> src_view;
+    const size_t channel_size =
+        sizeof(float) * formats_.api_format.reverse_input_stream().num_frames();
 
+    for (size_t i = 0;
+         i < formats_.api_format.reverse_input_stream().num_channels(); ++i) {

[peah-webrtc, 2017/04/19 12:30:29]

See above comment about heap allocation

[aleloi, 2017/04/20 15:26:23]

Done.

+      src_view.emplace_back(src[i], channel_size);
+    }
+    aec_dump_->WriteRenderStreamMessage(src_view);
+  }
   render_.render_audio->CopyFrom(src,
                                  formats_.api_format.reverse_input_stream());
   return ProcessRenderStreamLocked();
@@ -1429,6 +1509,10 @@ int AudioProcessingImpl::ProcessReverseStream(AudioFrame* frame) {
                                           &crit_debug_, &debug_dump_.render));
   }
 #endif
+  if (aec_dump_) {
+    aec_dump_->WriteRenderStreamMessage(*frame);
+  }
+
   render_.render_audio->DeinterleaveFrom(frame);
   RETURN_ON_ERR(ProcessRenderStreamLocked());
   render_.render_audio->InterleaveTo(
@@ -1512,6 +1596,21 @@ int AudioProcessingImpl::delay_offset_ms() const {
   return capture_.delay_offset_ms;
 }
 
+void AudioProcessingImpl::StartDebugRecording(
+    std::unique_ptr<AecDump> aec_dump) {
+  rtc::CritScope cs_render(&crit_render_);
+  rtc::CritScope cs_capture(&crit_capture_);
+  RTC_DCHECK(aec_dump);
+  aec_dump_ = std::move(aec_dump);
+
+#ifdef WEBRTC_AUDIOPROC_DEBUG_DUMP
+  const int error = WriteConfigMessage(true);

[peah-webrtc, 2017/04/19 12:30:29]

It would be nice to avoid having the #ifdef-endif around the new aec-dump
functionality.

Would it be possible to split WriteConfigMessage allready now so that the new
aecdump code can be written without  #ifdef-endif's ? 

[aleloi, 2017/04/20 15:26:24]

Yes, I've made an aec-dump-only vesrion of WriteConfigMessage.

+  RTC_DCHECK(error);
+#endif  // WEBRTC_AUDIOPROC_DEBUG_DUMP
+
+  aec_dump_->WriteInitMessage(ToStreamsConfig(formats_.api_format));
+}
+
 int AudioProcessingImpl::StartDebugRecording(
     const char filename[AudioProcessing::kMaxFilenameSize],
     int64_t max_log_size_bytes) {
@@ -1586,6 +1685,7 @@ int AudioProcessingImpl::StopDebugRecording() {
   // Run in a single-threaded manner.
   rtc::CritScope cs_render(&crit_render_);
   rtc::CritScope cs_capture(&crit_capture_);
+  aec_dump_ = nullptr;

[peah-webrtc, 2017/04/19 12:30:29]

aec_dump_.reset()?

[aleloi, 2017/04/20 15:26:24]

Done.

 
 #ifdef WEBRTC_AUDIOPROC_DEBUG_DUMP
   // We just return if recording hasn't started.
@@ -1965,6 +2065,48 @@ int AudioProcessingImpl::WriteConfigMessage(bool forced) {
   }
   config.set_experiments_description(experiments_description);
 
+  if (aec_dump_) {
+    InternalAPMConfig apm_config;
+
+    apm_config.aec_enabled =
+        public_submodules_->echo_cancellation->is_enabled();
+    apm_config.aec_delay_agnostic_enabled =
+        public_submodules_->echo_cancellation->is_delay_agnostic_enabled();
+    apm_config.aec_drift_compensation_enabled =
+        public_submodules_->echo_cancellation->is_drift_compensation_enabled();
+    apm_config.aec_extended_filter_enabled =
+        public_submodules_->echo_cancellation->is_extended_filter_enabled();
+    apm_config.aec_suppression_level = static_cast<int>(
+        public_submodules_->echo_cancellation->suppression_level());
+
+    apm_config.aecm_enabled =
+        public_submodules_->echo_control_mobile->is_enabled();
+    apm_config.aecm_comfort_noise_enabled =
+        public_submodules_->echo_control_mobile->is_comfort_noise_enabled();
+    apm_config.aecm_routing_mode = static_cast<int>(
+        public_submodules_->echo_control_mobile->routing_mode());
+
+    apm_config.agc_enabled = public_submodules_->gain_control->is_enabled();
+    apm_config.agc_mode =
+        static_cast<int>(public_submodules_->gain_control->mode());
+    apm_config.agc_limiter_enabled =
+        public_submodules_->gain_control->is_limiter_enabled();
+    apm_config.noise_robust_agc_enabled = constants_.use_experimental_agc;
+
+    apm_config.hpf_enabled = config_.high_pass_filter.enabled;
+
+    apm_config.ns_enabled = public_submodules_->noise_suppression->is_enabled();
+    apm_config.ns_level =
+        static_cast<int>(public_submodules_->noise_suppression->level());
+
+    apm_config.transient_suppression_enabled =
+        capture_.transient_suppressor_enabled;
+    apm_config.intelligibility_enhancer_enabled =
+        capture_nonlocked_.intelligibility_enabled;
+    apm_config.experiments_description = experiments_description;
+    aec_dump_->WriteConfig(apm_config, forced);
+  }
+
   ProtoString serialized_config = config.SerializeAsString();
   if (!forced &&
       debug_dump_.capture.last_serialized_config == serialized_config) {