Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Should copyToChannel accept a shared Float32Array? #2565

Open
kettle11 opened this issue Jan 9, 2024 · 2 comments
Open

Should copyToChannel accept a shared Float32Array? #2565

kettle11 opened this issue Jan 9, 2024 · 2 comments
Labels
Needs Discussion The issue needs more discussion before it can be fixed.

Comments

@kettle11
Copy link

kettle11 commented Jan 9, 2024

Presently calling AudioBuffer: copyToChannel(source, channelNumber) with a source backed by a SharedArrayBuffer is disallowed. In Chrome you get the following crash: Failed to execute 'copyToChannel' on 'AudioBuffer': The provided Float32Array value must not be shared.

This is a footgun when working with WebAssembly. Creating a Float32Array backed by the Wasm memory and passing that in to copyToChannel works fine unless the Wasm memory is backed by a SharedArrayBuffer.

This has resulted in bugs like this one, which impacts a popular audio library from the Rust ecosystem: RustAudio/cpal#656

Should calls like copyToChannel be allowed to accept shared Float32Arrays or is it necessary to forbid them?

@padenot
Copy link
Member

padenot commented Jan 10, 2024

That sounds like a reasonable addition.

That said, cpal's code is really inefficient and should use SharedArrayBuffer and a ring buffer, as explained in https://blog.paul.cx/post/a-wait-free-spsc-ringbuffer-for-the-web/. https://crates.io/crates/ringbuf/ is a nice one in rust.

@hoch hoch added the Needs Discussion The issue needs more discussion before it can be fixed. label Jan 25, 2024
@hoch
Copy link
Member

hoch commented Feb 8, 2024

From teleconference:

  1. Copying the data into an active buffer is not allowed according to "acquire content" process.
  2. Chromium doesn't have a complete implementation of this feature yet. (WPT)

Next steps:

  1. Figure out if this is possible on each browser implementation. If this is not really feasible, we need more discussion.

@hoch hoch added Needs Edits Decision has been made, the issue can be fixed. https://speced.github.io/spec-maintenance/about/ Needs Discussion The issue needs more discussion before it can be fixed. and removed Needs Discussion The issue needs more discussion before it can be fixed. Needs Edits Decision has been made, the issue can be fixed. https://speced.github.io/spec-maintenance/about/ labels Jul 24, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Needs Discussion The issue needs more discussion before it can be fixed.
Projects
None yet
Development

No branches or pull requests

3 participants