Do not use malloc/free for Mac FFTFrame.

Use a static local to hold the vector of all the FFTSetup objects that
can be shared across all FFTFrame objects.

Modeled after how PFFFT does this.

Internal change with no user-visible effects.

Bug: 962644
Change-Id: I9325f94f3ee18dd19014c00a4872c74d667bf497
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1629429Reviewed-by: Hongchan Choi <hongchan@chromium.org>
Commit-Queue: Raymond Toy <rtoy@chromium.org>
Cr-Commit-Position: refs/heads/master@{#665232}

third_party/blink/renderer/platform/audio/fft_frame.h

@@ -145,10 +145,28 @@ class PLATFORM_EXPORT FFTFrame {
   AudioFloatArray imag_data_;
 
 #if defined(OS_MACOSX)
+  // Thin wrapper around FFTSetup so we can call the appropriate routines to
+  // construct or release the FFTSetup objects.
+  class FFTSetupDatum {
+   public:
+    FFTSetupDatum(unsigned fft_size);
+    ~FFTSetupDatum();
+    FFTSetup GetSetup() const { return setup_; }
+
+   private:
+    FFTSetup setup_;
+  };
+
+  // Returns the vector that holds all of the possible FFTSetupData
+  // objects. This should be set up in the |Initialize()| method that is called
+  // when the context is created.
+  static Vector<std::unique_ptr<FFTSetupDatum>>& FFTSetups();
+
+  static void InitializeFFTSetupForSize(wtf_size_t fft_order);
+
   DSPSplitComplex& DspSplitComplex() { return frame_; }
   DSPSplitComplex DspSplitComplex() const { return frame_; }
   static FFTSetup FftSetupForSize(unsigned fft_size);
-  static FFTSetup* fft_setups_;
   FFTSetup fft_setup_;
   DSPSplitComplex frame_;
 #elif defined(WTF_USE_WEBAUDIO_FFMPEG)

third_party/blink/renderer/platform/audio/mac/fft_frame_mac.cc

@@ -33,26 +33,75 @@
 #if defined(OS_MACOSX)
 
 #include "third_party/blink/renderer/platform/audio/fft_frame.h"
+#include "third_party/blink/renderer/platform/audio/hrtf_panner.h"
 #include "third_party/blink/renderer/platform/audio/vector_math.h"
+#include "third_party/blink/renderer/platform/wtf/std_lib_extras.h"
 
 namespace blink {
 
 const int kMaxFFTPow2Size = 24;
 const int kMinFFTPow2Size = 2;
 
-FFTSetup* FFTFrame::fft_setups_ = nullptr;
+FFTFrame::FFTSetupDatum::FFTSetupDatum(unsigned log2fft_size) {
+  // We only need power-of-two sized FFTS, so FFT_RADIX2.
+  setup_ = vDSP_create_fftsetup(log2fft_size, FFT_RADIX2);
+  DCHECK(setup_);
+}
+
+FFTFrame::FFTSetupDatum::~FFTSetupDatum() {
+  DCHECK(setup_);
+
+  vDSP_destroy_fftsetup(setup_);
+}
+
+Vector<std::unique_ptr<FFTFrame::FFTSetupDatum>>& FFTFrame::FFTSetups() {
+  // TODO(rtoy): Let this bake for a bit and then remove the assertions after
+  // we're confident the first call is from the main thread.
+  static bool first_call = true;
+
+  if (first_call) {
+    // Make sure we construct the fft_setups vector below on the main thread.
+    // Once constructed, we can access it from any thread.
+    DCHECK(IsMainThread());
+    first_call = false;
+  }
+
+  // A vector to hold all of the possible FFT setups we need.  The setups are
+  // initialized lazily.
+  DEFINE_STATIC_LOCAL(Vector<std::unique_ptr<FFTSetupDatum>>, fft_setups,
+                      (kMaxFFTPow2Size));
+
+  return fft_setups;
+}
+
+void FFTFrame::InitializeFFTSetupForSize(wtf_size_t log2fft_size) {
+  auto& setup = FFTSetups();
+
+  if (!setup[log2fft_size]) {
+    // Make sure allocation of a new setup only occurs on the main thread so we
+    // don't have a race condition with multiple threads trying to write to the
+    // same element of the vector.
+    DCHECK(IsMainThread());
+
+    setup[log2fft_size] = std::make_unique<FFTSetupDatum>(log2fft_size);
+  }
+}
 
 // Normal constructor: allocates for a given fftSize
 FFTFrame::FFTFrame(unsigned fft_size)
-    : real_data_(fft_size), imag_data_(fft_size) {
-  fft_size_ = fft_size;
-  log2fft_size_ = static_cast<unsigned>(log2(fft_size));
-
+    : fft_size_(fft_size),
+      log2fft_size_(static_cast<unsigned>(log2(fft_size))),
+      real_data_(fft_size),
+      imag_data_(fft_size) {
   // We only allow power of two
   DCHECK_EQ(1UL << log2fft_size_, fft_size_);
 
-  // Lazily create and share fftSetup with other frames
-  fft_setup_ = FftSetupForSize(fft_size);
+  // Initialize the PFFFT_Setup object here so that it will be ready when we
+  // compute FFTs.
+  InitializeFFTSetupForSize(log2fft_size_);
+
+  // Get a copy of the setup from the table.
+  fft_setup_ = FftSetupForSize(log2fft_size_);
 
   // Setup frame data
   frame_.realp = real_data_.Data();
@@ -112,18 +161,9 @@ void FFTFrame::DoInverseFFT(float* data) {
   vector_math::Vsmul(data, 1, &scale, data, 1, fft_size_);
 }
 
-FFTSetup FFTFrame::FftSetupForSize(unsigned fft_size) {
-  if (!fft_setups_) {
-    fft_setups_ = (FFTSetup*)malloc(sizeof(FFTSetup) * kMaxFFTPow2Size);
-    memset(fft_setups_, 0, sizeof(FFTSetup) * kMaxFFTPow2Size);
-  }
-
-  int pow2size = static_cast<int>(log2(fft_size));
-  DCHECK_LT(pow2size, kMaxFFTPow2Size);
-  if (!fft_setups_[pow2size])
-    fft_setups_[pow2size] = vDSP_create_fftsetup(pow2size, FFT_RADIX2);
-
-  return fft_setups_[pow2size];
+FFTSetup FFTFrame::FftSetupForSize(unsigned log2fft_size) {
+  auto& setup = FFTSetups();
+  return setup[log2fft_size]->GetSetup();
 }
 
 int FFTFrame::MinFFTSize() {
@@ -134,19 +174,29 @@ int FFTFrame::MaxFFTSize() {
   return 1 << kMaxFFTPow2Size;
 }
 
-void FFTFrame::Initialize(float sample_rate) {}
+void FFTFrame::Initialize(float sample_rate) {
+  // Initialize the vector now so it's ready for use when we construct
+  // FFTFrames.
+  FFTSetups();
+
+  // Determine the order of the convolvers used by the HRTF kernel.  Allocate
+  // FFT setups for that size and for half that size.  The HRTF kernel uses half
+  // size for analysis FFTs.
+  //
+  // TODO(rtoy): Try to come up with some way so that |Initialize()| doesn't
+  // need to know about how the HRTF panner uses FFTs.
+  unsigned hrtf_order = static_cast<unsigned>(
+      log2(HRTFPanner::FftSizeForSampleRate(sample_rate)));
+  InitializeFFTSetupForSize(hrtf_order);
+  InitializeFFTSetupForSize(hrtf_order - 1);
+}
 
 void FFTFrame::Cleanup() {
-  if (!fft_setups_)
-    return;
+  auto& setups = FFTSetups();
 
-  for (int i = 0; i < kMaxFFTPow2Size; ++i) {
-    if (fft_setups_[i])
-      vDSP_destroy_fftsetup(fft_setups_[i]);
+  for (wtf_size_t k = 0; k < setups.size(); ++k) {
+    setups[k].reset();
   }
-
-  free(fft_setups_);
-  fft_setups_ = nullptr;
 }
 
 }  // namespace blink