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}

api/audio/echo_control.h

@@ -48,6 +48,13 @@ class EchoControl {
   // 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;
 

modules/audio_processing/aec3/echo_canceller3.cc

@@ -833,6 +833,11 @@ void EchoCanceller3::SetAudioBufferDelay(int delay_ms) {
   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;
 }

modules/audio_processing/aec3/echo_canceller3.h

@@ -118,6 +118,12 @@ class EchoCanceller3 : public EchoControl {
   // 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

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 @@ AudioProcessingImpl::AudioProcessingImpl(
           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 @@ void AudioProcessingImpl::HandleCaptureOutputUsedSetting(
     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 @@ AudioProcessingImpl::RuntimeSettingEnqueuer::~RuntimeSettingEnqueuer() =
 
 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 @@ int AudioProcessingImpl::ProcessStream(const float* const* src,
 
 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 @@ void AudioProcessingImpl::HandleCaptureRuntimeSettings() {
         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);
   }
 }
 

modules/audio_processing/audio_processing_impl.h

@@ -342,6 +342,12 @@ class AudioProcessingImpl : public AudioProcessing {
   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_;

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 @@ TEST(AudioProcessingImplTest, UpdateCapturePreGainRuntimeSetting) {
       << "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

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:

modules/audio_processing/include/audio_processing.h

@@ -431,8 +431,9 @@ class RTC_EXPORT AudioProcessing : public rtc::RefCountInterface {
       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_; }

modules/audio_processing/test/echo_control_mock.h

@@ -34,6 +34,10 @@ class MockEchoControl : public EchoControl {
               (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));
 };