Initialize value since calculateFinalValues may fail to do so.

Source/modules/webaudio/BiquadDSPKernel.cpp

Index: Source/modules/webaudio/BiquadDSPKernel.cpp
diff --git a/Source/modules/webaudio/BiquadDSPKernel.cpp b/Source/modules/webaudio/BiquadDSPKernel.cpp
index 5136ae118cd3e5b1ebea5b4bdcc70be50c3d3332..0516ba623c409a4bf9b796c03bdf811571a2ddc2 100644
--- a/Source/modules/webaudio/BiquadDSPKernel.cpp
+++ b/Source/modules/webaudio/BiquadDSPKernel.cpp
@@ -40,73 +40,73 @@ namespace WebCore {
 // settings of the Biquad.
 static const double MaxBiquadDelayTime = 0.2;
 
-void BiquadDSPKernel::updateCoefficientsIfNecessary(bool useSmoothing, bool forceUpdate)
+void BiquadDSPKernel::updateCoefficientsIfNecessary()
 {
-    if (forceUpdate || biquadProcessor()->filterCoefficientsDirty()) {
-        double value1;
-        double value2;
+    if (biquadProcessor()->filterCoefficientsDirty()) {
+        double cutoffFrequency;
+        double Q;
         double gain;
         double detune; // in Cents
 
         if (biquadProcessor()->hasSampleAccurateValues()) {
-            value1 = biquadProcessor()->parameter1()->finalValue();
-            value2 = biquadProcessor()->parameter2()->finalValue();
+            cutoffFrequency = biquadProcessor()->parameter1()->finalValue();
+            Q = biquadProcessor()->parameter2()->finalValue();
             gain = biquadProcessor()->parameter3()->finalValue();
             detune = biquadProcessor()->parameter4()->finalValue();
-        } else if (useSmoothing) {
-            value1 = biquadProcessor()->parameter1()->smoothedValue();
-            value2 = biquadProcessor()->parameter2()->smoothedValue();
+        } else {
+            cutoffFrequency = biquadProcessor()->parameter1()->smoothedValue();
+            Q = biquadProcessor()->parameter2()->smoothedValue();
             gain = biquadProcessor()->parameter3()->smoothedValue();
             detune = biquadProcessor()->parameter4()->smoothedValue();
-        } else {
-            value1 = biquadProcessor()->parameter1()->value();
-            value2 = biquadProcessor()->parameter2()->value();
-            gain = biquadProcessor()->parameter3()->value();
-            detune = biquadProcessor()->parameter4()->value();
         }
 
-        // Convert from Hertz to normalized frequency 0 -> 1.
-        double nyquist = this->nyquist();
-        double normalizedFrequency = value1 / nyquist;
+        updateCoefficients(cutoffFrequency, Q, gain, detune);
+    }
+}
 
-        // Offset frequency by detune.
-        if (detune)
-            normalizedFrequency *= pow(2, detune / 1200);
+void BiquadDSPKernel::updateCoefficients(double cutoffFrequency, double Q, double gain, double detune)
+{
+    // Convert from Hertz to normalized frequency 0 -> 1.
+    double nyquist = this->nyquist();
+    double normalizedFrequency = cutoffFrequency / nyquist;
 
-        // Configure the biquad with the new filter parameters for the appropriate type of filter.
-        switch (biquadProcessor()->type()) {
-        case BiquadProcessor::LowPass:
-            m_biquad.setLowpassParams(normalizedFrequency, value2);
-            break;
+    // Offset frequency by detune.
+    if (detune)
+        normalizedFrequency *= pow(2, detune / 1200);
 
-        case BiquadProcessor::HighPass:
-            m_biquad.setHighpassParams(normalizedFrequency, value2);
-            break;
+    // Configure the biquad with the new filter parameters for the appropriate type of filter.
+    switch (biquadProcessor()->type()) {
+    case BiquadProcessor::LowPass:
+        m_biquad.setLowpassParams(normalizedFrequency, Q);
+        break;
 
-        case BiquadProcessor::BandPass:
-            m_biquad.setBandpassParams(normalizedFrequency, value2);
-            break;
+    case BiquadProcessor::HighPass:
+        m_biquad.setHighpassParams(normalizedFrequency, Q);
+        break;
 
-        case BiquadProcessor::LowShelf:
-            m_biquad.setLowShelfParams(normalizedFrequency, gain);
-            break;
+    case BiquadProcessor::BandPass:
+        m_biquad.setBandpassParams(normalizedFrequency, Q);
+        break;
 
-        case BiquadProcessor::HighShelf:
-            m_biquad.setHighShelfParams(normalizedFrequency, gain);
-            break;
+    case BiquadProcessor::LowShelf:
+        m_biquad.setLowShelfParams(normalizedFrequency, gain);
+        break;
 
-        case BiquadProcessor::Peaking:
-            m_biquad.setPeakingParams(normalizedFrequency, value2, gain);
-            break;
+    case BiquadProcessor::HighShelf:
+        m_biquad.setHighShelfParams(normalizedFrequency, gain);
+        break;
 
-        case BiquadProcessor::Notch:
-            m_biquad.setNotchParams(normalizedFrequency, value2);
-            break;
+    case BiquadProcessor::Peaking:
+        m_biquad.setPeakingParams(normalizedFrequency, Q, gain);
+        break;
 
-        case BiquadProcessor::Allpass:
-            m_biquad.setAllpassParams(normalizedFrequency, value2);
-            break;
-        }
+    case BiquadProcessor::Notch:
+        m_biquad.setNotchParams(normalizedFrequency, Q);
+        break;
+
+    case BiquadProcessor::Allpass:
+        m_biquad.setAllpassParams(normalizedFrequency, Q);
+        break;
     }
 }
 
@@ -118,7 +118,14 @@ void BiquadDSPKernel::process(const float* source, float* destination, size_t fr
     // FIXME: as an optimization, implement a way that a Biquad object can simply copy its internal filter coefficients from another Biquad object.
     // Then re-factor this code to only run for the first BiquadDSPKernel of each BiquadProcessor.
 
-    updateCoefficientsIfNecessary(true, false);
+
+    // The audio thread can't block on this lock; skip updating the coefficients for this block if
+    // necessary. We'll get them the next time around.
+    {
+        MutexTryLocker tryLocker(m_processLock);
+        if (tryLocker.locked())
+            updateCoefficientsIfNecessary();
+    }
 
     m_biquad.process(source, destination, framesToProcess);
 }
@@ -142,12 +149,28 @@ void BiquadDSPKernel::getFrequencyResponse(int nFrequencies,
     for (int k = 0; k < nFrequencies; ++k)
         frequency[k] = narrowPrecisionToFloat(frequencyHz[k] / nyquist);
 
-    // We want to get the final values of the coefficients and compute
-    // the response from that instead of some intermediate smoothed
-    // set. Forcefully update the coefficients even if they are not
-    // dirty.
+    double cutoffFrequency;
+    double Q;
+    double gain;
+    double 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 biquad object here is not the same as the one being processed because a new biquad is

[Ken Russell (switch to Gerrit), 2014/06/30 19:09:54]

Please point out it's actually both the BiquadDSPKernel and its contained Biquad
objects that are different from the one being processed.

[Raymond Toy, 2014/06/30 20:45:40]

Done.

+        // created in BiquadProcessor::getFrequencyResponse.
+        MutexLocker processLocker(m_processLock);
+
+        cutoffFrequency = biquadProcessor()->parameter1()->value();
+        Q = biquadProcessor()->parameter2()->value();
+        gain = biquadProcessor()->parameter3()->value();
+        detune = biquadProcessor()->parameter4()->value();
+    }
 
-    updateCoefficientsIfNecessary(false, true);
+    updateCoefficients(cutoffFrequency, Q, gain, detune);

[Ken Russell (switch to Gerrit), 2014/06/30 17:48:55]

Does this call to updateCoefficients need to be covered by m_processLock? In
process(), above, the call to updateCoefficientsIfNecessary and from there to
updateCoefficients is covered by the lock.

[Raymond Toy, 2014/06/30 18:06:11]

It shouldn't be needed because updateCoefficients here is updating the biquad
used for the freqency response, which is not the same as the one in process().

[Ken Russell (switch to Gerrit), 2014/06/30 18:26:38]

Both BiquadDSPKernel::getFrequencyResponse and BiquadDSPKernel::process end up
calling BiquadDSPKernel::updateCoefficients which mutates
BiquadDSPKernel::m_biquad. What am I missing?

[Raymond Toy, 2014/06/30 18:44:51]

They are different m_biquad objects.  The m_biquad for getFrequencyResponse is
specially created in BiquadProcessor::getFrequencyResponse. The m_biquad in
process() is the m_biquad actually associated with the biquad in
BiquadFilterNode.
(Yes, I find this confusing too.)

[Ken Russell (switch to Gerrit), 2014/06/30 19:09:54]

It's not only different m_biquad (Biquad) objects, it's different
BiquadDSPKernel objects, both pointing to the same BiquadProcessor. One is for
actually processing and one is for querying the frequency response. Could you
please clarify this in the comment above?

It would be good if this could be made less confusing.

[Raymond Toy, 2014/06/30 20:45:40]

Clarified this in the comments.

Yes, it shouldn't this confusing:  crbug.com/390266.

 
     m_biquad.getFrequencyResponse(nFrequencies, frequency.data(), magResponse, phaseResponse);
 }