From 0a144a705ac1fb91ee8fc1c7415354006ef6a9de Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Per=20=C3=85hgren?= Date: Tue, 9 Feb 2021 08:47:51 +0100 Subject: [PATCH] Adding initial support for lock-less informing of muting MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This CL adds the initial support for letting APM know when its output will be used or not. It also adds a new method for passing RuntimeInformation to APM that returns a bool indicating the success of the passing of information. Bug: b/177830919 Change-Id: Ic2e1b92c37241d74ca6394b785b91736ca7532aa Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/206061 Commit-Queue: Per Åhgren Reviewed-by: Alessio Bazzica Cr-Commit-Position: refs/heads/master@{#33201} --- .../agc/agc_manager_direct.cc | 26 +++++----- .../audio_processing/agc/agc_manager_direct.h | 13 +++-- .../agc/agc_manager_direct_unittest.cc | 10 ++-- .../audio_processing/audio_processing_impl.cc | 49 +++++++++++++------ .../audio_processing/audio_processing_impl.h | 9 +++- .../include/audio_processing.h | 10 +++- .../include/mock_audio_processing.h | 1 + 7 files changed, 73 insertions(+), 45 deletions(-) diff --git a/modules/audio_processing/agc/agc_manager_direct.cc b/modules/audio_processing/agc/agc_manager_direct.cc index 3f467ce1be..2454d1bbb1 100644 --- a/modules/audio_processing/agc/agc_manager_direct.cc +++ b/modules/audio_processing/agc/agc_manager_direct.cc @@ -158,7 +158,7 @@ void MonoAgc::Initialize() { target_compression_ = disable_digital_adaptive_ ? 0 : kDefaultCompressionGain; compression_ = disable_digital_adaptive_ ? 0 : target_compression_; compression_accumulator_ = compression_; - capture_muted_ = false; + capture_output_used_ = true; check_volume_on_next_process_ = true; } @@ -256,14 +256,14 @@ void MonoAgc::SetMaxLevel(int level) { << ", max_compression_gain_=" << max_compression_gain_; } -void MonoAgc::SetCaptureMuted(bool muted) { - if (capture_muted_ == muted) { +void MonoAgc::HandleCaptureOutputUsedChange(bool capture_output_used) { + if (capture_output_used_ == capture_output_used) { return; } - capture_muted_ = muted; + capture_output_used_ = capture_output_used; - if (!muted) { - // When we unmute, we should reset things to be safe. + if (capture_output_used) { + // When we start using the output, we should reset things to be safe. check_volume_on_next_process_ = true; } } @@ -427,7 +427,7 @@ AgcManagerDirect::AgcManagerDirect(int num_capture_channels, num_capture_channels_(num_capture_channels), disable_digital_adaptive_(disable_digital_adaptive), frames_since_clipped_(kClippedWaitFrames), - capture_muted_(false), + capture_output_used_(true), channel_agcs_(num_capture_channels), new_compressions_to_set_(num_capture_channels) { const int min_mic_level = GetMinMicLevel(); @@ -450,7 +450,7 @@ void AgcManagerDirect::Initialize() { for (size_t ch = 0; ch < channel_agcs_.size(); ++ch) { channel_agcs_[ch]->Initialize(); } - capture_muted_ = false; + capture_output_used_ = true; AggregateChannelLevels(); } @@ -485,7 +485,7 @@ void AgcManagerDirect::AnalyzePreProcess(const float* const* audio, size_t samples_per_channel) { RTC_DCHECK(audio); AggregateChannelLevels(); - if (capture_muted_) { + if (!capture_output_used_) { return; } @@ -520,7 +520,7 @@ void AgcManagerDirect::AnalyzePreProcess(const float* const* audio, void AgcManagerDirect::Process(const AudioBuffer* audio) { AggregateChannelLevels(); - if (capture_muted_) { + if (!capture_output_used_) { return; } @@ -549,11 +549,11 @@ absl::optional AgcManagerDirect::GetDigitalComressionGain() { return new_compressions_to_set_[channel_controlling_gain_]; } -void AgcManagerDirect::SetCaptureMuted(bool muted) { +void AgcManagerDirect::HandleCaptureOutputUsedChange(bool capture_output_used) { for (size_t ch = 0; ch < channel_agcs_.size(); ++ch) { - channel_agcs_[ch]->SetCaptureMuted(muted); + channel_agcs_[ch]->HandleCaptureOutputUsedChange(capture_output_used); } - capture_muted_ = muted; + capture_output_used_ = capture_output_used; } float AgcManagerDirect::voice_probability() const { diff --git a/modules/audio_processing/agc/agc_manager_direct.h b/modules/audio_processing/agc/agc_manager_direct.h index 8356b0c2ac..f9417cffff 100644 --- a/modules/audio_processing/agc/agc_manager_direct.h +++ b/modules/audio_processing/agc/agc_manager_direct.h @@ -51,10 +51,9 @@ class AgcManagerDirect final { void AnalyzePreProcess(const AudioBuffer* audio); void Process(const AudioBuffer* audio); - // Call when the capture stream has been muted/unmuted. This causes the - // manager to disregard all incoming audio; chances are good it's background - // noise to which we'd like to avoid adapting. - void SetCaptureMuted(bool muted); + // Call when the capture stream output has been flagged to be used/not-used. + // If unused, the manager disregards all incoming audio. + void HandleCaptureOutputUsedChange(bool capture_output_used); float voice_probability() const; int stream_analog_level() const { return stream_analog_level_; } @@ -103,7 +102,7 @@ class AgcManagerDirect final { int frames_since_clipped_; int stream_analog_level_ = 0; - bool capture_muted_; + bool capture_output_used_; int channel_controlling_gain_ = 0; std::vector> channel_agcs_; @@ -122,7 +121,7 @@ class MonoAgc { MonoAgc& operator=(const MonoAgc&) = delete; void Initialize(); - void SetCaptureMuted(bool muted); + void HandleCaptureOutputUsedChange(bool capture_output_used); void HandleClipping(); @@ -166,7 +165,7 @@ class MonoAgc { int target_compression_; int compression_; float compression_accumulator_; - bool capture_muted_ = false; + bool capture_output_used_ = true; bool check_volume_on_next_process_ = true; bool startup_ = true; int startup_min_level_; diff --git a/modules/audio_processing/agc/agc_manager_direct_unittest.cc b/modules/audio_processing/agc/agc_manager_direct_unittest.cc index c909b0879d..1954ed4b21 100644 --- a/modules/audio_processing/agc/agc_manager_direct_unittest.cc +++ b/modules/audio_processing/agc/agc_manager_direct_unittest.cc @@ -375,7 +375,7 @@ TEST_F(AgcManagerDirectTest, CompressorReachesMinimum) { } TEST_F(AgcManagerDirectTest, NoActionWhileMuted) { - manager_.SetCaptureMuted(true); + manager_.HandleCaptureOutputUsedChange(false); manager_.Process(nullptr); absl::optional new_digital_gain = manager_.GetDigitalComressionGain(); if (new_digital_gain) { @@ -386,8 +386,8 @@ TEST_F(AgcManagerDirectTest, NoActionWhileMuted) { TEST_F(AgcManagerDirectTest, UnmutingChecksVolumeWithoutRaising) { FirstProcess(); - manager_.SetCaptureMuted(true); - manager_.SetCaptureMuted(false); + manager_.HandleCaptureOutputUsedChange(false); + manager_.HandleCaptureOutputUsedChange(true); ExpectCheckVolumeAndReset(127); // SetMicVolume should not be called. EXPECT_CALL(*agc_, GetRmsErrorDb(_)).WillOnce(Return(false)); @@ -398,8 +398,8 @@ TEST_F(AgcManagerDirectTest, UnmutingChecksVolumeWithoutRaising) { TEST_F(AgcManagerDirectTest, UnmutingRaisesTooLowVolume) { FirstProcess(); - manager_.SetCaptureMuted(true); - manager_.SetCaptureMuted(false); + manager_.HandleCaptureOutputUsedChange(false); + manager_.HandleCaptureOutputUsedChange(true); ExpectCheckVolumeAndReset(11); EXPECT_CALL(*agc_, GetRmsErrorDb(_)).WillOnce(Return(false)); CallProcess(1); diff --git a/modules/audio_processing/audio_processing_impl.cc b/modules/audio_processing/audio_processing_impl.cc index 501f65933d..1d32c851ab 100644 --- a/modules/audio_processing/audio_processing_impl.cc +++ b/modules/audio_processing/audio_processing_impl.cc @@ -664,35 +664,49 @@ size_t AudioProcessingImpl::num_output_channels() const { void AudioProcessingImpl::set_output_will_be_muted(bool muted) { MutexLock lock(&mutex_capture_); - capture_.output_will_be_muted = muted; + HandleCaptureOutputUsedSetting(!muted); +} + +void AudioProcessingImpl::HandleCaptureOutputUsedSetting( + bool capture_output_used) { + capture_.capture_output_used = capture_output_used; if (submodules_.agc_manager.get()) { - submodules_.agc_manager->SetCaptureMuted(capture_.output_will_be_muted); + submodules_.agc_manager->HandleCaptureOutputUsedChange( + capture_.capture_output_used); } } void AudioProcessingImpl::SetRuntimeSetting(RuntimeSetting setting) { + PostRuntimeSetting(setting); +} + +bool AudioProcessingImpl::PostRuntimeSetting(RuntimeSetting setting) { switch (setting.type()) { case RuntimeSetting::Type::kCustomRenderProcessingRuntimeSetting: case RuntimeSetting::Type::kPlayoutAudioDeviceChange: - render_runtime_settings_enqueuer_.Enqueue(setting); - return; + return render_runtime_settings_enqueuer_.Enqueue(setting); case RuntimeSetting::Type::kCapturePreGain: case RuntimeSetting::Type::kCaptureCompressionGain: case RuntimeSetting::Type::kCaptureFixedPostGain: case RuntimeSetting::Type::kCaptureOutputUsed: - capture_runtime_settings_enqueuer_.Enqueue(setting); - return; - case RuntimeSetting::Type::kPlayoutVolumeChange: - capture_runtime_settings_enqueuer_.Enqueue(setting); - render_runtime_settings_enqueuer_.Enqueue(setting); - return; + return capture_runtime_settings_enqueuer_.Enqueue(setting); + case RuntimeSetting::Type::kPlayoutVolumeChange: { + bool enqueueing_successful; + enqueueing_successful = + capture_runtime_settings_enqueuer_.Enqueue(setting); + enqueueing_successful = + render_runtime_settings_enqueuer_.Enqueue(setting) && + enqueueing_successful; + return enqueueing_successful; + } case RuntimeSetting::Type::kNotSpecified: RTC_NOTREACHED(); - return; + return true; } // The language allows the enum to have a non-enumerator // value. Check that this doesn't happen. RTC_NOTREACHED(); + return true; } AudioProcessingImpl::RuntimeSettingEnqueuer::RuntimeSettingEnqueuer( @@ -704,7 +718,7 @@ AudioProcessingImpl::RuntimeSettingEnqueuer::RuntimeSettingEnqueuer( AudioProcessingImpl::RuntimeSettingEnqueuer::~RuntimeSettingEnqueuer() = default; -void AudioProcessingImpl::RuntimeSettingEnqueuer::Enqueue( +bool AudioProcessingImpl::RuntimeSettingEnqueuer::Enqueue( RuntimeSetting setting) { int remaining_attempts = 10; while (!runtime_settings_.Insert(&setting) && remaining_attempts-- > 0) { @@ -718,6 +732,7 @@ void AudioProcessingImpl::RuntimeSettingEnqueuer::Enqueue( RTC_HISTOGRAM_BOOLEAN("WebRTC.Audio.ApmRuntimeSettingCannotEnqueue", 1); RTC_LOG(LS_ERROR) << "Cannot enqueue a new runtime setting."; } + return remaining_attempts == 0; } int AudioProcessingImpl::MaybeInitializeCapture( @@ -844,8 +859,9 @@ void AudioProcessingImpl::HandleCaptureRuntimeSettings() { RTC_NOTREACHED(); break; case RuntimeSetting::Type::kCaptureOutputUsed: - // TODO(b/154437967): Add support for reducing complexity when it is - // known that the capture output will not be used. + bool value; + setting.GetBool(&value); + HandleCaptureOutputUsedSetting(value); break; } } @@ -1782,7 +1798,8 @@ void AudioProcessingImpl::InitializeGainController1() { submodules_.agc_manager->Initialize(); submodules_.agc_manager->SetupDigitalGainControl( submodules_.gain_control.get()); - submodules_.agc_manager->SetCaptureMuted(capture_.output_will_be_muted); + submodules_.agc_manager->HandleCaptureOutputUsedChange( + capture_.capture_output_used); } void AudioProcessingImpl::InitializeGainController2() { @@ -1993,7 +2010,7 @@ void AudioProcessingImpl::RecordAudioProcessingState() { AudioProcessingImpl::ApmCaptureState::ApmCaptureState() : was_stream_delay_set(false), - output_will_be_muted(false), + capture_output_used(true), key_pressed(false), capture_processing_format(kSampleRate16kHz), split_rate(kSampleRate16kHz), diff --git a/modules/audio_processing/audio_processing_impl.h b/modules/audio_processing/audio_processing_impl.h index 3dc13bd96d..23028ec788 100644 --- a/modules/audio_processing/audio_processing_impl.h +++ b/modules/audio_processing/audio_processing_impl.h @@ -82,6 +82,7 @@ class AudioProcessingImpl : public AudioProcessing { void AttachAecDump(std::unique_ptr aec_dump) override; void DetachAecDump() override; void SetRuntimeSetting(RuntimeSetting setting) override; + bool PostRuntimeSetting(RuntimeSetting setting) override; // Capture-side exclusive methods possibly running APM in a // multi-threaded manner. Acquire the capture lock. @@ -96,6 +97,8 @@ class AudioProcessingImpl : public AudioProcessing { bool GetLinearAecOutput( rtc::ArrayView> linear_output) const override; void set_output_will_be_muted(bool muted) override; + void HandleCaptureOutputUsedSetting(bool capture_output_used) + RTC_EXCLUSIVE_LOCKS_REQUIRED(mutex_capture_); int set_stream_delay_ms(int delay) override; void set_stream_key_pressed(bool key_pressed) override; void set_stream_analog_level(int level) override; @@ -166,7 +169,9 @@ class AudioProcessingImpl : public AudioProcessing { explicit RuntimeSettingEnqueuer( SwapQueue* runtime_settings); ~RuntimeSettingEnqueuer(); - void Enqueue(RuntimeSetting setting); + + // Enqueue setting and return whether the setting was successfully enqueued. + bool Enqueue(RuntimeSetting setting); private: SwapQueue& runtime_settings_; @@ -421,7 +426,7 @@ class AudioProcessingImpl : public AudioProcessing { ApmCaptureState(); ~ApmCaptureState(); bool was_stream_delay_set; - bool output_will_be_muted; + bool capture_output_used; bool key_pressed; std::unique_ptr capture_audio; std::unique_ptr capture_fullband_audio; diff --git a/modules/audio_processing/include/audio_processing.h b/modules/audio_processing/include/audio_processing.h index 785a2b114d..d725eed129 100644 --- a/modules/audio_processing/include/audio_processing.h +++ b/modules/audio_processing/include/audio_processing.h @@ -525,12 +525,18 @@ class RTC_EXPORT AudioProcessing : public rtc::RefCountInterface { // Set to true when the output of AudioProcessing will be muted or in some // other way not used. Ideally, the captured audio would still be processed, // but some components may change behavior based on this information. - // Default false. + // Default false. This method takes a lock. To achieve this in a lock-less + // manner the PostRuntimeSetting can instead be used. virtual void set_output_will_be_muted(bool muted) = 0; - // Enqueue a runtime setting. + // Enqueues a runtime setting. virtual void SetRuntimeSetting(RuntimeSetting setting) = 0; + // Enqueues a runtime setting. Returns a bool indicating whether the + // enqueueing was successfull. + // TODO(b/177830919): Change this to pure virtual. + virtual bool PostRuntimeSetting(RuntimeSetting setting) { return false; } + // Accepts and produces a 10 ms frame interleaved 16 bit integer audio as // specified in |input_config| and |output_config|. |src| and |dest| may use // the same memory, if desired. diff --git a/modules/audio_processing/include/mock_audio_processing.h b/modules/audio_processing/include/mock_audio_processing.h index db9ab975ff..46c5f0efbe 100644 --- a/modules/audio_processing/include/mock_audio_processing.h +++ b/modules/audio_processing/include/mock_audio_processing.h @@ -96,6 +96,7 @@ class MockAudioProcessing : public AudioProcessing { MOCK_METHOD(size_t, num_reverse_channels, (), (const, override)); MOCK_METHOD(void, set_output_will_be_muted, (bool muted), (override)); MOCK_METHOD(void, SetRuntimeSetting, (RuntimeSetting setting), (override)); + MOCK_METHOD(bool, PostRuntimeSetting, (RuntimeSetting setting), (override)); MOCK_METHOD(int, ProcessStream, (const int16_t* const src,