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

Allow sharing AudioBufferSourceNode buffer #71

Closed
wants to merge 1 commit into from

Conversation

@ferjm
Copy link
Member

ferjm commented Jul 5, 2018

@ferjm ferjm requested a review from Manishearth Jul 5, 2018
@ferjm
Copy link
Member Author

ferjm commented Jul 5, 2018

I am not sure if locking here is preferred over copying (in general, what I hear for real-time audio is that while copying may take longer, being deterministic makes it preferable over locking), but since we talked about sharing the buffer, I'm proposing this patch.

The key parts of the DOM bindings where we are using this are here and here

@Manishearth
Copy link
Member

Manishearth commented Jul 5, 2018

I don't think we should be locking here; for one there will be a process boundary here which makes locks hard to do, and for the other this means that not only can audio code block DOM (it can already), DOM code can now block audio. The way I thought we could share it would be to make it possible to send messages to the thread asking for buffer changes and asking for the buffer contents. But this has its own problems.

I suspect the realtime requirements for uploading buffers are more relaxed, though, so maybe just copying is the best.

Feel free to merge -- I don't have an objection to this PR; just not sure if it's the best model.

@ferjm
Copy link
Member Author

ferjm commented Jul 6, 2018

We'll go with the copy approach.

@ferjm ferjm closed this Jul 6, 2018
@ferjm ferjm deleted the ferjm:shareable.audiobuffer branch Jul 6, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

2 participants
You can’t perform that action at this time.