Bug 213393

Created attachment 402296 [details]
Patch

Comment on attachment 402296 [details]
Patch

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

> Source/WebCore/Modules/webaudio/AnalyserNode.idl:45
> +    void getFloatTimeDomainData(Float32Array? array); // FIXME: The parameter should not be nullable.

Since this is new code, why a FIXME comment? Why not make the parameter non-nullable?

Also, everything you add should be behind the Modern Web Audio flag so:
[EnabledBySetting=ModernUnprefixedWebAudio] void getFloatTimeDomainData(Float32Array array);

> Source/WebCore/Modules/webaudio/RealtimeAnalyser.cpp:265
> +void RealtimeAnalyser::getFloatTimeDomainData(Float32Array* destinationArray)

If this implementation is based on the Blink one, please state so in the ChangeLog file.

> Source/WebCore/Modules/webaudio/RealtimeAnalyser.cpp:273
> +    size_t len = std::min(fftSize, destinationArray->length());

No abbreviations in WebKit: len -> length

> Source/WebCore/Modules/webaudio/RealtimeAnalyser.cpp:285
> +        for (unsigned i = 0; i < len; ++i) {

len is a size_t so please use same type for i.

> Source/WebCore/Modules/webaudio/RealtimeAnalyser.cpp:287
> +            float value = inputBuffer[(i + writeIndex - fftSize + InputBufferSize) % InputBufferSize];

This local variable is not needed.

Created attachment 402335 [details]
Patch

Comment on attachment 402335 [details]
Patch

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

> Source/WebCore/Modules/webaudio/AnalyserNode.h:58
> +    void getFloatTimeDomainData(const RefPtr<JSC::Float32Array>& array) { m_analyser.getFloatTimeDomainData(array.get()); }

const Ref<JSC::Float32Array>&

Same for others.

> Source/WebCore/Modules/webaudio/RealtimeAnalyser.cpp:-107
> -        return;    

Please configure your IDE to stop doing these things. We try to avoid unrelated changes in patches as much as possible.

> Source/WebCore/Modules/webaudio/RealtimeAnalyser.cpp:-207
> -    if (!destinationArray)

Since destinationArray cannot take null anymore, the parameter should be of type:
Float32Array& destinationArray

This means you'll need to update AnalyzerNode::getFloatFrequencyData() to take in a Ref<> instead of a RefPtr<>.

> Source/WebCore/Modules/webaudio/RealtimeAnalyser.cpp:-232
> -    if (!destinationArray)

ditto

> Source/WebCore/Modules/webaudio/RealtimeAnalyser.cpp:-269
> -    if (!destinationArray)

ditto.

Created attachment 402474 [details]
Patch

Committed r263359: <https://trac.webkit.org/changeset/263359>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 402474 [details].

<rdar://problem/64603798>

*** Bug 220023 has been marked as a duplicate of this bug. ***