Bug 45003 – Add AudioBuffer files

• Status: RESOLVED
• Resolution: FIXED
• Component: New Bugs

[Chris Rogers]

Add AudioBuffer files

[Chris Rogers]

Created attachment 66136 [details]
Patch

[Chris Rogers]

Implements AudioBuffer as described in the web audio specification:
http://chromium.googlecode.com/svn/trunk/samples/audio/specification/specification.html#AudioBuffer-section

[Kenneth Russell]

Comment on attachment 66136 [details]
Patch

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

[Chris Rogers]

Created attachment 66963 [details]
Patch

[Chris Rogers]

Hi Ken, I've addressed your comments including adding setRange() and zeroRange() to TypedArrayBase which I've added in a separate patch:

https://bugs.webkit.org/show_bug.cgi?id=45419

[Kenneth Russell]

Comment on attachment 66963 [details]
Patch

Thanks for making these changes; this looks good.

[WebKit Commit Bot]

Comment on attachment 66963 [details]
Patch

Clearing flags on attachment: 66963

Committed r67128: <http://trac.webkit.org/changeset/67128>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[WebKit Review Bot]

http://trac.webkit.org/changeset/67128 might have broken Chromium Mac Release
The following changes are on the blame list:
http://trac.webkit.org/changeset/67128
http://trac.webkit.org/changeset/67129
http://trac.webkit.org/changeset/67126
http://trac.webkit.org/changeset/67127