From 0ade98316c212a84fc94679fda32c324fe23cfef Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Per=20=C3=85hgren?= Date: Tue, 1 Sep 2020 23:57:20 +0200 Subject: [PATCH] Cleanup of the APM creation MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This CL removes the possibility that APM cannot be created, i.e., that the create method can return nullptr. That was already the case implicitly but this CL makes that behavior explicit. Bug: webrtc:5298 Change-Id: I2706ea538c9d1b4bcd65faecab637640a209a4dc Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/183101 Reviewed-by: Sam Zackrisson Commit-Queue: Per Åhgren Cr-Commit-Position: refs/heads/master@{#32029} --- .../audio_processing/audio_processing_builder_impl.cc | 8 +------- modules/audio_processing/audio_processing_impl.cc | 11 +++++++---- modules/audio_processing/audio_processing_impl.h | 2 +- .../audio_processing_impl_unittest.cc | 6 +++--- modules/audio_processing/include/audio_processing.h | 1 + .../test/audio_processing_builder_for_testing.cc | 5 +---- 6 files changed, 14 insertions(+), 19 deletions(-) diff --git a/modules/audio_processing/audio_processing_builder_impl.cc b/modules/audio_processing/audio_processing_builder_impl.cc index e89bbecc68..f55c9158fb 100644 --- a/modules/audio_processing/audio_processing_builder_impl.cc +++ b/modules/audio_processing/audio_processing_builder_impl.cc @@ -35,16 +35,10 @@ AudioProcessing* AudioProcessingBuilder::Create(const webrtc::Config& config) { #else // Standard implementation. - AudioProcessingImpl* apm = new rtc::RefCountedObject( + return new rtc::RefCountedObject( config, std::move(capture_post_processing_), std::move(render_pre_processing_), std::move(echo_control_factory_), std::move(echo_detector_), std::move(capture_analyzer_)); - if (apm->Initialize() != AudioProcessing::kNoError) { - delete apm; - apm = nullptr; - } - return apm; - #endif } diff --git a/modules/audio_processing/audio_processing_impl.cc b/modules/audio_processing/audio_processing_impl.cc index 485022accf..82cc6f1148 100644 --- a/modules/audio_processing/audio_processing_impl.cc +++ b/modules/audio_processing/audio_processing_impl.cc @@ -307,6 +307,8 @@ AudioProcessingImpl::AudioProcessingImpl( config_.gain_controller1.analog_gain_controller.enable_digital_adaptive = !config.Get().digital_adaptive_disabled; #endif + + Initialize(); } AudioProcessingImpl::~AudioProcessingImpl() = default; @@ -315,7 +317,8 @@ int AudioProcessingImpl::Initialize() { // Run in a single-threaded manner during initialization. MutexLock lock_render(&mutex_render_); MutexLock lock_capture(&mutex_capture_); - return InitializeLocked(); + InitializeLocked(); + return kNoError; } int AudioProcessingImpl::Initialize(int capture_input_sample_rate_hz, @@ -356,7 +359,7 @@ int AudioProcessingImpl::MaybeInitializeRender( return InitializeLocked(processing_config); } -int AudioProcessingImpl::InitializeLocked() { +void AudioProcessingImpl::InitializeLocked() { UpdateActiveSubmoduleStates(); const int render_audiobuffer_sample_rate_hz = @@ -425,7 +428,6 @@ int AudioProcessingImpl::InitializeLocked() { if (aec_dump_) { aec_dump_->WriteInitMessage(formats_.api_format, rtc::TimeUTCMillis()); } - return kNoError; } int AudioProcessingImpl::InitializeLocked(const ProcessingConfig& config) { @@ -519,7 +521,8 @@ int AudioProcessingImpl::InitializeLocked(const ProcessingConfig& config) { capture_nonlocked_.capture_processing_format.sample_rate_hz(); } - return InitializeLocked(); + InitializeLocked(); + return kNoError; } void AudioProcessingImpl::ApplyConfig(const AudioProcessing::Config& config) { diff --git a/modules/audio_processing/audio_processing_impl.h b/modules/audio_processing/audio_processing_impl.h index dfd5f63c51..ac411b48cf 100644 --- a/modules/audio_processing/audio_processing_impl.h +++ b/modules/audio_processing/audio_processing_impl.h @@ -140,7 +140,7 @@ class AudioProcessingImpl : public AudioProcessing { protected: // Overridden in a mock. - virtual int InitializeLocked() + virtual void InitializeLocked() RTC_EXCLUSIVE_LOCKS_REQUIRED(mutex_render_, mutex_capture_); private: diff --git a/modules/audio_processing/audio_processing_impl_unittest.cc b/modules/audio_processing/audio_processing_impl_unittest.cc index 8f28941cdf..e289c316bc 100644 --- a/modules/audio_processing/audio_processing_impl_unittest.cc +++ b/modules/audio_processing/audio_processing_impl_unittest.cc @@ -37,9 +37,9 @@ class MockInitialize : public AudioProcessingImpl { explicit MockInitialize(const webrtc::Config& config) : AudioProcessingImpl(config) {} - MOCK_METHOD(int, InitializeLocked, (), (override)); - int RealInitializeLocked() RTC_NO_THREAD_SAFETY_ANALYSIS { - return AudioProcessingImpl::InitializeLocked(); + MOCK_METHOD(void, InitializeLocked, (), (override)); + void RealInitializeLocked() RTC_NO_THREAD_SAFETY_ANALYSIS { + AudioProcessingImpl::InitializeLocked(); } MOCK_METHOD(void, AddRef, (), (const, override)); diff --git a/modules/audio_processing/include/audio_processing.h b/modules/audio_processing/include/audio_processing.h index 9e2d6dcb4a..fe45e644d1 100644 --- a/modules/audio_processing/include/audio_processing.h +++ b/modules/audio_processing/include/audio_processing.h @@ -488,6 +488,7 @@ class RTC_EXPORT AudioProcessing : public rtc::RefCountInterface { // rate and number of channels) have changed. Passing updated parameters // directly to |ProcessStream()| and |ProcessReverseStream()| is permissible. // If the parameters are known at init-time though, they may be provided. + // TODO(webrtc:5298): Change to return void. virtual int Initialize() = 0; // The int16 interfaces require: diff --git a/modules/audio_processing/test/audio_processing_builder_for_testing.cc b/modules/audio_processing/test/audio_processing_builder_for_testing.cc index 26ed679d7f..faab18f7fd 100644 --- a/modules/audio_processing/test/audio_processing_builder_for_testing.cc +++ b/modules/audio_processing/test/audio_processing_builder_for_testing.cc @@ -30,13 +30,10 @@ AudioProcessing* AudioProcessingBuilderForTesting::Create() { AudioProcessing* AudioProcessingBuilderForTesting::Create( const webrtc::Config& config) { - AudioProcessingImpl* apm = new rtc::RefCountedObject( + return new rtc::RefCountedObject( config, std::move(capture_post_processing_), std::move(render_pre_processing_), std::move(echo_control_factory_), std::move(echo_detector_), std::move(capture_analyzer_)); - int error = apm->Initialize(); - RTC_CHECK_EQ(error, AudioProcessing::kNoError); - return apm; } #else