diff --git a/modules/audio_processing/audio_processing_impl.cc b/modules/audio_processing/audio_processing_impl.cc index 9712d42d22..376464793c 100644 --- a/modules/audio_processing/audio_processing_impl.cc +++ b/modules/audio_processing/audio_processing_impl.cc @@ -227,7 +227,6 @@ bool AudioProcessingImpl::ApmSubmoduleStates::LowCutFilteringRequired() const { struct AudioProcessingImpl::ApmPublicSubmodules { ApmPublicSubmodules() {} // Accessed externally of APM without any lock acquired. - std::unique_ptr echo_control_mobile; std::unique_ptr gain_control; std::unique_ptr level_estimator; std::unique_ptr noise_suppression; @@ -253,8 +252,9 @@ struct AudioProcessingImpl::ApmPrivateSubmodules { std::unique_ptr gain_controller2; std::unique_ptr low_cut_filter; rtc::scoped_refptr echo_detector; - std::unique_ptr echo_controller; std::unique_ptr echo_cancellation; + std::unique_ptr echo_controller; + std::unique_ptr echo_control_mobile; std::unique_ptr capture_post_processor; std::unique_ptr render_pre_processor; std::unique_ptr pre_amplifier; @@ -367,9 +367,6 @@ AudioProcessingImpl::AudioProcessingImpl( capture_nonlocked_.echo_controller_enabled = static_cast(echo_control_factory_); - private_submodules_->echo_cancellation.reset(new EchoCancellationImpl()); - public_submodules_->echo_control_mobile.reset( - new EchoControlMobileImpl(&crit_render_, &crit_capture_)); public_submodules_->gain_control.reset( new GainControlImpl(&crit_render_, &crit_capture_)); public_submodules_->level_estimator.reset( @@ -388,6 +385,8 @@ AudioProcessingImpl::AudioProcessingImpl( new rtc::RefCountedObject(); } + private_submodules_->echo_cancellation.reset(new EchoCancellationImpl()); + private_submodules_->echo_control_mobile.reset(new EchoControlMobileImpl()); // TODO(alessiob): Move the injected gain controller once injection is // implemented. private_submodules_->gain_controller2.reset(new GainController2()); @@ -515,7 +514,7 @@ int AudioProcessingImpl::InitializeLocked() { RTC_DCHECK_EQ(0, success); success = private_submodules_->echo_cancellation->enable_delay_logging(true); RTC_DCHECK_EQ(0, success); - public_submodules_->echo_control_mobile->Initialize( + private_submodules_->echo_control_mobile->Initialize( proc_split_sample_rate_hz(), num_reverse_channels(), num_output_channels()); @@ -646,7 +645,7 @@ void AudioProcessingImpl::ApplyConfig(const AudioProcessing::Config& config) { private_submodules_->echo_cancellation->Enable( config_.echo_canceller.enabled && !config_.echo_canceller.mobile_mode); - public_submodules_->echo_control_mobile->Enable( + private_submodules_->echo_control_mobile->Enable( config_.echo_canceller.enabled && config_.echo_canceller.mobile_mode); private_submodules_->echo_cancellation->set_suppression_level( @@ -1063,7 +1062,7 @@ void AudioProcessingImpl::EmptyQueuedRenderAudio() { } while (aecm_render_signal_queue_->Remove(&aecm_capture_queue_buffer_)) { - public_submodules_->echo_control_mobile->ProcessRenderAudio( + private_submodules_->echo_control_mobile->ProcessRenderAudio( aecm_capture_queue_buffer_); } @@ -1085,9 +1084,6 @@ int AudioProcessingImpl::ProcessStream(AudioFrame* frame) { // Acquire the capture lock in order to safely call the function // that retrieves the render side data. This function accesses APM // getters that need the capture lock held when being called. - // The lock needs to be released as - // public_submodules_->echo_control_mobile->is_enabled() acquires this lock - // as well. rtc::CritScope cs_capture(&crit_capture_); EmptyQueuedRenderAudio(); } @@ -1157,7 +1153,7 @@ int AudioProcessingImpl::ProcessCaptureStreamLocked() { // TODO(peah): Simplify once the public API Enable functions for these // are moved to APM. RTC_DCHECK(!(private_submodules_->echo_cancellation->is_enabled() && - public_submodules_->echo_control_mobile->is_enabled())); + private_submodules_->echo_control_mobile->is_enabled())); MaybeUpdateHistograms(); @@ -1261,7 +1257,7 @@ int AudioProcessingImpl::ProcessCaptureStreamLocked() { capture_buffer, stream_delay_ms())); } - if (public_submodules_->echo_control_mobile->is_enabled() && + if (private_submodules_->echo_control_mobile->is_enabled() && public_submodules_->noise_suppression->is_enabled()) { capture_buffer->CopyLowPassToReference(); } @@ -1269,14 +1265,14 @@ int AudioProcessingImpl::ProcessCaptureStreamLocked() { // Ensure that the stream delay was set before the call to the // AECM ProcessCaptureAudio function. - if (public_submodules_->echo_control_mobile->is_enabled() && + if (private_submodules_->echo_control_mobile->is_enabled() && !was_stream_delay_set()) { return AudioProcessing::kStreamParameterNotSetError; } if (!(private_submodules_->echo_controller || private_submodules_->echo_cancellation->is_enabled())) { - RETURN_ON_ERR(public_submodules_->echo_control_mobile->ProcessCaptureAudio( + RETURN_ON_ERR(private_submodules_->echo_control_mobile->ProcessCaptureAudio( capture_buffer, stream_delay_ms())); } @@ -1676,7 +1672,7 @@ bool AudioProcessingImpl::UpdateActiveSubmoduleStates() { return submodule_states_.Update( config_.high_pass_filter.enabled, private_submodules_->echo_cancellation->is_enabled(), - public_submodules_->echo_control_mobile->is_enabled(), + private_submodules_->echo_control_mobile->is_enabled(), config_.residual_echo_detector.enabled, public_submodules_->noise_suppression->is_enabled(), public_submodules_->gain_control->is_enabled(), @@ -1864,11 +1860,11 @@ void AudioProcessingImpl::WriteAecDumpConfigMessage(bool forced) { private_submodules_->echo_cancellation->suppression_level()); apm_config.aecm_enabled = - public_submodules_->echo_control_mobile->is_enabled(); + private_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(public_submodules_->echo_control_mobile->routing_mode()); + private_submodules_->echo_control_mobile->is_comfort_noise_enabled(); + apm_config.aecm_routing_mode = static_cast( + private_submodules_->echo_control_mobile->routing_mode()); apm_config.agc_enabled = public_submodules_->gain_control->is_enabled(); apm_config.agc_mode = diff --git a/modules/audio_processing/echo_control_mobile_bit_exact_unittest.cc b/modules/audio_processing/echo_control_mobile_bit_exact_unittest.cc index 8b8369260a..7844daf522 100644 --- a/modules/audio_processing/echo_control_mobile_bit_exact_unittest.cc +++ b/modules/audio_processing/echo_control_mobile_bit_exact_unittest.cc @@ -64,9 +64,7 @@ void RunBitexactnessTest(int sample_rate_hz, EchoControlMobileImpl::RoutingMode routing_mode, bool comfort_noise_enabled, const rtc::ArrayView& output_reference) { - rtc::CriticalSection crit_render; - rtc::CriticalSection crit_capture; - EchoControlMobileImpl echo_control_mobile(&crit_render, &crit_capture); + EchoControlMobileImpl echo_control_mobile; SetupComponent(sample_rate_hz, routing_mode, comfort_noise_enabled, &echo_control_mobile); diff --git a/modules/audio_processing/echo_control_mobile_impl.cc b/modules/audio_processing/echo_control_mobile_impl.cc index 3bfb65f303..b9fbf42052 100644 --- a/modules/audio_processing/echo_control_mobile_impl.cc +++ b/modules/audio_processing/echo_control_mobile_impl.cc @@ -57,10 +57,6 @@ AudioProcessing::Error MapError(int err) { } } // namespace -size_t EchoControlMobileImpl::echo_path_size_bytes() { - return WebRtcAecm_echo_path_size_bytes(); -} - struct EchoControlMobileImpl::StreamProperties { StreamProperties() = delete; StreamProperties(int sample_rate_hz, @@ -92,17 +88,10 @@ class EchoControlMobileImpl::Canceller { return state_; } - void Initialize(int sample_rate_hz, - unsigned char* external_echo_path, - size_t echo_path_size_bytes) { + void Initialize(int sample_rate_hz) { RTC_DCHECK(state_); int error = WebRtcAecm_Init(state_, sample_rate_hz); RTC_DCHECK_EQ(AudioProcessing::kNoError, error); - if (external_echo_path != NULL) { - error = WebRtcAecm_InitEchoPath(state_, external_echo_path, - echo_path_size_bytes); - RTC_DCHECK_EQ(AudioProcessing::kNoError, error); - } } private: @@ -110,27 +99,13 @@ class EchoControlMobileImpl::Canceller { RTC_DISALLOW_COPY_AND_ASSIGN(Canceller); }; -EchoControlMobileImpl::EchoControlMobileImpl(rtc::CriticalSection* crit_render, - rtc::CriticalSection* crit_capture) - : crit_render_(crit_render), - crit_capture_(crit_capture), - routing_mode_(kSpeakerphone), - comfort_noise_enabled_(false), - external_echo_path_(NULL) { - RTC_DCHECK(crit_render); - RTC_DCHECK(crit_capture); -} +EchoControlMobileImpl::EchoControlMobileImpl() + : routing_mode_(kSpeakerphone), comfort_noise_enabled_(false) {} -EchoControlMobileImpl::~EchoControlMobileImpl() { - if (external_echo_path_ != NULL) { - delete[] external_echo_path_; - external_echo_path_ = NULL; - } -} +EchoControlMobileImpl::~EchoControlMobileImpl() {} void EchoControlMobileImpl::ProcessRenderAudio( rtc::ArrayView packed_render_audio) { - rtc::CritScope cs_capture(crit_capture_); if (!enabled_) { return; } @@ -183,7 +158,6 @@ size_t EchoControlMobileImpl::NumCancellersRequired( int EchoControlMobileImpl::ProcessCaptureAudio(AudioBuffer* audio, int stream_delay_ms) { - rtc::CritScope cs_capture(crit_capture_); if (!enabled_) { return AudioProcessing::kNoError; } @@ -230,8 +204,6 @@ int EchoControlMobileImpl::ProcessCaptureAudio(AudioBuffer* audio, int EchoControlMobileImpl::Enable(bool enable) { // Ensure AEC and AECM are not both enabled. - rtc::CritScope cs_render(crit_render_); - rtc::CritScope cs_capture(crit_capture_); RTC_DCHECK(stream_properties_); if (enable && @@ -254,7 +226,6 @@ int EchoControlMobileImpl::Enable(bool enable) { } bool EchoControlMobileImpl::is_enabled() const { - rtc::CritScope cs(crit_capture_); return enabled_; } @@ -262,90 +233,26 @@ int EchoControlMobileImpl::set_routing_mode(RoutingMode mode) { if (MapSetting(mode) == -1) { return AudioProcessing::kBadParameterError; } - - { - rtc::CritScope cs(crit_capture_); routing_mode_ = mode; - } return Configure(); } EchoControlMobileImpl::RoutingMode EchoControlMobileImpl::routing_mode() const { - rtc::CritScope cs(crit_capture_); return routing_mode_; } int EchoControlMobileImpl::enable_comfort_noise(bool enable) { - { - rtc::CritScope cs(crit_capture_); comfort_noise_enabled_ = enable; - } return Configure(); } bool EchoControlMobileImpl::is_comfort_noise_enabled() const { - rtc::CritScope cs(crit_capture_); return comfort_noise_enabled_; } -int EchoControlMobileImpl::SetEchoPath(const void* echo_path, - size_t size_bytes) { - { - rtc::CritScope cs_render(crit_render_); - rtc::CritScope cs_capture(crit_capture_); - if (echo_path == NULL) { - return AudioProcessing::kNullPointerError; - } - if (size_bytes != echo_path_size_bytes()) { - // Size mismatch - return AudioProcessing::kBadParameterError; - } - - if (external_echo_path_ == NULL) { - external_echo_path_ = new unsigned char[size_bytes]; - } - memcpy(external_echo_path_, echo_path, size_bytes); - } - - // TODO(peah): Simplify once the Enable function has been removed from - // the public APM API. - RTC_DCHECK(stream_properties_); - Initialize(stream_properties_->sample_rate_hz, - stream_properties_->num_reverse_channels, - stream_properties_->num_output_channels); - return AudioProcessing::kNoError; -} - -int EchoControlMobileImpl::GetEchoPath(void* echo_path, - size_t size_bytes) const { - rtc::CritScope cs(crit_capture_); - if (echo_path == NULL) { - return AudioProcessing::kNullPointerError; - } - if (size_bytes != echo_path_size_bytes()) { - // Size mismatch - return AudioProcessing::kBadParameterError; - } - if (!enabled_) { - return AudioProcessing::kNotEnabledError; - } - - // Get the echo path from the first channel - int32_t err = - WebRtcAecm_GetEchoPath(cancellers_[0]->state(), echo_path, size_bytes); - if (err != 0) { - return MapError(err); - } - - return AudioProcessing::kNoError; -} - void EchoControlMobileImpl::Initialize(int sample_rate_hz, size_t num_reverse_channels, size_t num_output_channels) { - rtc::CritScope cs_render(crit_render_); - rtc::CritScope cs_capture(crit_capture_); - stream_properties_.reset(new StreamProperties( sample_rate_hz, num_reverse_channels, num_output_channels)); @@ -365,16 +272,12 @@ void EchoControlMobileImpl::Initialize(int sample_rate_hz, if (!canceller) { canceller.reset(new Canceller()); } - canceller->Initialize(sample_rate_hz, external_echo_path_, - echo_path_size_bytes()); + canceller->Initialize(sample_rate_hz); } - Configure(); } int EchoControlMobileImpl::Configure() { - rtc::CritScope cs_render(crit_render_); - rtc::CritScope cs_capture(crit_capture_); AecmConfig config; config.cngMode = comfort_noise_enabled_; config.echoMode = MapSetting(routing_mode_); diff --git a/modules/audio_processing/echo_control_mobile_impl.h b/modules/audio_processing/echo_control_mobile_impl.h index 6d49ef7695..d5a8b0b445 100644 --- a/modules/audio_processing/echo_control_mobile_impl.h +++ b/modules/audio_processing/echo_control_mobile_impl.h @@ -17,9 +17,6 @@ #include #include "api/array_view.h" -#include "rtc_base/constructormagic.h" -#include "rtc_base/criticalsection.h" -#include "rtc_base/thread_annotations.h" namespace webrtc { @@ -29,8 +26,7 @@ class AudioBuffer; // robust option intended for use on mobile devices. class EchoControlMobileImpl { public: - EchoControlMobileImpl(rtc::CriticalSection* crit_render, - rtc::CriticalSection* crit_capture); + EchoControlMobileImpl(); ~EchoControlMobileImpl(); @@ -58,26 +54,6 @@ class EchoControlMobileImpl { int enable_comfort_noise(bool enable); bool is_comfort_noise_enabled() const; - // A typical use case is to initialize the component with an echo path from a - // previous call. The echo path is retrieved using |GetEchoPath()|, typically - // at the end of a call. The data can then be stored for later use as an - // initializer before the next call, using |SetEchoPath()|. - // - // Controlling the echo path this way requires the data |size_bytes| to match - // the internal echo path size. This size can be acquired using - // |echo_path_size_bytes()|. |SetEchoPath()| causes an entire reset, worth - // noting if it is to be called during an ongoing call. - // - // It is possible that version incompatibilities may result in a stored echo - // path of the incorrect size. In this case, the stored path should be - // discarded. - int SetEchoPath(const void* echo_path, size_t size_bytes); - int GetEchoPath(void* echo_path, size_t size_bytes) const; - - // The returned path size is guaranteed not to change for the lifetime of - // the application. - static size_t echo_path_size_bytes(); - void ProcessRenderAudio(rtc::ArrayView packed_render_audio); int ProcessCaptureAudio(AudioBuffer* audio, int stream_delay_ms); @@ -99,20 +75,13 @@ class EchoControlMobileImpl { int Configure(); - rtc::CriticalSection* const crit_render_ RTC_ACQUIRED_BEFORE(crit_capture_); - rtc::CriticalSection* const crit_capture_; - bool enabled_ = false; - RoutingMode routing_mode_ RTC_GUARDED_BY(crit_capture_); - bool comfort_noise_enabled_ RTC_GUARDED_BY(crit_capture_); - unsigned char* external_echo_path_ RTC_GUARDED_BY(crit_render_) - RTC_GUARDED_BY(crit_capture_); + RoutingMode routing_mode_; + bool comfort_noise_enabled_; std::vector> cancellers_; std::unique_ptr stream_properties_; - - RTC_DISALLOW_IMPLICIT_CONSTRUCTORS(EchoControlMobileImpl); }; } // namespace webrtc diff --git a/modules/audio_processing/echo_control_mobile_unittest.cc b/modules/audio_processing/echo_control_mobile_unittest.cc index d7e470ca12..f0e60483d4 100644 --- a/modules/audio_processing/echo_control_mobile_unittest.cc +++ b/modules/audio_processing/echo_control_mobile_unittest.cc @@ -18,9 +18,7 @@ namespace webrtc { TEST(EchoControlMobileTest, InterfaceConfiguration) { - rtc::CriticalSection crit_render; - rtc::CriticalSection crit_capture; - EchoControlMobileImpl aecm(&crit_render, &crit_capture); + EchoControlMobileImpl aecm; aecm.Initialize(AudioProcessing::kSampleRate16kHz, 2, 2); // Turn AECM on @@ -46,28 +44,6 @@ TEST(EchoControlMobileTest, InterfaceConfiguration) { EXPECT_EQ(0, aecm.enable_comfort_noise(true)); EXPECT_TRUE(aecm.is_comfort_noise_enabled()); - // Set and get echo path - const size_t echo_path_size = aecm.echo_path_size_bytes(); - std::vector echo_path_in(echo_path_size); - std::vector echo_path_out(echo_path_size); - EXPECT_EQ(AudioProcessing::kNullPointerError, - aecm.SetEchoPath(nullptr, echo_path_size)); - EXPECT_EQ(AudioProcessing::kNullPointerError, - aecm.GetEchoPath(nullptr, echo_path_size)); - EXPECT_EQ(AudioProcessing::kBadParameterError, - aecm.GetEchoPath(echo_path_out.data(), 1)); - EXPECT_EQ(0, aecm.GetEchoPath(echo_path_out.data(), echo_path_size)); - for (size_t i = 0; i < echo_path_size; i++) { - echo_path_in[i] = echo_path_out[i] + 1; - } - EXPECT_EQ(AudioProcessing::kBadParameterError, - aecm.SetEchoPath(echo_path_in.data(), 1)); - EXPECT_EQ(0, aecm.SetEchoPath(echo_path_in.data(), echo_path_size)); - EXPECT_EQ(0, aecm.GetEchoPath(echo_path_out.data(), echo_path_size)); - for (size_t i = 0; i < echo_path_size; i++) { - EXPECT_EQ(echo_path_in[i], echo_path_out[i]); - } - // Turn AECM off EXPECT_EQ(0, aecm.Enable(false)); EXPECT_FALSE(aecm.is_enabled());