Removed the dependency in GainControlImpl on the ProcessingComponent class

webrtc/modules/audio_processing/gain_control_impl.cc

Index: webrtc/modules/audio_processing/gain_control_impl.cc
diff --git a/webrtc/modules/audio_processing/gain_control_impl.cc b/webrtc/modules/audio_processing/gain_control_impl.cc
index 04a6c7ba29c264d4d8f1ccd6f7c26d29ad3e25c9..4c643c1c855f3a31dd10766c812f291a37929434 100644
--- a/webrtc/modules/audio_processing/gain_control_impl.cc
+++ b/webrtc/modules/audio_processing/gain_control_impl.cc
@@ -10,8 +10,7 @@
 
 #include "webrtc/modules/audio_processing/gain_control_impl.h"
 
-#include <assert.h>
-
+#include "webrtc/base/optional.h"
 #include "webrtc/modules/audio_processing/audio_buffer.h"
 #include "webrtc/modules/audio_processing/agc/legacy/gain_control.h"
 
@@ -42,11 +41,54 @@ static const size_t kMaxNumFramesToBuffer = 100;
 
 }  // namespace
 
+class GainControlImpl::GainController {
+ public:
+  explicit GainController() {
+    state_ = WebRtcAgc_Create();
+    RTC_CHECK(state_);
+  }
+
+  ~GainController() { WebRtcAgc_Free(state_); }

[the sun, 2016/03/09 13:01:53]

Add DCHECK here or remove in the other places (and rely on class invariant).

[peah-webrtc, 2016/03/09 21:06:44]

Done.

+
+  Handle* state() {
+    RTC_DCHECK(state_);
+    return state_;
+  }
+
+  void Initialize(int minimum_capture_level,
+                  int maximum_capture_level,
+                  Mode mode,
+                  int sample_rate_hz,
+                  int capture_level) {
+    RTC_DCHECK(state_);
+    const int error =

[the sun, 2016/03/09 13:01:53]

remove const

[peah-webrtc, 2016/03/09 21:06:44]

Done.

+        WebRtcAgc_Init(state_, minimum_capture_level, maximum_capture_level,
+                       MapSetting(mode), sample_rate_hz);
+    RTC_DCHECK_EQ(0, error);
+
+    set_capture_level(capture_level);
+  }
+
+  void set_capture_level(int capture_level) {
+    capture_level_ = rtc::Optional<int>(capture_level);
+  }
+
+  int get_capture_level() {
+    RTC_DCHECK(capture_level_);
+    return *capture_level_;
+  }
+
+ private:
+  Handle* state_;
+  rtc::Optional<int> capture_level_;

[the sun, 2016/03/09 13:01:53]

The Optional seems a bit overkill in this situation, but sure, it allows to have
the DCHECK in get_capture_level(). Add a TODO that it should be removed once
GainController::Initialize() is folded into the ctor.

[peah-webrtc, 2016/03/09 21:06:44]

Done.

+
+  RTC_DISALLOW_COPY_AND_ASSIGN(GainController);
+};
+
 GainControlImpl::GainControlImpl(const AudioProcessing* apm,
                                  rtc::CriticalSection* crit_render,
                                  rtc::CriticalSection* crit_capture)
-    : ProcessingComponent(),
-      apm_(apm),
+    : apm_(apm),
       crit_render_(crit_render),
       crit_capture_(crit_capture),
       mode_(kAdaptiveAnalog),
@@ -68,20 +110,20 @@ GainControlImpl::~GainControlImpl() {}
 
 int GainControlImpl::ProcessRenderAudio(AudioBuffer* audio) {
   rtc::CritScope cs(crit_render_);
-  if (!is_component_enabled()) {
+  if (!enabled_) {
     return AudioProcessing::kNoError;
   }
 
-  assert(audio->num_frames_per_band() <= 160);
+  RTC_DCHECK_GE(160u, audio->num_frames_per_band());
 
   render_queue_buffer_.resize(0);
-  for (size_t i = 0; i < num_handles(); i++) {
-    Handle* my_handle = static_cast<Handle*>(handle(i));
+  for (size_t i = 0; i < num_handles_required(); i++) {
+    Handle* my_handle = gain_controllers_[i]->state();
     int err =
         WebRtcAgc_GetAddFarendError(my_handle, audio->num_frames_per_band());
 
     if (err != AudioProcessing::kNoError)
-      return GetHandleError(my_handle);
+      return AudioProcessing::kUnspecifiedError;
 
     // Buffer the samples in the render queue.
     render_queue_buffer_.insert(
@@ -106,16 +148,16 @@ int GainControlImpl::ProcessRenderAudio(AudioBuffer* audio) {
 void GainControlImpl::ReadQueuedRenderData() {
   rtc::CritScope cs(crit_capture_);
 
-  if (!is_component_enabled()) {
+  if (!enabled_) {
     return;
   }
 
   while (render_signal_queue_->Remove(&capture_queue_buffer_)) {
     size_t buffer_index = 0;
     const size_t num_frames_per_band =
-        capture_queue_buffer_.size() / num_handles();
-    for (size_t i = 0; i < num_handles(); i++) {
-      Handle* my_handle = static_cast<Handle*>(handle(i));
+        capture_queue_buffer_.size() / num_handles_required();
+    for (size_t i = 0; i < num_handles_required(); i++) {
+      Handle* my_handle = gain_controllers_[i]->state();
       WebRtcAgc_AddFarend(my_handle, &capture_queue_buffer_[buffer_index],
                           num_frames_per_band);
 
@@ -127,19 +169,20 @@ void GainControlImpl::ReadQueuedRenderData() {
 int GainControlImpl::AnalyzeCaptureAudio(AudioBuffer* audio) {
   rtc::CritScope cs(crit_capture_);
 
-  if (!is_component_enabled()) {
+  if (!enabled_) {
     return AudioProcessing::kNoError;
   }
 
-  assert(audio->num_frames_per_band() <= 160);
-  assert(audio->num_channels() == num_handles());
+  RTC_DCHECK_GE(160u, audio->num_frames_per_band());
+  RTC_DCHECK_EQ(audio->num_channels(), num_handles_required());
+  RTC_DCHECK_LE(num_handles_required(), gain_controllers_.size());
 
   int err = AudioProcessing::kNoError;
 
   if (mode_ == kAdaptiveAnalog) {
-    capture_levels_.assign(num_handles(), analog_capture_level_);
-    for (size_t i = 0; i < num_handles(); i++) {
-      Handle* my_handle = static_cast<Handle*>(handle(i));
+    for (size_t i = 0; i < num_handles_required(); i++) {
+      gain_controllers_[i]->set_capture_level(analog_capture_level_);
+      Handle* my_handle = gain_controllers_[i]->state();
       err = WebRtcAgc_AddMic(
           my_handle,
           audio->split_bands(i),
@@ -147,13 +190,12 @@ int GainControlImpl::AnalyzeCaptureAudio(AudioBuffer* audio) {
           audio->num_frames_per_band());
 
       if (err != AudioProcessing::kNoError) {
-        return GetHandleError(my_handle);
+        return AudioProcessing::kUnspecifiedError;
       }
     }
   } else if (mode_ == kAdaptiveDigital) {
-
-    for (size_t i = 0; i < num_handles(); i++) {
-      Handle* my_handle = static_cast<Handle*>(handle(i));
+    for (size_t i = 0; i < num_handles_required(); i++) {
+      Handle* my_handle = gain_controllers_[i]->state();
       int32_t capture_level_out = 0;
 
       err = WebRtcAgc_VirtualMic(
@@ -164,10 +206,10 @@ int GainControlImpl::AnalyzeCaptureAudio(AudioBuffer* audio) {
           analog_capture_level_,
           &capture_level_out);
 
-      capture_levels_[i] = capture_level_out;
+      gain_controllers_[i]->set_capture_level(capture_level_out);
 
       if (err != AudioProcessing::kNoError) {
-        return GetHandleError(my_handle);
+        return AudioProcessing::kUnspecifiedError;
       }
 
     }
@@ -179,7 +221,7 @@ int GainControlImpl::AnalyzeCaptureAudio(AudioBuffer* audio) {
 int GainControlImpl::ProcessCaptureAudio(AudioBuffer* audio) {
   rtc::CritScope cs(crit_capture_);
 
-  if (!is_component_enabled()) {
+  if (!enabled_) {
     return AudioProcessing::kNoError;
   }
 
@@ -187,33 +229,28 @@ int GainControlImpl::ProcessCaptureAudio(AudioBuffer* audio) {
     return AudioProcessing::kStreamParameterNotSetError;
   }
 
-  assert(audio->num_frames_per_band() <= 160);
-  assert(audio->num_channels() == num_handles());
+  RTC_DCHECK_GE(160u, audio->num_frames_per_band());
+  RTC_DCHECK_EQ(audio->num_channels(), num_handles_required());
 
   stream_is_saturated_ = false;
-  for (size_t i = 0; i < num_handles(); i++) {
-    Handle* my_handle = static_cast<Handle*>(handle(i));
+  for (size_t i = 0; i < num_handles_required(); i++) {
+    Handle* my_handle = gain_controllers_[i]->state();
     int32_t capture_level_out = 0;
     uint8_t saturation_warning = 0;
 
     // The call to stream_has_echo() is ok from a deadlock perspective
     // as the capture lock is allready held.
     int err = WebRtcAgc_Process(
-        my_handle,
-        audio->split_bands_const(i),
-        audio->num_bands(),
-        audio->num_frames_per_band(),
-        audio->split_bands(i),
-        capture_levels_[i],
-        &capture_level_out,
-        apm_->echo_cancellation()->stream_has_echo(),
-        &saturation_warning);
+        my_handle, audio->split_bands_const(i), audio->num_bands(),
+        audio->num_frames_per_band(), audio->split_bands(i),
+        gain_controllers_[i]->get_capture_level(), &capture_level_out,
+        apm_->echo_cancellation()->stream_has_echo(), &saturation_warning);
 
     if (err != AudioProcessing::kNoError) {
-      return GetHandleError(my_handle);
+      return AudioProcessing::kUnspecifiedError;
     }
 
-    capture_levels_[i] = capture_level_out;
+    gain_controllers_[i]->set_capture_level(capture_level_out);
     if (saturation_warning == 1) {
       stream_is_saturated_ = true;
     }
@@ -222,11 +259,11 @@ int GainControlImpl::ProcessCaptureAudio(AudioBuffer* audio) {
   if (mode_ == kAdaptiveAnalog) {
     // Take the analog level to be the average across the handles.
     analog_capture_level_ = 0;
-    for (size_t i = 0; i < num_handles(); i++) {
-      analog_capture_level_ += capture_levels_[i];
+    for (size_t i = 0; i < num_handles_required(); i++) {
+      analog_capture_level_ += gain_controllers_[i]->get_capture_level();
     }
 
-    analog_capture_level_ /= num_handles();
+    analog_capture_level_ /= num_handles_required();
   }
 
   was_analog_level_set_ = false;
@@ -257,12 +294,18 @@ int GainControlImpl::stream_analog_level() {
 int GainControlImpl::Enable(bool enable) {
   rtc::CritScope cs_render(crit_render_);
   rtc::CritScope cs_capture(crit_capture_);
-  return EnableComponent(enable);
+  if (enable && !enabled_) {
+    enabled_ = enable;  // Must be set before Initialize() is called.
+    Initialize();
+  } else {
+    enabled_ = enable;
+  }
+  return AudioProcessing::kNoError;
 }
 
 bool GainControlImpl::is_enabled() const {
   rtc::CritScope cs(crit_capture_);
-  return is_component_enabled();
+  return enabled_;
 }
 
 int GainControlImpl::set_mode(Mode mode) {
@@ -273,7 +316,8 @@ int GainControlImpl::set_mode(Mode mode) {
   }
 
   mode_ = mode;
-  return Initialize();
+  Initialize();
+  return AudioProcessing::kNoError;
 }
 
 GainControl::Mode GainControlImpl::mode() const {
@@ -299,7 +343,8 @@ int GainControlImpl::set_analog_level_limits(int minimum,
   minimum_capture_level_ = minimum;
   maximum_capture_level_ = maximum;
 
-  return Initialize();
+  Initialize();
+  return AudioProcessing::kNoError;
 }
 
 int GainControlImpl::analog_level_minimum() const {
@@ -318,12 +363,14 @@ bool GainControlImpl::stream_is_saturated() const {
 }
 
 int GainControlImpl::set_target_level_dbfs(int level) {
-  rtc::CritScope cs(crit_capture_);
-  if (level > 31 || level < 0) {
-    return AudioProcessing::kBadParameterError;
-  }
+  {
+    if (level > 31 || level < 0) {

[the sun, 2016/03/09 13:01:53]

Move out of scope; scope is there to control the CritScope lifetime.

[peah-webrtc, 2016/03/09 21:06:44]

Done.

+      return AudioProcessing::kBadParameterError;
+    }
 
-  target_level_dbfs_ = level;
+    rtc::CritScope cs(crit_capture_);
+    target_level_dbfs_ = level;
+  }
   return Configure();
 }
 
@@ -333,12 +380,14 @@ int GainControlImpl::target_level_dbfs() const {
 }
 
 int GainControlImpl::set_compression_gain_db(int gain) {
-  rtc::CritScope cs(crit_capture_);
-  if (gain < 0 || gain > 90) {
-    return AudioProcessing::kBadParameterError;
-  }
+  {
+    if (gain < 0 || gain > 90) {

[the sun, 2016/03/09 13:01:53]

ditto

[peah-webrtc, 2016/03/09 21:06:44]

Done.

+      return AudioProcessing::kBadParameterError;
+    }
 
-  compression_gain_db_ = gain;
+    rtc::CritScope cs(crit_capture_);
+    compression_gain_db_ = gain;
+  }
   return Configure();
 }
 
@@ -348,8 +397,10 @@ int GainControlImpl::compression_gain_db() const {
 }
 
 int GainControlImpl::enable_limiter(bool enable) {
-  rtc::CritScope cs(crit_capture_);
-  limiter_enabled_ = enable;
+  {
+    rtc::CritScope cs(crit_capture_);
+    limiter_enabled_ = enable;
+  }
   return Configure();
 }
 
@@ -358,26 +409,39 @@ bool GainControlImpl::is_limiter_enabled() const {
   return limiter_enabled_;
 }
 
-int GainControlImpl::Initialize() {
-  int err = ProcessingComponent::Initialize();
-  if (err != AudioProcessing::kNoError || !is_component_enabled()) {
-    return err;
+void GainControlImpl::Initialize() {
+  rtc::CritScope cs_render(crit_render_);
+  rtc::CritScope cs_capture(crit_capture_);
+  if (!enabled_) {
+    return;
   }
 
-  AllocateRenderQueue();
+  int sample_rate_hz = apm_->proc_sample_rate_hz();
 
-  rtc::CritScope cs_capture(crit_capture_);
-  const int n = num_handles();
-  RTC_CHECK_GE(n, 0) << "Bad number of handles: " << n;
+  if (num_handles_required() > gain_controllers_.size()) {
+    size_t gain_controllers_old_size = gain_controllers_.size();
+    gain_controllers_.resize(num_handles_required());
 
-  capture_levels_.assign(n, analog_capture_level_);
-  return AudioProcessing::kNoError;
+    for (size_t i = gain_controllers_old_size; i < gain_controllers_.size();
+         ++i) {
+      gain_controllers_[i].reset(new GainController());
+    }
+  }
+
+  for (auto& gain_controller : gain_controllers_) {
+    gain_controller->Initialize(minimum_capture_level_, maximum_capture_level_,
+                                mode_, sample_rate_hz, analog_capture_level_);
+  }
+
+  Configure();
+
+  AllocateRenderQueue();
 }
 
 void GainControlImpl::AllocateRenderQueue() {
-  const size_t new_render_queue_element_max_size =
-      std::max<size_t>(static_cast<size_t>(1),
-                       kMaxAllowedValuesOfSamplesPerFrame * num_handles());
+  const size_t new_render_queue_element_max_size = std::max<size_t>(
+      static_cast<size_t>(1),
+      kMaxAllowedValuesOfSamplesPerFrame * num_handles_required());
 
   rtc::CritScope cs_render(crit_render_);
   rtc::CritScope cs_capture(crit_capture_);
@@ -398,26 +462,7 @@ void GainControlImpl::AllocateRenderQueue() {
   }
 }
 
-void* GainControlImpl::CreateHandle() const {
-  return WebRtcAgc_Create();
-}
-
-void GainControlImpl::DestroyHandle(void* handle) const {
-  WebRtcAgc_Free(static_cast<Handle*>(handle));
-}
-
-int GainControlImpl::InitializeHandle(void* handle) const {
-  rtc::CritScope cs_render(crit_render_);
-  rtc::CritScope cs_capture(crit_capture_);
-
-  return WebRtcAgc_Init(static_cast<Handle*>(handle),
-                          minimum_capture_level_,
-                          maximum_capture_level_,
-                          MapSetting(mode_),
-                          apm_->proc_sample_rate_hz());
-}
-
-int GainControlImpl::ConfigureHandle(void* handle) const {
+int GainControlImpl::Configure() {
   rtc::CritScope cs_render(crit_render_);
   rtc::CritScope cs_capture(crit_capture_);
   WebRtcAgcConfig config;
@@ -430,18 +475,19 @@ int GainControlImpl::ConfigureHandle(void* handle) const {
       static_cast<int16_t>(compression_gain_db_);
   config.limiterEnable = limiter_enabled_;
 
-  return WebRtcAgc_set_config(static_cast<Handle*>(handle), config);
+  int error = AudioProcessing::kNoError;
+  for (auto& gain_controller : gain_controllers_) {
+    const int handle_error =
+        WebRtcAgc_set_config(gain_controller->state(), config);
+    if (handle_error != AudioProcessing::kNoError) {
+      error = handle_error;
+    }
+  }
+  return error;
 }
 
 size_t GainControlImpl::num_handles_required() const {
   // Not locked as it only relies on APM public API which is threadsafe.
   return apm_->num_proc_channels();
 }
-
-int GainControlImpl::GetHandleError(void* handle) const {
-  // The AGC has no get_error() function.
-  // (Despite listing errors in its interface...)
-  assert(handle != NULL);
-  return AudioProcessing::kUnspecifiedError;
-}
 }  // namespace webrtc