1280343 - AudioBuffer copy methods do not work with SharedArrayBuffer

Status: NEW

(Core :: Web Audio, defect, P3)

Description

[Ryan Conrad]

User Agent: Mozilla/5.0 (Windows NT 10.0; WOW64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/51.0.2704.84 Safari/537.36

Steps to reproduce:

Call AudioBuffer.copyFromChannel or AudioBuffer.copyToChannel with a Float32Array view of a SharedArrayBuffer.


Actual results:

No data is copied to or from the SharedArrayBuffer. Specifically, nothing happens.


Expected results:

Data should be copied between the SharedArrayBuffer and the AudioBuffer as with a normal ArrayBuffer.

Comment 1

[Paul Adenot (:padenot)]

Ryan, do you have a test case handy where it fails for you? This is unit tested on our side and nothing has changed recently.

Thanks !

Comment 2

[Ryan Conrad]

Attachment: copyToChannel/copyFromChannel repro

SharedArrayBuffer assertions fail in Nightly

Comment 3

[Lars T Hansen [:lth]]

Sorry I did not see this earlier.

DOM APIs always must opt into using shared memory.  The DOM APIs that currently do opt into shared memory are listed in the "DOM companion spec" for the shared memory proposal: https://tc39.github.io/ecmascript_sharedmem/dom_shmem.html.  Currently there are no Audio APIs in that spec, only WebGL APIs in addition to postMessage.

Ryan, if you think such an API would be helpful then please file an issue on https://github.com/tc39/ecmascript_sharedmem so that it's visible also to the other browser vendors and the DOM people, and we can have a conversation about it.  I have no objections myself but I don't know the domain.

I'm concerned about the comment "SharedArrayBuffer assertions fail in Nightly" above so I will investigate that.  (An error is expected to be signaled in JS code but not an assertion in Firefox.)

Comment 4

[Lars T Hansen [:lth]]

Jukka, do you have an opinion about whether it would be useful to add audio APIs?

Comment 5

[Paul Adenot (:padenot)]

Lars, we're going to need to pass non-mutable slices of `ArrayBuffer` accross threads, with the upcoming `AudioWorklet` [0] spec. It's been a while I haven't looked at `SharedArrayBuffer`, but this looks like what we need.

The use case is as such:
- A few words about the model: with `AudioWorklet`s, there is some js code running on a single high priority audio thread, that calls back into a js module every now and then (the system is isosynchronous and hard-real time). 
- Every invocation, an input buffer and an output buffer are passed in as arguments.
- You can either generate audio (not touching the input buffer), and put this new generated audio in the output buffer, or process the input buffer and write the result to the output buffer. You can load a js module multiple times into different `AudioWorkletProcessors`. Those `AudioWorkletProcessors` are going to run sequentially on the audio thread.
- It's very common to load up, decode (say from mp3) and store in memory large (1GB+) amount of audio data, for example, every note on a piano, with different velocities (roughly the speed at which the finger hits the key). Because latency has to be minimized between the user doing an interaction and hearing the sound, everything is in memory ready to be read.
- We'd like to be able to send read-only copies of those buffers to different `AudioWorkletProcessors`, so they can read the data and copy them to the ouput buffer.
- We are going to build a `postMessage`-like method. We can't use `postMessage` because we don't want to be able to pass communication objects via this mechanism, but we want to be able to send buffer, either via detaching at the call-site, copying or using a `SharedArrayBuffer`.
- Ideally, doing some advanced concurrency patterns with atomics will be needed (say, a non-blocking ring-buffer), is that something that we can hope to achieve ?

Happy to provide in-depth details if needed, either here or on irc, #media, `padenot`, Paris time.

[0]: https://webaudio.github.io/web-audio-api/#AudioWorklet

Comment 6

[Lars T Hansen [:lth]]

Hm, a SharedArrayBuffer is always going to be mutable, which is why DOM APIs have to opt into accepting it (to avoid data races that they aren't aware of).  From my perspective, the only thing I would probably think about doing here is implement changes that would allow copyFromChannel and copyToChannel accept a SharedArrayBuffer as the output and input buffer, respectively, in order to avoid having to use those methods with ArrayBuffer and then copying that data into shared memory.

(Full spec here, in case that answers more questions: http://tc39.github.io/ecmascript_sharedmem/shmem.html)

If you can live with mutability, perhaps we can talk further :)

Comment 7

[Jukka Jylänki]

Oh, this is a really good catch. I missed this when looking at Web Audio interaction points. Emscripten's SDL and OpenAL audio backends don't call copyFrom/ToChannel() functions at the moment, but I can see how these could become useful in the future.

There are some other interfaces in Web Audio that do take in a Float32Array, which are quite special to a specific audio task, like for example

 void BiquadFilterNode.getFrequencyResponse (Float32Array frequencyHz, Float32Array magResponse, Float32Array phaseResponse);

which is expected to be quite rarely used, but if someone is using that, it would be useful to allow a shared Float32Array there.

Comment 8

[Jukka Jylänki]

(In reply to Paul Adenot (:padenot) from comment #5)
> Lars, we're going to need to pass non-mutable slices of `ArrayBuffer`
> accross threads, with the upcoming `AudioWorklet` [0] spec. It's been a
> while I haven't looked at `SharedArrayBuffer`, but this looks like what we
> need.

One peculiar behavior about SharedArrayBuffer interaction with any web API that knows about typed arrays (e.g. Web Audio), is that this interaction arises even in asm.js/wasm applications that might not do anything with respect to multithreading audio workloads, because of how the asm.js heap structure is laid out.

That is, in an application
 - that is written in asm.js/wasm,
 - that uses Web Audio, even if singlethreaded only on the main browser thread,
 - that happens to use SharedArrayBuffer to do multithreading for tasks unrelated to Web Audio (e.g. physics),
 - that wants to call any Web Audio API function that takes an existing Float32Array, using the asm.js application HEAPF32 (float32 view to the asm.js heap) as the buffer it passes to there,

it happens that the asm.js/wasm heap will be of SharedArrayBuffer type, so calls to the Web Audio API functions will also receive views to SAB as a result, which like mentioned in comment 0, is currently rejected.

The ways to work around this would be to either a) create a temporary unshared Float32Array to pass to Web Audio, and then copy that element by element to the shared Float32Array, or b) augment Web Audio to allow passing in Float32Arrays that view a SharedArrayBuffer in the API signatures.

The scenario a) has the annoyance of requiring a big temp memory allocation and an element-by-element copy loop in JS side, so if there's no conflicts otherwise, going for b) would be good.

Comment 9

[Lars T Hansen [:lth]]

(In reply to Lars T Hansen [:lth] from comment #3)
> 
> I'm concerned about the comment "SharedArrayBuffer assertions fail in
> Nightly" above so I will investigate that.  (An error is expected to be
> signaled in JS code but not an assertion in Firefox.)

Nothing actionable in regard to that narrow point, the assertion in question is in the test case, and it fails as expected.  A warning is printed in the terminal, 

"WARNING: Audio Buffer is not full by the end of the callback.: 'Available() == 0 || mSampleWriteOffset == 0', file /Users/lhansen/moz/mozilla-inbound/dom/media/AudioBufferUtils.h, line 88"

but I'm not sure if that is indicative of anything other than the shared memory not being readable by the audio layer, as it should not be (by the current design).

Comment 10

[Bulk Bug Changes for mreavy's org]

Mass change P2->P3 to align with new Mozilla triage process.

Comment 11

[Anne (:annevk)]

Paul, I think this can be closed, right? Audio now uses postMessage() infrastructure. If there are remaining methods that need this they can use [AllowShared] in IDL, though we have to be careful that such a thing doesn't end up exposing a high-resolution timer on its own.