Bug 1265408 - Avoid complex division in getFrequencyResponse; r=padenot
☠☠ backed out by 9870ee9198c8 ☠ ☠
authorDan Minor <dminor@mozilla.com>
Thu, 12 May 2016 09:15:18 -0400
changeset 325788 4dd3a54d766d9960319f6356064ccdd9d3feda88
parent 325787 24d271e05ae2268c96f1ffefe68a722b8b56765a
child 325789 d7d5bbd33e0e94f6cb024680b9efe3494aa10f3c
push id9858
push userjlund@mozilla.com
push dateMon, 01 Aug 2016 14:37:10 +0000
treeherdermozilla-aurora@203106ef6cb6 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerspadenot
bugs1265408
milestone49.0a1
Bug 1265408 - Avoid complex division in getFrequencyResponse; r=padenot Using the division operator on std::complex values fails on our linux64 AWS test machines, yielding infinities and NaNs for valid inputs. Presumably this could affect machines in the wild as well. This patch removes the use of the division operator and replaces it with real operations. MozReview-Commit-ID: 4s7xUf9ja0F
dom/media/webaudio/blink/Biquad.cpp
dom/media/webaudio/blink/IIRFilter.cpp
--- a/dom/media/webaudio/blink/Biquad.cpp
+++ b/dom/media/webaudio/blink/Biquad.cpp
@@ -170,17 +170,17 @@ void Biquad::setHighpassParams(double cu
         setNormalizedCoefficients(1, 0, 0,
                                   1, 0, 0);
     }
 }
 
 void Biquad::setNormalizedCoefficients(double b0, double b1, double b2, double a0, double a1, double a2)
 {
     double a0Inverse = 1 / a0;
-    
+
     m_b0 = b0 * a0Inverse;
     m_b1 = b1 * a0Inverse;
     m_b2 = b2 * a0Inverse;
     m_a1 = a1 * a0Inverse;
     m_a2 = a2 * a0Inverse;
 }
 
 void Biquad::setLowShelfParams(double frequency, double dbGain)
@@ -371,17 +371,17 @@ void Biquad::setBandpassParams(double fr
     // Don't let Q go negative, which causes an unstable filter.
     Q = std::max(0.0, Q);
 
     if (frequency > 0 && frequency < 1) {
         double w0 = M_PI * frequency;
         if (Q > 0) {
             double alpha = sin(w0) / (2 * Q);
             double k = cos(w0);
-    
+
             double b0 = alpha;
             double b1 = 0;
             double b2 = -alpha;
             double a0 = 1 + alpha;
             double a1 = -2 * k;
             double a2 = 1 - alpha;
 
             setNormalizedCoefficients(b0, b1, b2, a0, a1, a2);
@@ -446,22 +446,30 @@ void Biquad::getFrequencyResponse(int nF
     // with z1 = 1/z and z = exp(j*pi*frequency). Hence z1 = exp(-j*pi*frequency)
 
     // Make local copies of the coefficients as a micro-optimization.
     double b0 = m_b0;
     double b1 = m_b1;
     double b2 = m_b2;
     double a1 = m_a1;
     double a2 = m_a2;
-    
+
     for (int k = 0; k < nFrequencies; ++k) {
         double omega = -M_PI * frequency[k];
         Complex z = Complex(cos(omega), sin(omega));
         Complex numerator = b0 + (b1 + b2 * z) * z;
         Complex denominator = Complex(1, 0) + (a1 + a2 * z) * z;
-        Complex response = numerator / denominator;
+        // Strangely enough, using complex division:
+        // e.g. Complex response = numerator / denominator;
+        // fails on our test machines, yielding infinities and NaNs, so we do
+        // things the long way here.
+        double n = norm(denominator);
+        double r = (real(numerator)*real(denominator) + imag(numerator)*imag(denominator)) / n;
+        double i = (imag(numerator)*real(denominator) - real(numerator)*imag(denominator)) / n;
+        std::complex<double> response = std::complex<double>(r, i);
+
         magResponse[k] = static_cast<float>(abs(response));
         phaseResponse[k] = static_cast<float>(atan2(imag(response), real(response)));
     }
 }
 
 } // namespace WebCore
 
--- a/dom/media/webaudio/blink/IIRFilter.cpp
+++ b/dom/media/webaudio/blink/IIRFilter.cpp
@@ -124,17 +124,25 @@ void IIRFilter::getFrequencyResponse(int
 
     for (int k = 0; k < nFrequencies; ++k) {
         // zRecip = 1/z = exp(-j*frequency)
         double omega = -M_PI * frequency[k];
         std::complex<double> zRecip = std::complex<double>(cos(omega), sin(omega));
 
         std::complex<double> numerator = evaluatePolynomial(m_feedforward->Elements(), zRecip, m_feedforward->Length() - 1);
         std::complex<double> denominator = evaluatePolynomial(m_feedback->Elements(), zRecip, m_feedback->Length() - 1);
-        std::complex<double> response = numerator / denominator;
+        // Strangely enough, using complex division:
+        // e.g. Complex response = numerator / denominator;
+        // fails on our test machines, yielding infinities and NaNs, so we do
+        // things the long way here.
+        double n = norm(denominator);
+        double r = (real(numerator)*real(denominator) + imag(numerator)*imag(denominator)) / n;
+        double i = (imag(numerator)*real(denominator) - real(numerator)*imag(denominator)) / n;
+        std::complex<double> response = std::complex<double>(r, i);
+
         magResponse[k] = static_cast<float>(abs(response));
         phaseResponse[k] = static_cast<float>(atan2(imag(response), real(response)));
     }
 }
 
 bool IIRFilter::buffersAreZero()
 {
     double* xBuffer = m_xBuffer.Elements();