Bug 214926 - Added copyFromChannel, copyToChannel to AudioBuffer
Summary: Added copyFromChannel, copyToChannel to AudioBuffer
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Web Audio (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Clark Wang
URL:
Keywords: InRadar
Depends on:
Blocks: 212611
  Show dependency treegraph
 
Reported: 2020-07-29 11:43 PDT by Clark Wang
Modified: 2020-07-29 15:08 PDT (History)
11 users (show)

See Also:


Attachments
Patch (21.47 KB, patch)
2020-07-29 11:48 PDT, Clark Wang
no flags Details | Formatted Diff | Diff
Patch (21.46 KB, patch)
2020-07-29 13:15 PDT, Clark Wang
no flags Details | Formatted Diff | Diff
Patch (21.43 KB, patch)
2020-07-29 14:12 PDT, Clark Wang
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Clark Wang 2020-07-29 11:43:23 PDT
Added copyFromChannel, copyToChannel methods to AudioBuffer according to spec: https://www.w3.org/TR/webaudio/#dom-audiobuffer-copyfromchannel-destination.
Comment 1 Clark Wang 2020-07-29 11:48:33 PDT
Created attachment 405477 [details]
Patch
Comment 2 Darin Adler 2020-07-29 11:51:11 PDT
Comment on attachment 405477 [details]
Patch

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

> Source/WebCore/Modules/webaudio/AudioBuffer.h:57
> +    ExceptionOr<void> copyFromChannel(Ref<Float32Array>, size_t channelNumber, size_t bufferOffset);
> +    ExceptionOr<void> copyToChannel(Ref<Float32Array>, size_t channelNumber, size_t startInChannel);

Ref<Float32Array> is not the correct argument type; creates unnecessary reference count churn. Ref<Float32Array>&& is better.
Comment 3 Chris Dumez 2020-07-29 11:59:18 PDT
Comment on attachment 405477 [details]
Patch

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

> Source/WebCore/Modules/webaudio/AudioBuffer.cpp:128
> +ExceptionOr<void> AudioBuffer::copyFromChannel(Ref<Float32Array> destination, size_t channelNumber, size_t bufferOffset)

Ref<Float32Array>&&. It may be a good idea to use unsigned type for the other parameters, to match the IDL

> Source/WebCore/Modules/webaudio/AudioBuffer.cpp:130
> +    if (destination.get().isShared())

destination->isShared()

> Source/WebCore/Modules/webaudio/AudioBuffer.cpp:134
> +        return Exception { IndexSizeError, "channelNumber is out of index."_s };

"out of index" ?

> Source/WebCore/Modules/webaudio/AudioBuffer.cpp:147
> +    float* dst = destination.get().data();

destination->data()

> Source/WebCore/Modules/webaudio/AudioBuffer.cpp:156
> +ExceptionOr<void> AudioBuffer::copyToChannel(Ref<Float32Array> source, size_t channelNumber, size_t bufferOffset)

Ref<Float32Array>&& and unsigned for others.

> Source/WebCore/Modules/webaudio/AudioBuffer.cpp:162
> +        return Exception { IndexSizeError, "channelNumber is out of index."_s };

"out of index"?
Comment 4 Clark Wang 2020-07-29 13:15:06 PDT
Created attachment 405491 [details]
Patch
Comment 5 Clark Wang 2020-07-29 14:12:27 PDT
Created attachment 405498 [details]
Patch
Comment 6 Clark Wang 2020-07-29 14:24:17 PDT
New patch includes a re-baselined test that incorporates new ExceptionError message.
Comment 7 EWS 2020-07-29 15:07:09 PDT
Committed r265062: <https://trac.webkit.org/changeset/265062>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 405498 [details].
Comment 8 Radar WebKit Bug Importer 2020-07-29 15:08:20 PDT
<rdar://problem/66294740>