HighPassFilter not a ProcessingComponent anymore (bit exact).

webrtc/modules/audio_processing/high_pass_filter_impl.cc

Index: webrtc/modules/audio_processing/high_pass_filter_impl.cc
diff --git a/webrtc/modules/audio_processing/high_pass_filter_impl.cc b/webrtc/modules/audio_processing/high_pass_filter_impl.cc
index 2ad0a5098cc734da3457193330aad1df2cdc7d2f..c869f01c5b47a585f3a34966819ef5e440dc3f33 100644
--- a/webrtc/modules/audio_processing/high_pass_filter_impl.cc
+++ b/webrtc/modules/audio_processing/high_pass_filter_impl.cc
@@ -10,165 +10,116 @@
 
 #include "webrtc/modules/audio_processing/high_pass_filter_impl.h"
 
-#include <assert.h>
-
 #include "webrtc/common_audio/signal_processing/include/signal_processing_library.h"
 #include "webrtc/modules/audio_processing/audio_buffer.h"
 #include "webrtc/system_wrappers/include/critical_section_wrapper.h"
-#include "webrtc/typedefs.h"
-
 
 namespace webrtc {
 namespace {
-const int16_t kFilterCoefficients8kHz[5] =
-    {3798, -7596, 3798, 7807, -3733};
-
-const int16_t kFilterCoefficients[5] =
-    {4012, -8024, 4012, 8002, -3913};
-
-struct FilterState {
-  int16_t y[4];
-  int16_t x[2];
-  const int16_t* ba;
-};
-
-int InitializeFilter(FilterState* hpf, int sample_rate_hz) {
-  assert(hpf != NULL);
+const int16_t kFilterCoefficients8kHz[5] = {3798, -7596, 3798, 7807, -3733};
+const int16_t kFilterCoefficients[5] = {4012, -8024, 4012, 8002, -3913};
+}  // namespace
 
-  if (sample_rate_hz == AudioProcessing::kSampleRate8kHz) {
-    hpf->ba = kFilterCoefficients8kHz;
-  } else {
-    hpf->ba = kFilterCoefficients;
+class HighPassFilterImpl::MonoFilter {
+ public:
+  explicit MonoFilter(int sample_rate_hz) {

[hlundin-webrtc, 2015/12/04 12:30:09]

If you initialize ba_ as
explicit MonoFilter(int sample_rate_hz)
    : ba_(sample_rate_hz == AudioProcessing::kSampleRate8kHz ?
kFilterCoefficients8kHz : kFilterCoefficients) { ...

you can const-declare ba_:
const int16_t* const ba_ = nullptr;

[the sun, 2015/12/04 14:53:31]

Done.

+    if (sample_rate_hz == AudioProcessing::kSampleRate8kHz) {
+      ba_ = kFilterCoefficients8kHz;
+    } else {
+      ba_ = kFilterCoefficients;
+    }
+    std::memset(x_, 0, sizeof(x_));
+    std::memset(y_, 0, sizeof(y_));
   }
 
-  WebRtcSpl_MemSetW16(hpf->x, 0, 2);
-  WebRtcSpl_MemSetW16(hpf->y, 0, 4);
-
-  return AudioProcessing::kNoError;
-}
-
-int Filter(FilterState* hpf, int16_t* data, size_t length) {
-  assert(hpf != NULL);
-
-  int32_t tmp_int32 = 0;
-  int16_t* y = hpf->y;
-  int16_t* x = hpf->x;
-  const int16_t* ba = hpf->ba;
-
-  for (size_t i = 0; i < length; i++) {
-    //  y[i] = b[0] * x[i] + b[1] * x[i-1] + b[2] * x[i-2]
-    //         + -a[1] * y[i-1] + -a[2] * y[i-2];
-
-    tmp_int32 = y[1] * ba[3];  // -a[1] * y[i-1] (low part)
-    tmp_int32 += y[3] * ba[4];  // -a[2] * y[i-2] (low part)
-    tmp_int32 = (tmp_int32 >> 15);
-    tmp_int32 += y[0] * ba[3];  // -a[1] * y[i-1] (high part)
-    tmp_int32 += y[2] * ba[4];  // -a[2] * y[i-2] (high part)
-    tmp_int32 = (tmp_int32 << 1);
-
-    tmp_int32 += data[i] * ba[0];  // b[0]*x[0]
-    tmp_int32 += x[0] * ba[1];  // b[1]*x[i-1]
-    tmp_int32 += x[1] * ba[2];  // b[2]*x[i-2]
-
-    // Update state (input part)
-    x[1] = x[0];
-    x[0] = data[i];
-
-    // Update state (filtered part)
-    y[2] = y[0];
-    y[3] = y[1];
-    y[0] = static_cast<int16_t>(tmp_int32 >> 13);
-    y[1] = static_cast<int16_t>(
-        (tmp_int32 - (static_cast<int32_t>(y[0]) << 13)) << 2);
-
-    // Rounding in Q12, i.e. add 2^11
-    tmp_int32 += 2048;
-
-    // Saturate (to 2^27) so that the HP filtered signal does not overflow
-    tmp_int32 = WEBRTC_SPL_SAT(static_cast<int32_t>(134217727),
-                               tmp_int32,
-                               static_cast<int32_t>(-134217728));
-
-    // Convert back to Q0 and use rounding.
-    data[i] = (int16_t)(tmp_int32 >> 12);
+  void Filter(int16_t* data, size_t length) {
+    const int16_t* ba = ba_;

[hlundin-webrtc, 2015/12/04 12:30:09]

Why these local variables? ba, x, and y. (I guess because copy-paste, but I
think you shouldn't.)

[the sun, 2015/12/04 14:53:31]

I've tried not to touch the actual implementation too much...

[hlundin-webrtc, 2015/12/04 15:11:09]

Acknowledged.

+    int16_t* x = x_;
+    int16_t* y = y_;
+    int32_t tmp_int32 = 0;
+
+    for (size_t i = 0; i < length; i++) {
+      //  y[i] = b[0] * x[i] + b[1] * x[i-1] + b[2] * x[i-2]
+      //         + -a[1] * y[i-1] + -a[2] * y[i-2];
+
+      tmp_int32 = y[1] * ba[3];  // -a[1] * y[i-1] (low part)

[hlundin-webrtc, 2015/12/04 12:30:09]

Did you git cl format this? I was expecting the comments to align.

[the sun, 2015/12/04 14:53:30]

Done.

+      tmp_int32 += y[3] * ba[4];  // -a[2] * y[i-2] (low part)
+      tmp_int32 = (tmp_int32 >> 15);
+      tmp_int32 += y[0] * ba[3];  // -a[1] * y[i-1] (high part)
+      tmp_int32 += y[2] * ba[4];  // -a[2] * y[i-2] (high part)
+      tmp_int32 = (tmp_int32 << 1);
+
+      tmp_int32 += data[i] * ba[0];  // b[0]*x[0]
+      tmp_int32 += x[0] * ba[1];  // b[1]*x[i-1]
+      tmp_int32 += x[1] * ba[2];  // b[2]*x[i-2]
+
+      // Update state (input part)
+      x[1] = x[0];
+      x[0] = data[i];
+
+      // Update state (filtered part)
+      y[2] = y[0];
+      y[3] = y[1];
+      y[0] = static_cast<int16_t>(tmp_int32 >> 13);
+      y[1] = static_cast<int16_t>(
+          (tmp_int32 - (static_cast<int32_t>(y[0]) << 13)) << 2);
+
+      // Rounding in Q12, i.e. add 2^11

[hlundin-webrtc, 2015/12/04 12:30:09]

End with .

[the sun, 2015/12/04 14:53:30]

Done.

+      tmp_int32 += 2048;
+
+      // Saturate (to 2^27) so that the HP filtered signal does not overflow

[hlundin-webrtc, 2015/12/04 12:30:09]

End with .

[the sun, 2015/12/04 14:53:31]

Done.

+      tmp_int32 = WEBRTC_SPL_SAT(static_cast<int32_t>(134217727),
+                                 tmp_int32,
+                                 static_cast<int32_t>(-134217728));
+
+      // Convert back to Q0 and use rounding.
+      data[i] = static_cast<int16_t>(tmp_int32 >> 12);
+    }
   }
 
-  return AudioProcessing::kNoError;
-}
-}  // namespace
-
-typedef FilterState Handle;
+ private:
+  const int16_t* ba_ = nullptr;
+  int16_t x_[2];
+  int16_t y_[4];
+};
 
-HighPassFilterImpl::HighPassFilterImpl(const AudioProcessing* apm,
-                                       rtc::CriticalSection* crit)
-    : ProcessingComponent(), apm_(apm), crit_(crit) {
-  RTC_DCHECK(apm);
-  RTC_DCHECK(crit);
+HighPassFilterImpl::HighPassFilterImpl(rtc::CriticalSection* crit)
+    : crit_(crit) {
+  RTC_DCHECK(crit_);
 }
 
 HighPassFilterImpl::~HighPassFilterImpl() {}
 
-int HighPassFilterImpl::ProcessCaptureAudio(AudioBuffer* audio) {
+void HighPassFilterImpl::Initialize(int channels, int sample_rate_hz) {
   rtc::CritScope cs(crit_);
-  int err = AudioProcessing::kNoError;
-
-  if (!is_component_enabled()) {
-    return AudioProcessing::kNoError;
+  std::vector<rtc::scoped_ptr<MonoFilter>> new_filters(channels);
+  for (int i = 0; i < channels; i++) {
+    new_filters[i].reset(new MonoFilter(sample_rate_hz));
   }
+  filters_.swap(new_filters);
+}
 
-  assert(audio->num_frames_per_band() <= 160);
-
-  for (int i = 0; i < num_handles(); i++) {
-    Handle* my_handle = static_cast<Handle*>(handle(i));
-    err = Filter(my_handle,
-                 audio->split_bands(i)[kBand0To8kHz],
-                 audio->num_frames_per_band());
-
-    if (err != AudioProcessing::kNoError) {
-      return GetHandleError(my_handle);
-    }
+void HighPassFilterImpl::ProcessCaptureAudio(AudioBuffer* audio) {
+  rtc::CritScope cs(crit_);
+  if (!enabled_) {
+    return;
   }
 
-  return AudioProcessing::kNoError;
+  RTC_DCHECK_LE(audio->num_frames_per_band(), 160u);
+  for (size_t i = 0; i < filters_.size(); i++) {
+    filters_[i]->Filter(audio->split_bands(i)[kBand0To8kHz],
+                        audio->num_frames_per_band());
+  }
 }
 
 int HighPassFilterImpl::Enable(bool enable) {
   rtc::CritScope cs(crit_);
-  return EnableComponent(enable);
+  enabled_ = enable;
+  return AudioProcessing::kNoError;
 }
 
 bool HighPassFilterImpl::is_enabled() const {
   rtc::CritScope cs(crit_);
-  return is_component_enabled();
-}
-
-void* HighPassFilterImpl::CreateHandle() const {
-  return new FilterState;
-}
-
-void HighPassFilterImpl::DestroyHandle(void* handle) const {
-  delete static_cast<Handle*>(handle);
-}
-
-int HighPassFilterImpl::InitializeHandle(void* handle) const {
-  // TODO(peah): Remove dependency on apm for the
-  // capture side sample rate.
-  rtc::CritScope cs(crit_);
-  return InitializeFilter(static_cast<Handle*>(handle),
-                          apm_->proc_sample_rate_hz());
-}
-
-int HighPassFilterImpl::ConfigureHandle(void* /*handle*/) const {
-  return AudioProcessing::kNoError;  // Not configurable.
-}
-
-int HighPassFilterImpl::num_handles_required() const {
-  return apm_->num_output_channels();
-}
-
-int HighPassFilterImpl::GetHandleError(void* handle) const {
-  // The component has no detailed errors.
-  assert(handle != NULL);
-  return AudioProcessing::kUnspecifiedError;
+  return enabled_;
 }
 }  // namespace webrtc