Enabling a safe fall-back functionality for overruns in the runtime settings
Bug: b/177830919
Change-Id: I9369f6fc004ceb2b626d33b36262bc8aeabdb1a0
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/206988
Commit-Queue: Per Åhgren <peah@webrtc.org>
Reviewed-by: Alessio Bazzica <alessiob@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#33371}
diff --git a/api/audio/echo_control.h b/api/audio/echo_control.h
index 8d567bf..74fbc27 100644
--- a/api/audio/echo_control.h
+++ b/api/audio/echo_control.h
@@ -48,6 +48,13 @@
// Provides an optional external estimate of the audio buffer delay.
virtual void SetAudioBufferDelay(int delay_ms) = 0;
+ // Specifies whether the capture output will be used. The purpose of this is
+ // to allow the echo controller to deactivate some of the processing when the
+ // resulting output is anyway not used, for instance when the endpoint is
+ // muted.
+ // TODO(b/177830919): Make pure virtual.
+ virtual void SetCaptureOutputUsage(bool capture_output_used) {}
+
// Returns wheter the signal is altered.
virtual bool ActiveProcessing() const = 0;
diff --git a/modules/audio_processing/aec3/echo_canceller3.cc b/modules/audio_processing/aec3/echo_canceller3.cc
index 0f8e35d..698ede7 100644
--- a/modules/audio_processing/aec3/echo_canceller3.cc
+++ b/modules/audio_processing/aec3/echo_canceller3.cc
@@ -833,6 +833,11 @@
block_processor_->SetAudioBufferDelay(delay_ms);
}
+void EchoCanceller3::SetCaptureOutputUsage(bool capture_output_used) {
+ // TODO(b/177830919): Add functionality for reducing the complexity when the
+ // echo canceller output is not used.
+}
+
bool EchoCanceller3::ActiveProcessing() const {
return true;
}
diff --git a/modules/audio_processing/aec3/echo_canceller3.h b/modules/audio_processing/aec3/echo_canceller3.h
index bacd5df..a4aab49 100644
--- a/modules/audio_processing/aec3/echo_canceller3.h
+++ b/modules/audio_processing/aec3/echo_canceller3.h
@@ -118,6 +118,12 @@
// Provides an optional external estimate of the audio buffer delay.
void SetAudioBufferDelay(int delay_ms) override;
+ // Specifies whether the capture output will be used. The purpose of this is
+ // to allow the echo controller to deactivate some of the processing when the
+ // resulting output is anyway not used, for instance when the endpoint is
+ // muted.
+ void SetCaptureOutputUsage(bool capture_output_used) override;
+
bool ActiveProcessing() const override;
// Signals whether an external detector has detected echo leakage from the
diff --git a/modules/audio_processing/audio_processing_impl.cc b/modules/audio_processing/audio_processing_impl.cc
index 1d32c85..79a3151 100644
--- a/modules/audio_processing/audio_processing_impl.cc
+++ b/modules/audio_processing/audio_processing_impl.cc
@@ -49,8 +49,6 @@
namespace webrtc {
-constexpr int kRuntimeSettingQueueSize = 100;
-
namespace {
static bool LayoutHasKeyboard(AudioProcessing::ChannelLayout layout) {
@@ -253,8 +251,8 @@
new ApmDataDumper(rtc::AtomicOps::Increment(&instance_count_))),
use_setup_specific_default_aec3_config_(
UseSetupSpecificDefaultAec3Congfig()),
- capture_runtime_settings_(kRuntimeSettingQueueSize),
- render_runtime_settings_(kRuntimeSettingQueueSize),
+ capture_runtime_settings_(RuntimeSettingQueueSize()),
+ render_runtime_settings_(RuntimeSettingQueueSize()),
capture_runtime_settings_enqueuer_(&capture_runtime_settings_),
render_runtime_settings_enqueuer_(&render_runtime_settings_),
echo_control_factory_(std::move(echo_control_factory)),
@@ -674,6 +672,10 @@
submodules_.agc_manager->HandleCaptureOutputUsedChange(
capture_.capture_output_used);
}
+ if (submodules_.echo_controller) {
+ submodules_.echo_controller->SetCaptureOutputUsage(
+ capture_.capture_output_used);
+ }
}
void AudioProcessingImpl::SetRuntimeSetting(RuntimeSetting setting) {
@@ -720,19 +722,13 @@
bool AudioProcessingImpl::RuntimeSettingEnqueuer::Enqueue(
RuntimeSetting setting) {
- int remaining_attempts = 10;
- while (!runtime_settings_.Insert(&setting) && remaining_attempts-- > 0) {
- RuntimeSetting setting_to_discard;
- if (runtime_settings_.Remove(&setting_to_discard)) {
- RTC_LOG(LS_ERROR)
- << "The runtime settings queue is full. Oldest setting discarded.";
- }
- }
- if (remaining_attempts == 0) {
+ const bool successful_insert = runtime_settings_.Insert(&setting);
+
+ if (!successful_insert) {
RTC_HISTOGRAM_BOOLEAN("WebRTC.Audio.ApmRuntimeSettingCannotEnqueue", 1);
RTC_LOG(LS_ERROR) << "Cannot enqueue a new runtime setting.";
}
- return remaining_attempts == 0;
+ return successful_insert;
}
int AudioProcessingImpl::MaybeInitializeCapture(
@@ -806,6 +802,7 @@
void AudioProcessingImpl::HandleCaptureRuntimeSettings() {
RuntimeSetting setting;
+ int num_settings_processed = 0;
while (capture_runtime_settings_.Remove(&setting)) {
if (aec_dump_) {
aec_dump_->WriteRuntimeSetting(setting);
@@ -864,6 +861,23 @@
HandleCaptureOutputUsedSetting(value);
break;
}
+ ++num_settings_processed;
+ }
+
+ if (num_settings_processed >= RuntimeSettingQueueSize()) {
+ // Handle overrun of the runtime settings queue, which likely will has
+ // caused settings to be discarded.
+ HandleOverrunInCaptureRuntimeSettingsQueue();
+ }
+}
+
+void AudioProcessingImpl::HandleOverrunInCaptureRuntimeSettingsQueue() {
+ // Fall back to a safe state for the case when a setting for capture output
+ // usage setting has been missed.
+ capture_.capture_output_used = true;
+ if (submodules_.echo_controller) {
+ submodules_.echo_controller->SetCaptureOutputUsage(
+ capture_.capture_output_used);
}
}
diff --git a/modules/audio_processing/audio_processing_impl.h b/modules/audio_processing/audio_processing_impl.h
index 23028ec..8306ac7 100644
--- a/modules/audio_processing/audio_processing_impl.h
+++ b/modules/audio_processing/audio_processing_impl.h
@@ -342,6 +342,12 @@
void RecordAudioProcessingState()
RTC_EXCLUSIVE_LOCKS_REQUIRED(mutex_capture_);
+ // Ensures that overruns in the capture runtime settings queue is properly
+ // handled by the code, providing safe-fallbacks to mitigate the implications
+ // of any settings being missed.
+ void HandleOverrunInCaptureRuntimeSettingsQueue()
+ RTC_EXCLUSIVE_LOCKS_REQUIRED(mutex_capture_);
+
// AecDump instance used for optionally logging APM config, input
// and output to file in the AEC-dump format defined in debug.proto.
std::unique_ptr<AecDump> aec_dump_;
diff --git a/modules/audio_processing/audio_processing_impl_unittest.cc b/modules/audio_processing/audio_processing_impl_unittest.cc
index e289c31..65de4d6 100644
--- a/modules/audio_processing/audio_processing_impl_unittest.cc
+++ b/modules/audio_processing/audio_processing_impl_unittest.cc
@@ -14,6 +14,7 @@
#include <memory>
#include "api/scoped_refptr.h"
+#include "modules/audio_processing/common.h"
#include "modules/audio_processing/include/audio_processing.h"
#include "modules/audio_processing/optionally_built_submodule_creators.h"
#include "modules/audio_processing/test/audio_processing_builder_for_testing.h"
@@ -202,6 +203,88 @@
<< "Frame should be amplified.";
}
+TEST(AudioProcessingImplTest, EchoControllerObservesSetCaptureUsageChange) {
+ // Tests that the echo controller observes that the capture usage has been
+ // updated.
+ auto echo_control_factory = std::make_unique<MockEchoControlFactory>();
+ const MockEchoControlFactory* echo_control_factory_ptr =
+ echo_control_factory.get();
+
+ std::unique_ptr<AudioProcessing> apm(
+ AudioProcessingBuilderForTesting()
+ .SetEchoControlFactory(std::move(echo_control_factory))
+ .Create());
+
+ constexpr int16_t kAudioLevel = 10000;
+ constexpr int kSampleRateHz = 48000;
+ constexpr int kNumChannels = 2;
+ std::array<int16_t, kNumChannels * kSampleRateHz / 100> frame;
+ StreamConfig config(kSampleRateHz, kNumChannels, /*has_keyboard=*/false);
+ frame.fill(kAudioLevel);
+
+ MockEchoControl* echo_control_mock = echo_control_factory_ptr->GetNext();
+
+ // Ensure that SetCaptureOutputUsage is not called when no runtime settings
+ // are passed.
+ EXPECT_CALL(*echo_control_mock, SetCaptureOutputUsage(testing::_)).Times(0);
+ apm->ProcessStream(frame.data(), config, config, frame.data());
+
+ // Ensure that SetCaptureOutputUsage is called with the right information when
+ // a runtime setting is passed.
+ EXPECT_CALL(*echo_control_mock,
+ SetCaptureOutputUsage(/*capture_output_used=*/false))
+ .Times(1);
+ EXPECT_TRUE(apm->PostRuntimeSetting(
+ AudioProcessing::RuntimeSetting::CreateCaptureOutputUsedSetting(
+ /*capture_output_used=*/false)));
+ apm->ProcessStream(frame.data(), config, config, frame.data());
+
+ EXPECT_CALL(*echo_control_mock,
+ SetCaptureOutputUsage(/*capture_output_used=*/true))
+ .Times(1);
+ EXPECT_TRUE(apm->PostRuntimeSetting(
+ AudioProcessing::RuntimeSetting::CreateCaptureOutputUsedSetting(
+ /*capture_output_used=*/true)));
+ apm->ProcessStream(frame.data(), config, config, frame.data());
+
+ // The number of positions to place items in the queue is equal to the queue
+ // size minus 1.
+ constexpr int kNumSlotsInQueue = RuntimeSettingQueueSize();
+
+ // Ensure that SetCaptureOutputUsage is called with the right information when
+ // many runtime settings are passed.
+ for (int k = 0; k < kNumSlotsInQueue - 1; ++k) {
+ EXPECT_TRUE(apm->PostRuntimeSetting(
+ AudioProcessing::RuntimeSetting::CreateCaptureOutputUsedSetting(
+ /*capture_output_used=*/false)));
+ }
+ EXPECT_CALL(*echo_control_mock,
+ SetCaptureOutputUsage(/*capture_output_used=*/false))
+ .Times(kNumSlotsInQueue - 1);
+ apm->ProcessStream(frame.data(), config, config, frame.data());
+
+ // Ensure that SetCaptureOutputUsage is properly called with the fallback
+ // value when the runtime settings queue becomes full.
+ for (int k = 0; k < kNumSlotsInQueue; ++k) {
+ EXPECT_TRUE(apm->PostRuntimeSetting(
+ AudioProcessing::RuntimeSetting::CreateCaptureOutputUsedSetting(
+ /*capture_output_used=*/false)));
+ }
+ EXPECT_FALSE(apm->PostRuntimeSetting(
+ AudioProcessing::RuntimeSetting::CreateCaptureOutputUsedSetting(
+ /*capture_output_used=*/false)));
+ EXPECT_FALSE(apm->PostRuntimeSetting(
+ AudioProcessing::RuntimeSetting::CreateCaptureOutputUsedSetting(
+ /*capture_output_used=*/false)));
+ EXPECT_CALL(*echo_control_mock,
+ SetCaptureOutputUsage(/*capture_output_used=*/false))
+ .Times(kNumSlotsInQueue);
+ EXPECT_CALL(*echo_control_mock,
+ SetCaptureOutputUsage(/*capture_output_used=*/true))
+ .Times(1);
+ apm->ProcessStream(frame.data(), config, config, frame.data());
+}
+
TEST(AudioProcessingImplTest,
EchoControllerObservesPreAmplifierEchoPathGainChange) {
// Tests that the echo controller observes an echo path gain change when the
diff --git a/modules/audio_processing/common.h b/modules/audio_processing/common.h
index d8532c5..2c88c4e 100644
--- a/modules/audio_processing/common.h
+++ b/modules/audio_processing/common.h
@@ -16,6 +16,10 @@
namespace webrtc {
+constexpr int RuntimeSettingQueueSize() {
+ return 100;
+}
+
static inline size_t ChannelsFromLayout(AudioProcessing::ChannelLayout layout) {
switch (layout) {
case AudioProcessing::kMono:
diff --git a/modules/audio_processing/include/audio_processing.h b/modules/audio_processing/include/audio_processing.h
index 2312ffd..36d8256 100644
--- a/modules/audio_processing/include/audio_processing.h
+++ b/modules/audio_processing/include/audio_processing.h
@@ -431,8 +431,9 @@
return {Type::kCustomRenderProcessingRuntimeSetting, payload};
}
- static RuntimeSetting CreateCaptureOutputUsedSetting(bool payload) {
- return {Type::kCaptureOutputUsed, payload};
+ static RuntimeSetting CreateCaptureOutputUsedSetting(
+ bool capture_output_used) {
+ return {Type::kCaptureOutputUsed, capture_output_used};
}
Type type() const { return type_; }
diff --git a/modules/audio_processing/test/echo_control_mock.h b/modules/audio_processing/test/echo_control_mock.h
index 927de43..763d6e4 100644
--- a/modules/audio_processing/test/echo_control_mock.h
+++ b/modules/audio_processing/test/echo_control_mock.h
@@ -34,6 +34,10 @@
(override));
MOCK_METHOD(EchoControl::Metrics, GetMetrics, (), (const, override));
MOCK_METHOD(void, SetAudioBufferDelay, (int delay_ms), (override));
+ MOCK_METHOD(void,
+ SetCaptureOutputUsage,
+ (bool capture_output_used),
+ (override));
MOCK_METHOD(bool, ActiveProcessing, (), (const, override));
};
