Fix circular deps in common_audio.

This makes it easier to import cleanly downstream, and makes it
a lot easier to reason about.

Bug: webrtc:6828
Change-Id: I953e129de73053f8619333fe7e318b36e3a1fffa
Reviewed-on: https://webrtc-review.googlesource.com/22722
Commit-Queue: Patrik Höglund <phoglund@webrtc.org>
Reviewed-by: Henrik Lundin <henrik.lundin@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#20727}

common_audio/BUILD.gn

@@ -28,10 +28,6 @@ rtc_static_library("common_audio") {
     "blocker.h",
     "channel_buffer.cc",
     "channel_buffer.h",
-    "fir_filter.cc",
-    "fir_filter.h",
-    "fir_filter_neon.h",
-    "fir_filter_sse.h",
     "include/audio_util.h",
     "lapped_transform.cc",
     "lapped_transform.h",
@@ -256,16 +252,37 @@ rtc_source_set("sinc_resampler") {
   ]
 }
 
+rtc_source_set("fir_filter") {
+  sources = [
+    "fir_filter.h",
+  ]
+}
+
+rtc_source_set("fir_filter_factory") {
+  sources = [
+    "fir_filter_c.cc",
+    "fir_filter_c.h",
+    "fir_filter_factory.cc",
+    "fir_filter_factory.h",
+  ]
+  deps = [
+    ":fir_filter",
+    "../rtc_base:rtc_base_approved",
+    "../system_wrappers:cpu_features_api",
+  ]
+  if (current_cpu == "x86" || current_cpu == "x64") {
+    deps += [ ":common_audio_sse2" ]
+  }
+  if (rtc_build_with_neon) {
+    deps += [ ":common_audio_neon" ]
+  }
+}
+
 if (current_cpu == "x86" || current_cpu == "x64") {
   rtc_static_library("common_audio_sse2") {
-    # TODO(kjellander): Remove (bugs.webrtc.org/6828)
-    # Enabling GN check triggers dependency cycle:
-    #   :common_audio ->
-    #   :common_audio_sse2 ->
-    #   :common_audio
-    check_includes = false
     sources = [
       "fir_filter_sse.cc",
+      "fir_filter_sse.h",
       "resampler/sinc_resampler_sse.cc",
     ]
 
@@ -278,21 +295,19 @@ if (current_cpu == "x86" || current_cpu == "x64") {
       suppressed_configs += [ "//build/config/clang:find_bad_constructs" ]
     }
     deps = [
+      ":fir_filter",
       ":sinc_resampler",
+      "../rtc_base:rtc_base_approved",
+      "../system_wrappers:system_wrappers",
     ]
   }
 }
 
 if (rtc_build_with_neon) {
   rtc_static_library("common_audio_neon") {
-    # TODO(kjellander): Remove (bugs.webrtc.org/6828)
-    # Enabling GN check triggers dependency cycle:
-    #   :common_audio ->
-    #   :common_audio_neon ->
-    #   :common_audio
-    check_includes = false
     sources = [
       "fir_filter_neon.cc",
+      "fir_filter_neon.h",
       "resampler/sinc_resampler_neon.cc",
     ]
 
@@ -319,7 +334,10 @@ if (rtc_build_with_neon) {
     }
 
     deps = [
+      ":fir_filter",
       ":sinc_resampler",
+      "../rtc_base:rtc_base_approved",
+      "../system_wrappers:system_wrappers",
     ]
 
     public_deps = [
@@ -413,6 +431,8 @@ if (rtc_include_tests) {
 
     deps = [
       ":common_audio",
+      ":fir_filter",
+      ":fir_filter_factory",
       ":sinc_resampler",
       "..:webrtc_common",
       "../rtc_base:rtc_base_approved",

common_audio/fir_filter.h

@@ -18,16 +18,6 @@ namespace webrtc {
 // Finite Impulse Response filter using floating-point arithmetic.
 class FIRFilter {
  public:
-  // Creates a filter with the given coefficients. All initial state values will
-  // be zeros.
-  // The length of the chunks fed to the filter should never be greater than
-  // |max_input_length|. This is needed because, when vectorizing it is
-  // necessary to concatenate the input after the state, and resizing this array
-  // dynamically is expensive.
-  static FIRFilter* Create(const float* coefficients,
-                           size_t coefficients_length,
-                           size_t max_input_length);
-
   virtual ~FIRFilter() {}
 
   // Filters the |in| data supplied.

common_audio/fir_filter_c.cc

@@ -8,7 +8,7 @@
  *  be found in the AUTHORS file in the root of the source tree.
  */
 
-#include "common_audio/fir_filter.h"
+#include "common_audio/fir_filter_c.h"
 
 #include <string.h>
 
@@ -17,55 +17,10 @@
 #include "common_audio/fir_filter_neon.h"
 #include "common_audio/fir_filter_sse.h"
 #include "rtc_base/checks.h"
-#include "system_wrappers/include/cpu_features_wrapper.h"
 
 namespace webrtc {
 
-class FIRFilterC : public FIRFilter {
- public:
-  FIRFilterC(const float* coefficients,
-             size_t coefficients_length);
-
-  void Filter(const float* in, size_t length, float* out) override;
-
- private:
-  size_t coefficients_length_;
-  size_t state_length_;
-  std::unique_ptr<float[]> coefficients_;
-  std::unique_ptr<float[]> state_;
-};
-
-FIRFilter* FIRFilter::Create(const float* coefficients,
-                             size_t coefficients_length,
-                             size_t max_input_length) {
-  if (!coefficients || coefficients_length <= 0 || max_input_length <= 0) {
-    RTC_NOTREACHED();
-    return nullptr;
-  }
-
-  FIRFilter* filter = nullptr;
-// If we know the minimum architecture at compile time, avoid CPU detection.
-#if defined(WEBRTC_ARCH_X86_FAMILY)
-#if defined(__SSE2__)
-  filter =
-      new FIRFilterSSE2(coefficients, coefficients_length, max_input_length);
-#else
-  // x86 CPU detection required.
-  if (WebRtc_GetCPUInfo(kSSE2)) {
-    filter =
-        new FIRFilterSSE2(coefficients, coefficients_length, max_input_length);
-  } else {
-    filter = new FIRFilterC(coefficients, coefficients_length);
-  }
-#endif
-#elif defined(WEBRTC_HAS_NEON)
-  filter =
-      new FIRFilterNEON(coefficients, coefficients_length, max_input_length);
-#else
-  filter = new FIRFilterC(coefficients, coefficients_length);
-#endif
-
-  return filter;
+FIRFilterC::~FIRFilterC() {
 }
 
 FIRFilterC::FIRFilterC(const float* coefficients, size_t coefficients_length)

common_audio/fir_filter_c.h

@@ -0,0 +1,38 @@
+/*
+ *  Copyright (c) 2017 The WebRTC project authors. All Rights Reserved.
+ *
+ *  Use of this source code is governed by a BSD-style license
+ *  that can be found in the LICENSE file in the root of the source
+ *  tree. An additional intellectual property rights grant can be found
+ *  in the file PATENTS.  All contributing project authors may
+ *  be found in the AUTHORS file in the root of the source tree.
+ */
+
+#ifndef COMMON_AUDIO_FIR_FILTER_C_H_
+#define COMMON_AUDIO_FIR_FILTER_C_H_
+
+#include <string.h>
+#include <memory>
+
+#include "common_audio/fir_filter.h"
+
+namespace webrtc {
+
+class FIRFilterC : public FIRFilter {
+ public:
+  FIRFilterC(const float* coefficients,
+             size_t coefficients_length);
+  ~FIRFilterC() override;
+
+  void Filter(const float* in, size_t length, float* out) override;
+
+ private:
+  size_t coefficients_length_;
+  size_t state_length_;
+  std::unique_ptr<float[]> coefficients_;
+  std::unique_ptr<float[]> state_;
+};
+
+}  // namespace webrtc
+
+#endif  // COMMON_AUDIO_FIR_FILTER_C_H_

common_audio/fir_filter_factory.cc

@@ -0,0 +1,58 @@
+/*
+ *  Copyright (c) 2017 The WebRTC project authors. All Rights Reserved.
+ *
+ *  Use of this source code is governed by a BSD-style license
+ *  that can be found in the LICENSE file in the root of the source
+ *  tree. An additional intellectual property rights grant can be found
+ *  in the file PATENTS.  All contributing project authors may
+ *  be found in the AUTHORS file in the root of the source tree.
+ */
+
+#include "common_audio/fir_filter_factory.h"
+
+#include "common_audio/fir_filter_c.h"
+#include "rtc_base/checks.h"
+#include "system_wrappers/include/cpu_features_wrapper.h"
+
+#if defined(WEBRTC_HAS_NEON)
+#include "common_audio/fir_filter_neon.h"
+#elif defined(WEBRTC_ARCH_X86_FAMILY)
+#include "common_audio/fir_filter_sse.h"
+#endif
+
+namespace webrtc {
+
+FIRFilter* CreateFirFilter(const float* coefficients,
+                           size_t coefficients_length,
+                           size_t max_input_length) {
+  if (!coefficients || coefficients_length <= 0 || max_input_length <= 0) {
+    RTC_NOTREACHED();
+    return nullptr;
+  }
+
+  FIRFilter* filter = nullptr;
+// If we know the minimum architecture at compile time, avoid CPU detection.
+#if defined(WEBRTC_ARCH_X86_FAMILY)
+#if defined(__SSE2__)
+  filter =
+      new FIRFilterSSE2(coefficients, coefficients_length, max_input_length);
+#else
+  // x86 CPU detection required.
+  if (WebRtc_GetCPUInfo(kSSE2)) {
+    filter =
+        new FIRFilterSSE2(coefficients, coefficients_length, max_input_length);
+  } else {
+    filter = new FIRFilterC(coefficients, coefficients_length);
+  }
+#endif
+#elif defined(WEBRTC_HAS_NEON)
+  filter =
+      new FIRFilterNEON(coefficients, coefficients_length, max_input_length);
+#else
+  filter = new FIRFilterC(coefficients, coefficients_length);
+#endif
+
+  return filter;
+}
+
+}  // namespace webrtc

common_audio/fir_filter_factory.h

@@ -0,0 +1,32 @@
+  /*
+ *  Copyright (c) 2014 The WebRTC project authors. All Rights Reserved.
+ *
+ *  Use of this source code is governed by a BSD-style license
+ *  that can be found in the LICENSE file in the root of the source
+ *  tree. An additional intellectual property rights grant can be found
+ *  in the file PATENTS.  All contributing project authors may
+ *  be found in the AUTHORS file in the root of the source tree.
+ */
+
+#ifndef COMMON_AUDIO_FIR_FILTER_FACTORY_H_
+#define COMMON_AUDIO_FIR_FILTER_FACTORY_H_
+
+#include <string.h>
+
+namespace webrtc {
+
+class FIRFilter;
+
+// Creates a filter with the given coefficients. All initial state values will
+// be zeros.
+// The length of the chunks fed to the filter should never be greater than
+// |max_input_length|. This is needed because, when vectorizing it is
+// necessary to concatenate the input after the state, and resizing this array
+// dynamically is expensive.
+FIRFilter* CreateFirFilter(const float* coefficients,
+                           size_t coefficients_length,
+                           size_t max_input_length);
+
+}  // namespace webrtc
+
+#endif  // COMMON_AUDIO_FIR_FILTER_FACTORY_H_

common_audio/fir_filter_neon.cc

@@ -18,6 +18,9 @@
 
 namespace webrtc {
 
+FIRFilterNEON::~FIRFilterNEON() {
+}
+
 FIRFilterNEON::FIRFilterNEON(const float* coefficients,
                              size_t coefficients_length,
                              size_t max_input_length)

common_audio/fir_filter_neon.h

@@ -23,6 +23,7 @@ class FIRFilterNEON : public FIRFilter {
   FIRFilterNEON(const float* coefficients,
                 size_t coefficients_length,
                 size_t max_input_length);
+  ~FIRFilterNEON() override;
 
   void Filter(const float* in, size_t length, float* out) override;
 

common_audio/fir_filter_sse.cc

@@ -19,6 +19,9 @@
 
 namespace webrtc {
 
+FIRFilterSSE2::~FIRFilterSSE2() {
+}
+
 FIRFilterSSE2::FIRFilterSSE2(const float* coefficients,
                              size_t coefficients_length,
                              size_t max_input_length)

common_audio/fir_filter_sse.h

@@ -23,6 +23,7 @@ class FIRFilterSSE2 : public FIRFilter {
   FIRFilterSSE2(const float* coefficients,
                 size_t coefficients_length,
                 size_t max_input_length);
+  ~FIRFilterSSE2() override;
 
   void Filter(const float* in, size_t length, float* out) override;
 

common_audio/fir_filter_unittest.cc

@@ -9,6 +9,7 @@
  */
 
 #include "common_audio/fir_filter.h"
+#include "common_audio/fir_filter_factory.h"
 
 #include <string.h>
 
@@ -42,7 +43,7 @@ TEST(FIRFilterTest, FilterAsIdentity) {
   const float kCoefficients[] = {1.f, 0.f, 0.f, 0.f, 0.f};
   float output[kInputLength];
   std::unique_ptr<FIRFilter> filter(
-      FIRFilter::Create(kCoefficients, kCoefficientsLength, kInputLength));
+      CreateFirFilter(kCoefficients, kCoefficientsLength, kInputLength));
   filter->Filter(kInput, kInputLength, output);
 
   VerifyOutput(kInput, output, kInputLength);
@@ -52,7 +53,7 @@ TEST(FIRFilterTest, FilterUsedAsScalarMultiplication) {
   const float kCoefficients[] = {5.f, 0.f, 0.f, 0.f, 0.f};
   float output[kInputLength];
   std::unique_ptr<FIRFilter> filter(
-      FIRFilter::Create(kCoefficients, kCoefficientsLength, kInputLength));
+      CreateFirFilter(kCoefficients, kCoefficientsLength, kInputLength));
   filter->Filter(kInput, kInputLength, output);
 
   EXPECT_FLOAT_EQ(5.f, output[0]);
@@ -65,7 +66,7 @@ TEST(FIRFilterTest, FilterUsedAsInputShifting) {
   const float kCoefficients[] = {0.f, 0.f, 0.f, 0.f, 1.f};
   float output[kInputLength];
   std::unique_ptr<FIRFilter> filter(
-      FIRFilter::Create(kCoefficients, kCoefficientsLength, kInputLength));
+      CreateFirFilter(kCoefficients, kCoefficientsLength, kInputLength));
   filter->Filter(kInput, kInputLength, output);
 
   EXPECT_FLOAT_EQ(0.f, output[0]);
@@ -78,7 +79,7 @@ TEST(FIRFilterTest, FilterUsedAsInputShifting) {
 TEST(FIRFilterTest, FilterUsedAsArbitraryWeighting) {
   float output[kInputLength];
   std::unique_ptr<FIRFilter> filter(
-      FIRFilter::Create(kCoefficients, kCoefficientsLength, kInputLength));
+      CreateFirFilter(kCoefficients, kCoefficientsLength, kInputLength));
   filter->Filter(kInput, kInputLength, output);
 
   EXPECT_FLOAT_EQ(0.2f, output[0]);
@@ -91,12 +92,12 @@ TEST(FIRFilterTest, FilterUsedAsArbitraryWeighting) {
 TEST(FIRFilterTest, FilterInLengthLesserOrEqualToCoefficientsLength) {
   float output[kInputLength];
   std::unique_ptr<FIRFilter> filter(
-      FIRFilter::Create(kCoefficients, kCoefficientsLength, 2));
+      CreateFirFilter(kCoefficients, kCoefficientsLength, 2));
   filter->Filter(kInput, 2, output);
 
   EXPECT_FLOAT_EQ(0.2f, output[0]);
   EXPECT_FLOAT_EQ(0.7f, output[1]);
-  filter.reset(FIRFilter::Create(
+  filter.reset(CreateFirFilter(
       kCoefficients, kCoefficientsLength, kCoefficientsLength));
   filter->Filter(kInput, kCoefficientsLength, output);
 
@@ -108,7 +109,7 @@ TEST(FIRFilterTest, FilterInLengthLesserOrEqualToCoefficientsLength) {
 TEST(FIRFilterTest, MultipleFilterCalls) {
   float output[kInputLength];
   std::unique_ptr<FIRFilter> filter(
-      FIRFilter::Create(kCoefficients, kCoefficientsLength, 3));
+      CreateFirFilter(kCoefficients, kCoefficientsLength, 3));
   filter->Filter(kInput, 2, output);
   EXPECT_FLOAT_EQ(0.2f, output[0]);
   EXPECT_FLOAT_EQ(0.7f, output[1]);
@@ -139,11 +140,11 @@ TEST(FIRFilterTest, MultipleFilterCalls) {
 TEST(FIRFilterTest, VerifySampleBasedVsBlockBasedFiltering) {
   float output_block_based[kInputLength];
   std::unique_ptr<FIRFilter> filter(
-      FIRFilter::Create(kCoefficients, kCoefficientsLength, kInputLength));
+      CreateFirFilter(kCoefficients, kCoefficientsLength, kInputLength));
   filter->Filter(kInput, kInputLength, output_block_based);
 
   float output_sample_based[kInputLength];
-  filter.reset(FIRFilter::Create(kCoefficients, kCoefficientsLength, 1));
+  filter.reset(CreateFirFilter(kCoefficients, kCoefficientsLength, 1));
   for (size_t i = 0; i < kInputLength; ++i) {
     filter->Filter(&kInput[i], 1, &output_sample_based[i]);
   }
@@ -163,7 +164,7 @@ TEST(FIRFilterTest, SimplestHighPassFilter) {
       sizeof(kConstantInput[0]);
 
   float output[kConstantInputLength];
-  std::unique_ptr<FIRFilter> filter(FIRFilter::Create(
+  std::unique_ptr<FIRFilter> filter(CreateFirFilter(
       kCoefficients, kCoefficientsLength, kConstantInputLength));
   filter->Filter(kConstantInput, kConstantInputLength, output);
   EXPECT_FLOAT_EQ(1.f, output[0]);
@@ -182,7 +183,7 @@ TEST(FIRFilterTest, SimplestLowPassFilter) {
                                         sizeof(kHighFrequencyInput[0]);
 
   float output[kHighFrequencyInputLength];
-  std::unique_ptr<FIRFilter> filter(FIRFilter::Create(
+  std::unique_ptr<FIRFilter> filter(CreateFirFilter(
       kCoefficients, kCoefficientsLength, kHighFrequencyInputLength));
   filter->Filter(kHighFrequencyInput, kHighFrequencyInputLength, output);
   EXPECT_FLOAT_EQ(-1.f, output[0]);
@@ -194,12 +195,12 @@ TEST(FIRFilterTest, SimplestLowPassFilter) {
 TEST(FIRFilterTest, SameOutputWhenSwapedCoefficientsAndInput) {
   float output[kCoefficientsLength];
   float output_swaped[kCoefficientsLength];
-  std::unique_ptr<FIRFilter> filter(FIRFilter::Create(
+  std::unique_ptr<FIRFilter> filter(CreateFirFilter(
       kCoefficients, kCoefficientsLength, kCoefficientsLength));
   // Use kCoefficientsLength for in_length to get same-length outputs.
   filter->Filter(kInput, kCoefficientsLength, output);
 
-  filter.reset(FIRFilter::Create(
+  filter.reset(CreateFirFilter(
       kInput, kCoefficientsLength, kCoefficientsLength));
   filter->Filter(kCoefficients, kCoefficientsLength, output_swaped);
 

common_audio/sparse_fir_filter_unittest.cc

@@ -13,6 +13,7 @@
 #include "common_audio/sparse_fir_filter.h"
 
 #include "common_audio/fir_filter.h"
+#include "common_audio/fir_filter_factory.h"
 #include "rtc_base/arraysize.h"
 #include "test/gtest.h"
 
@@ -216,7 +217,7 @@ TEST(SparseFIRFilterTest, SameOutputAsFIRFilterWhenSparsityOneAndOffsetZero) {
   float output[arraysize(kInput)];
   float sparse_output[arraysize(kInput)];
   std::unique_ptr<FIRFilter> filter(
-      FIRFilter::Create(kCoeffs, arraysize(kCoeffs), arraysize(kInput)));
+      CreateFirFilter(kCoeffs, arraysize(kCoeffs), arraysize(kInput)));
   SparseFIRFilter sparse_filter(kCoeffs,
                                 arraysize(kCoeffs),
                                 kSparsity,

modules/audio_processing/BUILD.gn

@@ -304,6 +304,8 @@ rtc_static_library("audio_processing") {
 
   deps += [
     "../../common_audio",
+    "../../common_audio:fir_filter",
+    "../../common_audio:fir_filter_factory",
     "../../rtc_base:rtc_base_approved",
     "../../system_wrappers",
   ]

modules/audio_processing/transient/wpd_node.cc

@@ -14,6 +14,7 @@
 #include <string.h>
 
 #include "common_audio/fir_filter.h"
+#include "common_audio/fir_filter_factory.h"
 #include "modules/audio_processing/transient/dyadic_decimator.h"
 #include "rtc_base/checks.h"
 
@@ -26,9 +27,9 @@ WPDNode::WPDNode(size_t length,
       // it.
       data_(new float[2 * length + 1]),
       length_(length),
-      filter_(FIRFilter::Create(coefficients,
-                                coefficients_length,
-                                2 * length + 1)) {
+      filter_(CreateFirFilter(coefficients,
+                              coefficients_length,
+                              2 * length + 1)) {
   RTC_DCHECK_GT(length, 0);
   RTC_DCHECK(coefficients);
   RTC_DCHECK_GT(coefficients_length, 0);