From 1aec594018f29bda5a8fbe69d5d8cf6e635b7490 Mon Sep 17 00:00:00 2001 From: Alex Loiko Date: Tue, 15 May 2018 13:13:22 +0200 Subject: [PATCH] Merge :audio_processing and :aec_dump_interface. MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Merges the two targets in modules/audio_processing and removes some redundant code. This enables not writing a bunch of redundant code in https://webrtc-review.googlesource.com/c/src/+/70502 ':audio_processing' did depend on ':aec_dump_interface'. 'modules/audio_processing/aec_dump' did depend on 'aec_dump_interface' but not ':audio_processing'. Having the AecDump implementation not depending on 'audio_processing' allows to have faster compilation time and reduces the dependencies. However, maintaining such a decoupling makes APM and AecDump client code more complex. NOTRY=true # want this in and 'ios_api_framework' seems stuck. Bug: webrtc:7404 Change-Id: I75a5f234591014ac42d52bc1a36526072f5be89c Reviewed-on: https://webrtc-review.googlesource.com/76603 Commit-Queue: Alex Loiko Reviewed-by: Alessio Bazzica Reviewed-by: Per Åhgren Cr-Commit-Position: refs/heads/master@{#23244} --- modules/audio_processing/BUILD.gn | 18 ++------------- modules/audio_processing/aec_dump/BUILD.gn | 12 +++++----- .../aec_dump/aec_dump_impl.cc | 21 +++++++++--------- .../audio_processing/aec_dump/aec_dump_impl.h | 2 +- .../aec_dump/aec_dump_unittest.cc | 4 ++-- .../audio_processing/aec_dump/mock_aec_dump.h | 3 +-- .../audio_processing/audio_processing_impl.cc | 22 ++----------------- modules/audio_processing/include/aec_dump.h | 16 ++------------ 8 files changed, 27 insertions(+), 71 deletions(-) diff --git a/modules/audio_processing/BUILD.gn b/modules/audio_processing/BUILD.gn index 1d7d723b7a..e6c5933f0d 100644 --- a/modules/audio_processing/BUILD.gn +++ b/modules/audio_processing/BUILD.gn @@ -75,6 +75,8 @@ rtc_static_library("audio_processing") { "gain_control_impl.h", "gain_controller2.cc", "gain_controller2.h", + "include/aec_dump.cc", + "include/aec_dump.h", "include/audio_processing.cc", "include/audio_processing.h", "include/config.cc", @@ -116,7 +118,6 @@ rtc_static_library("audio_processing") { defines = [] deps = [ ":aec_core", - ":aec_dump_interface", ":apm_logging", ":audio_frame_view", ":audio_generator_interface", @@ -199,20 +200,6 @@ rtc_source_set("audio_frame_view") { ] } -rtc_source_set("aec_dump_interface") { - sources = [ - "include/aec_dump.cc", - "include/aec_dump.h", - ] - - deps = [ - ":audio_frame_view", - "../../api:array_view", - "../../api/audio:audio_frame_api", - "../../rtc_base:rtc_base_approved", - ] -} - rtc_source_set("audio_generator_interface") { visibility = [ "*" ] sources = [ @@ -445,7 +432,6 @@ if (rtc_include_tests) { "include/mock_audio_processing.h", ] deps = [ - ":aec_dump_interface", ":audio_processing", ":audio_processing_statistics", "../../test:test_support", diff --git a/modules/audio_processing/aec_dump/BUILD.gn b/modules/audio_processing/aec_dump/BUILD.gn index 152c290cbe..52a8f0999c 100644 --- a/modules/audio_processing/aec_dump/BUILD.gn +++ b/modules/audio_processing/aec_dump/BUILD.gn @@ -15,7 +15,7 @@ rtc_source_set("aec_dump") { ] deps = [ - "..:aec_dump_interface", + "../", "../../../rtc_base:rtc_base_approved", ] } @@ -28,7 +28,7 @@ rtc_source_set("mock_aec_dump") { ] deps = [ - "..:aec_dump_interface", + "../", "../../../test:test_support", ] } @@ -42,7 +42,7 @@ rtc_source_set("mock_aec_dump_unittests") { deps = [ ":mock_aec_dump", - "..:audio_processing", + "../", "../../../rtc_base:rtc_base_approved", "//testing/gtest", ] @@ -61,7 +61,7 @@ if (rtc_enable_protobuf) { deps = [ ":aec_dump", - "..:aec_dump_interface", + "../", "../../../api/audio:audio_frame_api", "../../../rtc_base:checks", "../../../rtc_base:protobuf_utils", @@ -80,8 +80,8 @@ if (rtc_enable_protobuf) { deps = [ ":aec_dump", ":aec_dump_impl", - "..:aec_dump_interface", "..:audioproc_debug_proto", + "../", "../../../rtc_base:rtc_task_queue", "../../../test:fileutils", "../../../test:test_support", @@ -101,6 +101,6 @@ rtc_source_set("null_aec_dump_factory") { deps = [ ":aec_dump", - "..:aec_dump_interface", + "../", ] } diff --git a/modules/audio_processing/aec_dump/aec_dump_impl.cc b/modules/audio_processing/aec_dump/aec_dump_impl.cc index ec35f0a6b4..9158f9b195 100644 --- a/modules/audio_processing/aec_dump/aec_dump_impl.cc +++ b/modules/audio_processing/aec_dump/aec_dump_impl.cc @@ -74,26 +74,27 @@ AecDumpImpl::~AecDumpImpl() { thread_sync_event.Wait(rtc::Event::kForever); } -void AecDumpImpl::WriteInitMessage( - const InternalAPMStreamsConfig& streams_config) { +void AecDumpImpl::WriteInitMessage(const ProcessingConfig& api_format) { 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_sample_rate(api_format.input_stream().sample_rate_hz()); + msg->set_output_sample_rate(api_format.output_stream().sample_rate_hz()); + msg->set_reverse_sample_rate( + api_format.reverse_input_stream().sample_rate_hz()); + msg->set_reverse_output_sample_rate( + api_format.reverse_output_stream().sample_rate_hz()); msg->set_num_input_channels( - static_cast(streams_config.input_num_channels)); + static_cast(api_format.input_stream().num_channels())); msg->set_num_output_channels( - static_cast(streams_config.output_num_channels)); + static_cast(api_format.output_stream().num_channels())); msg->set_num_reverse_channels( - static_cast(streams_config.render_input_num_channels)); + static_cast(api_format.reverse_input_stream().num_channels())); msg->set_num_reverse_output_channels( - streams_config.render_output_num_channels); + api_format.reverse_output_stream().num_channels()); worker_queue_->PostTask(std::unique_ptr(std::move(task))); } diff --git a/modules/audio_processing/aec_dump/aec_dump_impl.h b/modules/audio_processing/aec_dump/aec_dump_impl.h index 0d88a7ed96..1ad4fc88f5 100644 --- a/modules/audio_processing/aec_dump/aec_dump_impl.h +++ b/modules/audio_processing/aec_dump/aec_dump_impl.h @@ -52,7 +52,7 @@ class AecDumpImpl : public AecDump { ~AecDumpImpl() override; - void WriteInitMessage(const InternalAPMStreamsConfig& api_format) override; + void WriteInitMessage(const ProcessingConfig& api_format) override; void AddCaptureStreamInput(const AudioFrameView& src) override; void AddCaptureStreamOutput(const AudioFrameView& src) override; diff --git a/modules/audio_processing/aec_dump/aec_dump_unittest.cc b/modules/audio_processing/aec_dump/aec_dump_unittest.cc index 98640b9824..dea149d540 100644 --- a/modules/audio_processing/aec_dump/aec_dump_unittest.cc +++ b/modules/audio_processing/aec_dump/aec_dump_unittest.cc @@ -39,8 +39,8 @@ TEST(AecDumper, APICallsDoNotCrash) { webrtc::InternalAPMConfig apm_config; aec_dump->WriteConfig(apm_config); - webrtc::InternalAPMStreamsConfig streams_config; - aec_dump->WriteInitMessage(streams_config); + webrtc::ProcessingConfig api_format; + aec_dump->WriteInitMessage(api_format); } // Remove file after the AecDump d-tor has finished. ASSERT_EQ(0, remove(filename.c_str())); diff --git a/modules/audio_processing/aec_dump/mock_aec_dump.h b/modules/audio_processing/aec_dump/mock_aec_dump.h index c2088c0f4c..95f912fc64 100644 --- a/modules/audio_processing/aec_dump/mock_aec_dump.h +++ b/modules/audio_processing/aec_dump/mock_aec_dump.h @@ -25,8 +25,7 @@ class MockAecDump : public AecDump { MockAecDump(); virtual ~MockAecDump(); - MOCK_METHOD1(WriteInitMessage, - void(const InternalAPMStreamsConfig& streams_config)); + MOCK_METHOD1(WriteInitMessage, void(const ProcessingConfig& api_format)); MOCK_METHOD1(AddCaptureStreamInput, void(const AudioFrameView& src)); diff --git a/modules/audio_processing/audio_processing_impl.cc b/modules/audio_processing/audio_processing_impl.cc index e3b385729c..6d464901dc 100644 --- a/modules/audio_processing/audio_processing_impl.cc +++ b/modules/audio_processing/audio_processing_impl.cc @@ -148,24 +148,6 @@ class HighPassFilterImpl : public HighPassFilter { AudioProcessingImpl* apm_; 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. @@ -608,7 +590,7 @@ int AudioProcessingImpl::InitializeLocked() { InitializePreProcessor(); if (aec_dump_) { - aec_dump_->WriteInitMessage(ToStreamsConfig(formats_.api_format)); + aec_dump_->WriteInitMessage(formats_.api_format); } return kNoError; } @@ -1638,7 +1620,7 @@ void AudioProcessingImpl::AttachAecDump(std::unique_ptr aec_dump) { // 'aec_dump' parameter, which is after locks are released. aec_dump_.swap(aec_dump); WriteAecDumpConfigMessage(true); - aec_dump_->WriteInitMessage(ToStreamsConfig(formats_.api_format)); + aec_dump_->WriteInitMessage(formats_.api_format); } void AudioProcessingImpl::DetachAecDump() { diff --git a/modules/audio_processing/include/aec_dump.h b/modules/audio_processing/include/aec_dump.h index e824892c32..25c08b8266 100644 --- a/modules/audio_processing/include/aec_dump.h +++ b/modules/audio_processing/include/aec_dump.h @@ -18,6 +18,7 @@ #include "api/array_view.h" #include "api/audio/audio_frame.h" #include "modules/audio_processing/include/audio_frame_view.h" +#include "modules/audio_processing/include/audio_processing.h" namespace webrtc { @@ -55,18 +56,6 @@ struct InternalAPMConfig { std::string experiments_description = ""; }; -struct InternalAPMStreamsConfig { - int input_sample_rate = 0; - int output_sample_rate = 0; - int render_input_sample_rate = 0; - int render_output_sample_rate = 0; - - size_t input_num_channels = 0; - size_t output_num_channels = 0; - size_t render_input_num_channels = 0; - size_t render_output_num_channels = 0; -}; - // An interface for recording configuration and input/output streams // of the Audio Processing Module. The recordings are called // 'aec-dumps' and are stored in a protobuf format defined in @@ -87,8 +76,7 @@ class AecDump { virtual ~AecDump() = default; // Logs Event::Type INIT message. - virtual void WriteInitMessage( - const InternalAPMStreamsConfig& streams_config) = 0; + virtual void WriteInitMessage(const ProcessingConfig& api_format) = 0; // Logs Event::Type STREAM message. To log an input/output pair, // call the AddCapture* and AddAudioProcessingState methods followed