[webkit-reviews] review denied: [Bug 45003] Add AudioBuffer files : [Attachment 66136] Patch

[bugzilla-daemon at webkit.org]

Kenneth Russell <kbr at google.com> has denied Chris Rogers <crogers at google.com>'s
request for review:
Bug 45003: Add AudioBuffer files
https://bugs.webkit.org/show_bug.cgi?id=45003

Attachment 66136: Patch
https://bugs.webkit.org/attachment.cgi?id=66136&action=review

------- Additional Comments from Kenneth Russell <kbr at google.com>
View in context:
https://bugs.webkit.org/attachment.cgi?id=66136&action=prettypatch

Looks basically fine overall but I'd like to avoid memsets and memcpys strewn
throughout the code, so suggesting a couple of refactorings. Also two tiny
grammatical fixes.

> WebCore/webaudio/AudioBuffer.cpp:73
> +    // Copy audio data from the bus to the Float32Array's we manage.
Float32Array's -> Float32Arrays

> WebCore/webaudio/AudioBuffer.cpp:78
> +	   memcpy(channelDataArray->data(), bus->channel(i)->data(),
sizeof(float) * m_length);
Please go ahead and add the appropriate setRange method taking const T* as the
source data to TypedArrayBase.h, with the paired setRangeImpl in
ArrayBufferView.cpp. See the existing TypedArrayBase::set and
ArrayBufferView::setImpl.

> WebCore/webaudio/AudioBuffer.cpp:100
> +	       memset(getChannelData(i)->data(), 0, sizeof(float) * length());
Please go ahead and add the appropriate zeroRange call to TypedArrayBase and
implementation in ArrayBufferView.

> WebCore/webaudio/AudioBuffer.h:46
> +    // Returns 0 if data is not valid audio file.
not valid -> not a valid