On the list I suggested changing this to copyFromChannel(Float32Array array, unsigned long start, optional unsigned long length), and that suggestion seemed to go down well. I also suggested implementing copyToChannel with the same signature.

(In reply to Robert O'Callahan (:roc) (Mozilla Corporation) from comment #2) > On the list I suggested changing this to copyFromChannel(Float32Array array, > unsigned long start, optional unsigned long length), and that suggestion > seemed to go down well. > > I also suggested implementing copyToChannel with the same signature. Oh ok, sorry I'm way behind my email. I'll do that then.

Comment on attachment 803550 [details] [diff] [review] Patch (v2) Review of attachment 803550 [details] [diff] [review]: ----------------------------------------------------------------- Discussion on the list seems to have converged to: void copyFromChannel(Float32Array destination, long channelNumber, optional unsigned long startInChannel); void copyToChannel(Float32Array source, long channelNumber, optional unsigned long startInChannel);

Er, those startInChannel parameters should just be default parameters defaulting to 0.

I don't think this should block shipping Web Audio. Webkit/Blink don't have it.

Comment on attachment 805051 [details] [diff] [review] Patch (v3) Review of attachment 805051 [details] [diff] [review]: ----------------------------------------------------------------- ::: content/media/webaudio/AudioBuffer.cpp @@ +120,5 @@ > + } > + > + const float* sourceData = mSharedChannels ? > + mSharedChannels->GetData(aChannelNumber) : > + JS_GetFloat32ArrayData(mJSChannels[aChannelNumber]); I think we need to do something here to ensure that the array has not been neutered. @@ +142,5 @@ > + return; > + } > + > + PodCopy(JS_GetFloat32ArrayData(mJSChannels[aChannelNumber]) + aStartInChannel, > + aSource.Data(), length); I think we need to do something here to ensure that the array has not been neutered.

Comment on attachment 805051 [details] [diff] [review] Patch (v3) Review of attachment 805051 [details] [diff] [review]: ----------------------------------------------------------------- ::: content/media/webaudio/AudioBuffer.cpp @@ +120,5 @@ > + } > + > + const float* sourceData = mSharedChannels ? > + mSharedChannels->GetData(aChannelNumber) : > + JS_GetFloat32ArrayData(mJSChannels[aChannelNumber]); Hmm, why? We can get the data out of mSharedChannels just fine, right? @@ +142,5 @@ > + return; > + } > + > + PodCopy(JS_GetFloat32ArrayData(mJSChannels[aChannelNumber]) + aStartInChannel, > + aSource.Data(), length); The call to RestoreJSChannelData() should do that.

(In reply to :Ehsan Akhgari (needinfo? me!) from comment #10) > ::: content/media/webaudio/AudioBuffer.cpp > @@ +120,5 @@ > > + } > > + > > + const float* sourceData = mSharedChannels ? > > + mSharedChannels->GetData(aChannelNumber) : > > + JS_GetFloat32ArrayData(mJSChannels[aChannelNumber]); > > Hmm, why? We can get the data out of mSharedChannels just fine, right? I'm talking about the case where mSharedChannels is null and one of mJSChannels was neutered by passing it through postMessage as a Transferable or something like that.

Ah I see, good point.

Comment on attachment 805254 [details] [diff] [review] Patch (v4) Review of attachment 805254 [details] [diff] [review]: ----------------------------------------------------------------- ::: content/media/webaudio/AudioBuffer.cpp @@ +118,5 @@ > + aRv.Throw(NS_ERROR_DOM_INDEX_SIZE_ERR); > + return; > + } > + > + if (!mSharedChannels && JS_GetTypedArrayLength(mJSChannels[aChannelNumber]) != mLength) { Test mJSChannels instead of !mSharedChannels. @@ +120,5 @@ > + } > + > + if (!mSharedChannels && JS_GetTypedArrayLength(mJSChannels[aChannelNumber]) != mLength) { > + // The array was probably neutered > + aRv.Throw(NS_ERROR_DOM_NOT_SUPPORTED_ERR); Throw DOM_INDEX_SIZE_ERR @@ +146,5 @@ > + if (!mSharedChannels && JS_GetTypedArrayLength(mJSChannels[aChannelNumber]) != mLength) { > + // The array was probably neutered > + aRv.Throw(NS_ERROR_DOM_NOT_SUPPORTED_ERR); > + return; > + } Test mJSChannels instead of !mSharedChannels. and throw DOM_INDEC_SIZE_ERR

(In reply to Robert O'Callahan (:roc) (Mozilla Corporation) from comment #13) > Comment on attachment 805254 [details] [diff] [review] > Patch (v4) > > Review of attachment 805254 [details] [diff] [review]: > ----------------------------------------------------------------- > > ::: content/media/webaudio/AudioBuffer.cpp > @@ +118,5 @@ > > + aRv.Throw(NS_ERROR_DOM_INDEX_SIZE_ERR); > > + return; > > + } > > + > > + if (!mSharedChannels && JS_GetTypedArrayLength(mJSChannels[aChannelNumber]) != mLength) { > > Test mJSChannels instead of !mSharedChannels. There is no such a check in a meaningful way. Whether or not we're holding on to the stolen read-only buffer is determined by testing mSharedChannels.

Doc is up-to-date: https://developer.mozilla.org/en-US/docs/Web/API/AudioBuffer.copyFromChannel https://developer.mozilla.org/en-US/docs/Web/API/AudioBuffer.copyToChannel https://developer.mozilla.org/en-US/docs/Web/API/AudioBuffer and https://developer.mozilla.org/en-US/Firefox/Releases/27