Simplify biquad usage in getFrequencyResponse

The existing implementation was confusing because it looked like it
was modifying the filter coefficients of the running biquad filter.
Change this so that the biquad kernel is passed that will be used to
compute the response.  This makes it clear that the filter
coefficients aren't being modified. It's up to the caller to use the
correct biquad kernel.

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

third_party/blink/renderer/modules/webaudio/biquad_dsp_kernel.cc

@@ -186,10 +186,15 @@ void BiquadDSPKernel::Process(const float* source,
   biquad_.Process(source, destination, frames_to_process);
 }
 
-void BiquadDSPKernel::GetFrequencyResponse(int n_frequencies,
+void BiquadDSPKernel::GetFrequencyResponse(BiquadDSPKernel& kernel,
+                                           int n_frequencies,
                                            const float* frequency_hz,
                                            float* mag_response,
                                            float* phase_response) {
+  // Only allow on the main thread because we don't want the audio thread to be
+  // updating |kernel| while we're computing the response.
+  DCHECK(IsMainThread());
+
   bool is_good =
       n_frequencies > 0 && frequency_hz && mag_response && phase_response;
   DCHECK(is_good);
@@ -197,44 +202,15 @@ void BiquadDSPKernel::GetFrequencyResponse(int n_frequencies,
     return;
 
   Vector<float> frequency(n_frequencies);
-
-  double nyquist = this->Nyquist();
+  double nyquist = kernel.Nyquist();
 
   // Convert from frequency in Hz to normalized frequency (0 -> 1),
   // with 1 equal to the Nyquist frequency.
   for (int k = 0; k < n_frequencies; ++k)
     frequency[k] = frequency_hz[k] / nyquist;
 
-  float cutoff_frequency;
-  float q;
-  float gain;
-  float detune;  // in Cents
-
-  {
-    // Get a copy of the current biquad filter coefficients so we can update the
-    // biquad with these values. We need to synchronize with process() to
-    // prevent process() from updating the filter coefficients while we're
-    // trying to access them. The process will update it next time around.
-    //
-    // The BiquadDSPKernel object here (along with it's Biquad object) is for
-    // querying the frequency response and is NOT the same as the one in
-    // process() which is used for performing the actual filtering. This one is
-    // is created in BiquadProcessor::getFrequencyResponse for this purpose.
-    // Both, however, point to the same BiquadProcessor object.
-    //
-    // FIXME: Simplify this: crbug.com/390266
-    MutexLocker process_locker(process_lock_);
-
-    cutoff_frequency = GetBiquadProcessor()->Parameter1().Value();
-    q = GetBiquadProcessor()->Parameter2().Value();
-    gain = GetBiquadProcessor()->Parameter3().Value();
-    detune = GetBiquadProcessor()->Parameter4().Value();
-  }
-
-  UpdateCoefficients(1, &cutoff_frequency, &q, &gain, &detune);
-
-  biquad_.GetFrequencyResponse(n_frequencies, frequency.data(), mag_response,
-                               phase_response);
+  kernel.biquad_.GetFrequencyResponse(n_frequencies, frequency.data(),
+                                      mag_response, phase_response);
 }
 
 bool BiquadDSPKernel::RequiresTailProcessing() const {

third_party/blink/renderer/modules/webaudio/biquad_dsp_kernel.h

@@ -49,16 +49,24 @@ class BiquadDSPKernel final : public AudioDSPKernel {
                uint32_t frames_to_process) override;
   void Reset() override { biquad_.Reset(); }
 
-  // Get the magnitude and phase response of the filter at the given
-  // set of frequencies (in Hz). The phase response is in radians.
-  void GetFrequencyResponse(int n_frequencies,
-                            const float* frequency_hz,
-                            float* mag_response,
-                            float* phase_response);
+  // Get the magnitude and phase response of the given BiquadDSPKernel at the
+  // given set of frequencies (in Hz). The phase response is in radians.  This
+  // must be called from the main thread.
+  static void GetFrequencyResponse(BiquadDSPKernel& kernel,
+                                   int n_frequencies,
+                                   const float* frequency_hz,
+                                   float* mag_response,
+                                   float* phase_response);
 
   bool RequiresTailProcessing() const final;
   double TailTime() const override;
   double LatencyTime() const override;
+  // Update the biquad cofficients with the given parameters
+  void UpdateCoefficients(int number_of_frames,
+                          const float* frequency,
+                          const float* q,
+                          const float* gain,
+                          const float* detune);
 
  protected:
   Biquad biquad_;
@@ -69,12 +77,6 @@ class BiquadDSPKernel final : public AudioDSPKernel {
   // To prevent audio glitches when parameters are changed,
   // dezippering is used to slowly change the parameters.
   void UpdateCoefficientsIfNecessary(int);
-  // Update the biquad cofficients with the given parameters
-  void UpdateCoefficients(int,
-                          const float* frequency,
-                          const float* q,
-                          const float* gain,
-                          const float* detune);
 
  private:
   // Compute the tail time using the BiquadFilter coefficients at

third_party/blink/renderer/modules/webaudio/biquad_processor.cc

@@ -150,14 +150,39 @@ void BiquadProcessor::GetFrequencyResponse(int n_frequencies,
                                            const float* frequency_hz,
                                            float* mag_response,
                                            float* phase_response) {
+  DCHECK(IsMainThread());
+
   // Compute the frequency response on a separate temporary kernel
   // to avoid interfering with the processing running in the audio
   // thread on the main kernels.
 
   std::unique_ptr<BiquadDSPKernel> response_kernel =
       std::make_unique<BiquadDSPKernel>(this);
-  response_kernel->GetFrequencyResponse(n_frequencies, frequency_hz,
-                                        mag_response, phase_response);
+
+  float cutoff_frequency;
+  float q;
+  float gain;
+  float detune;  // in Cents
+
+  {
+    // Get a copy of the current biquad filter coefficients so we can update
+    // |response_kernel| with these values.  We need to synchronize with
+    // |Process()| to prevent process() from updating the filter coefficients
+    // while we're trying to access them.  Since this is on the main thread, we
+    // can wait.  The audio thread will update the coefficients the next time
+    // around, it it were blocked.
+    MutexLocker process_locker(process_lock_);
+
+    cutoff_frequency = Parameter1().Value();
+    q = Parameter2().Value();
+    gain = Parameter3().Value();
+    detune = Parameter4().Value();
+  }
+
+  response_kernel->UpdateCoefficients(1, &cutoff_frequency, &q, &gain, &detune);
+  BiquadDSPKernel::GetFrequencyResponse(*response_kernel, n_frequencies,
+                                        frequency_hz, mag_response,
+                                        phase_response);
 }
 
 }  // namespace blink