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

Support default arguments for the Web Audio backend #16

Merged
merged 1 commit into from
Jun 27, 2012

Conversation

ehsan
Copy link
Contributor

@ehsan ehsan commented Jun 26, 2012

This patch fixes a DOM error that is generated when the default Sink() constructor is called, which is caused by passing undefined values to createJavaScriptNode. Basically we need to first call the start function, and then read the requested values from the self object.

@jussi-kalliokoski
Copy link
Owner

Thank you Ehsan!

However, I'm a bit hesitant to pull this in, as the Web Audio API is in a very unstable state right now. More details here, but the gist is that the createJavaScriptNode's argument ordering may change, or the buffer size argument may be dropped, both of which would lead to a horrible sonic experience and possibly very high CPU usage. Then again, now it looks very likely that support for audio processing in the main thread will not be supported at all (Mozilla and Opera representatives are very strongly against main thread audio processing being possible, while I've been trying to defend it with cases such as emscripten in my mind... Maybe it would be useful if you could voice your opinion on the matter?).

Anyway, I wanted to wait until this goes to stable, but it doesn't look like it's happening any time soon, so I think I'm going to merge now.

jussi-kalliokoski added a commit that referenced this pull request Jun 27, 2012
Support default arguments for the Web Audio backend
@jussi-kalliokoski jussi-kalliokoski merged commit f71badb into jussi-kalliokoski:master Jun 27, 2012
@jussi-kalliokoski
Copy link
Owner

Oh, and for the record, here's a link to the discussion/issue on the Audio WG: https://www.w3.org/Bugs/Public/show_bug.cgi?id=17415

@ehsan
Copy link
Contributor Author

ehsan commented Jun 27, 2012

Thanks for pulling this! FWIW, this patch was about not passing undefined to the API. Even if the ordering of the parameters change, I don't think passing undefined would work. ;-)

I read the whatwg bug, and I think I agree with roc. We (browser vendors and developers) are still being bitten by synchronous XHR. There are just too many web developers who are not familiar with what blocking the main thread means for their application, and I personally agree that doing any audio processing on the main thread is a recipe for failure. Roc's suggestion of providing canvas access to workers is something that has been discussed with game developers for quite some time, and they seem to like it. And that would be awesome for things like Emscripten too, since you can just execute all of your code on the worker, and the UI will not be blocked, ever!

If you have specific concerns not covered in that discussion or the replies, please let me know. :-)

@jussi-kalliokoski
Copy link
Owner

Thanks for pulling this! FWIW, this patch was about not passing undefined to the API. Even if the ordering of the parameters change, I don't think passing undefined would work. ;-)

No, thank you for pushing. ;) Yes, I'm well aware, but if it were still passing undefined as arguments, and the argument ordering changed, it would just not work, instead of producing terrible results. Say the buffer size became the last argument, as it is now, after the changes, it would try to create a JSNode with a buffer size of two, and 4096 input channels, which would result in a highly unstable system that would make a lot of glitchy sounds and possibly even freeze the browser.

Yes, I agree that there is no need for main script audio access if workers will have graphics access as well. That's why I'm very sad to see Roc's MediaStreams Processing API abandoned by the group, as it would have catered both of these needs, as well as other use cases...

@charlieroberts
Copy link

The callback function passed to Sink does not get called in OS X under Chrome Version 20.0.1132.47 (which now the stable release version). I don't get any errors, but no audio is generated using the tests in the repo; inserting some log statements show the callback function is not triggered.

Has anybody had success with this in the current stable copy of Chrome or Chromium?

@malkomalko
Copy link

I can confirm the same problem in the same latest build of chrome charlie

@MiracleBlue
Copy link

I can also confirm. Tested just now :( Argh.

@jussi-kalliokoski
Copy link
Owner

Hmm, the onaudioprocess event never gets fired for some reason, I hope they haven't yet disabled main thread audio processing. I'll try to read if there are some changes in the spec regarding this and then the webkit changelog.

@jussi-kalliokoski
Copy link
Owner

Fixed per b5b15e6d2defb52d6e9153b6e086df7182a86639. I forgot that the new syntax also requires you to specify input channels even when there are none...

@charlieroberts
Copy link

That fixes it for me. Thank you for debugging that... an interesting syntax choice for sure :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants