[webkit-reviews] review granted: [Bug 215810] IIRFilterNode interface is not supported : [Attachment 407228] Patch

[bugzilla-daemon at webkit.org]

Darin Adler <darin at apple.com> has granted Chris Dumez <cdumez at apple.com>'s
request for review:
Bug 215810: IIRFilterNode interface is not supported
https://bugs.webkit.org/show_bug.cgi?id=215810

Attachment 407228: Patch

https://bugs.webkit.org/attachment.cgi?id=407228&action=review




--- Comment #4 from Darin Adler <darin at apple.com> ---
Comment on attachment 407228
  --> https://bugs.webkit.org/attachment.cgi?id=407228
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=407228&action=review

> Source/WebCore/Modules/webaudio/IIRDSPKernel.cpp:37
> +    m_tailTime = m_iirFilter.tailTime(processor.sampleRate(),
processor.isFilterStable());

Why not use construction syntax for this?

> Source/WebCore/Modules/webaudio/IIRDSPKernel.cpp:46
> +    Vector<float> frequency(length);

Inline capacity for better performance? How long are these likely to be?

> Source/WebCore/Modules/webaudio/IIRDSPKernel.h:45
> +    // AudioDSPKernel
> +    void process(const float* source, float* destination, size_t
framesToProcess) final;
> +    double tailTime() const final { return m_tailTime; }
> +    double latencyTime() const final { return 0; }
> +    void reset() final;

Make these private?

> Source/WebCore/platform/audio/IIRFilter.cpp:37
> +const size_t IIRFilter::maxOrder = 20;
> +const unsigned iirRenderingQuantum = 128;

constexpr

> Source/WebCore/platform/audio/IIRFilter.cpp:41
> +const int bufferLength = 32;

constexpr

> Source/WebCore/platform/audio/IIRFilter.cpp:148
> +	       std::complex<double> zRecip = std::complex<double>(cos(omega),
sin(omega));

auto

> Source/WebCore/platform/audio/IIRFilter.cpp:152
> +	       std::complex<double> numerator =
evaluatePolynomial(m_feedforward.data(), zRecip, m_feedforward.size() - 1);
> +	       std::complex<double> denominator =
evaluatePolynomial(m_feedback.data(), zRecip, m_feedback.size() - 1);
> +	       std::complex<double> response = numerator / denominator;

auto

> Source/WebCore/platform/audio/IIRFilter.cpp:164
> +    const double maxTailTime = 10;

constexpr

> Source/WebCore/platform/audio/IIRFilter.cpp:169
> +    const float maxTailAmplitude = 1 / 32768.0;

constexpr

> Source/WebCore/platform/audio/IIRFilter.h:65
> +    const Vector<double>& m_feedforward;
> +    const Vector<double>& m_feedback;

Seems like a dangerous design. What guarantees the lifetime fo the vectors?