Do we have a cross platform way of getting the number of channels that the audio hardware that cubeb talks to supports?

We don't. I can make you one, I guess.

(In reply to comment #1) > We don't. I can make you one, I guess. That would be nice! :-)

This adds a new function to all the backends, to get the maximum number of channels that the current hardware/software configuration supports.

Comment on attachment 755295 [details] [diff] [review] add a function to cubeb to get the max number of channels Alex, could you have a look at the sndio part? I haven't got around to setup a BSD vm to test.

I implemented my proposal [1], instead of the current spec, since there was agreement from Chris Rogers. [1] http://lists.w3.org/Archives/Public/public-audio/2013AprJun/0437.html

Comment on attachment 755296 [details] [diff] [review] Expose the new cubeb function in the AudioStream as a static method. Review of attachment 755296 [details] [diff] [review]: ----------------------------------------------------------------- Why are you using int here? We're going to need the unsigned value when we use this, so the conversion from unsigned to signed and then back to unsigned seems unnecessary.

Comment on attachment 755298 [details] [diff] [review] Implement the maxChannelCount attribute on top of the other patches. Review of attachment 755298 [details] [diff] [review]: ----------------------------------------------------------------- ::: content/media/webaudio/AudioDestinationNode.cpp @@ +196,5 @@ > > +unsigned long > +AudioDestinationNode::MaxChannelCount() const > +{ > + return AudioStream::MaxNumberOfChannels(); This does the wrong thing for offline destination nodes. ::: content/media/webaudio/AudioDestinationNode.h @@ +34,5 @@ > { > return 0; > } > > + unsigned long MaxChannelCount() const; uint32_t. ::: dom/webidl/AudioDestinationNode.webidl @@ +17,1 @@ > //attribute unsigned long numberOfChannels; While you're here, can you please remove numberOfChannels? It has long been removed from the spec, and I forgot to remove it here.

Comment on attachment 755299 [details] [diff] [review] Add a test for the maxChannelCount attribute. Review of attachment 755299 [details] [diff] [review]: ----------------------------------------------------------------- ::: content/media/webaudio/test/Makefile.in @@ +27,5 @@ > test_analyserNode.html \ > test_AudioBuffer.html \ > test_AudioContext.html \ > test_AudioListener.html \ > + test_AudioDestinationNode.html \ Nit: I'd call this test_maxChannelCount.html... ::: content/media/webaudio/test/test_AudioDestinationNode.html @@ +12,5 @@ > +SimpleTest.waitForExplicitFinish(); > +addLoadEvent(function() { > + SpecialPowers.setBoolPref("media.webaudio.enabled", true); > + var ac = new AudioContext(); > + ok(ac.destination.maxChannelCount != undefined, "We can query the maximum number of channels"); Hmm, I'm not sure what you wanted to do here, but in JS, comparing anything with undefined returns false, AFAIK, so this check always returns true, no matter what. :-) I think a better way to test this is to test whether maxChannelCount > 0. Also, you need to test with OfflineAudioContext as well. You can construct a couple of offline contexts with a different channel count and make sure that the correct number is reflected in maxChannelCount.

Comment on attachment 755295 [details] [diff] [review] add a function to cubeb to get the max number of channels Review of attachment 755295 [details] [diff] [review]: ----------------------------------------------------------------- ::: media/libcubeb/src/cubeb_pulse.c @@ +111,3 @@ > ctx->error = 1; > + } else if (state == PA_CONTEXT_READY){ > + WRAP(pa_context_get_server_info)(c, server_info_callback, ctx); Can we call this from cubeb_init after the context is ready, rather than here?

Hi, By default, sndio doesn't limit the number of channels. It routes stream channels that wouldn't be present on the hardware to existing ones (ex. a 8-channel file can be played on a 4-speaker set, and all channels of 8 channels will sound on the "appropriate" speakers). Similarly a stereo file will sound on all speakers of a 4-speaker set (each channel is routed to a pair of speakers). The "routing" is somewhat user-configurable; it's global, allowing the user to change the routing for all programs. If this is OK for cubeb, I'd suggest something like: static int sndio_get_max_channel_count(cubeb * ctx, uint32_t * max_channels) { assert(ctx && max_channels); *max_channels = 8; return CUBEB_OK; } FWIW, by default, the channels count returned by sio_getcap() doesn't correspond to number of channels present on the hardware; it's a hard-coded value similar to the above.

I found that the pulseaudio implementation was a bit racy: if one called the function right after the context init, and because the API to get the sink info is async, it is possible that `sink_info_callback` was not called yet. In pratice, this happended and the test went orange. This patch (to apply on top of the other) fixes it by waiting until we have the info to return. Theoretically, this can block the main thread for a short (1 or 2 ms) period of time the first time it is called.

(removed unwanted changes in the test).

This fixes the issues, and limits the number of channels to 32 for the OfflineAudioContext. From what I see in other bugs, you might already have the part that throws if the channel count is invalid.

forgot to qref.

Comment on attachment 755927 [details] [diff] [review] Test for AudioContext.destination.maxChannelCount. r= Review of attachment 755927 [details] [diff] [review]: ----------------------------------------------------------------- ::: content/media/webaudio/test/test_maxChannelCount.html @@ +16,5 @@ > + var ac = new AudioContext(); > + ok(ac.destination.maxChannelCount > 0, "We can query the maximum number of channels"); > + > + var oac = new OfflineAudioContext(2, 1024, 48000); > + ok(oac.destination.maxChannelCount, 32, "OfflineAudioContext has 32 max channels."); As discussed on IRC, this should be 2. Also please add another similar test which passes another value for the number of channels to the OfflineAudioContext ctor, such as 6 or whatever.

Comment on attachment 755925 [details] [diff] [review] Implement AudioContext.destination.maxChannelCount. r= Review of attachment 755925 [details] [diff] [review]: ----------------------------------------------------------------- ::: content/media/webaudio/AudioContext.cpp @@ +115,5 @@ > > + if (!aNumberOfChannels || aNumberOfChannels > WebAudioUtils::MaxChannelCount) { > + aRv.Throw(NS_ERROR_DOM_SYNTAX_ERR); > + return nullptr; > + } Bug 877231 which is on inbound now should make this unnecessary. @@ +386,5 @@ > > +uint32_t > +AudioContext::MaxChannelCount() > +{ > + return mIsOffline ? WebAudioUtils::MaxChannelCount : AudioStream::MaxNumberOfChannels(); As discussed on irc, for offline audio contexts, we need to return the number of channels passed to the ctor. Currently we only "keep track" of that value in the length of OfflineDestinationNodeEngine::mInputChannels, but it's not kosher to touch the engine's guts from the main thread, so you can just add a new member to AudioContext I guess... ::: content/media/webaudio/AudioContext.h @@ +195,5 @@ > void UnregisterPannerNode(PannerNode* aNode); > void UnregisterScriptProcessorNode(ScriptProcessorNode* aNode); > void UpdatePannerSource(); > > + uint32_t MaxChannelCount(); Nit: Make this const please. ::: content/media/webaudio/AudioDestinationNode.h @@ +34,5 @@ > { > return 0; > } > > + uint32_t MaxChannelCount() const; You should also make sure that setting destination.channelCount to something greater than destination.maxChannelCount throws, and test that in part 4.

Added another OfflineAudioContext with 6 channels, plus check that an exception is thrown when setting a channelCount higher than the initial channel count on an OfflineAudioContext. Carrying forward r+.

Comment on attachment 756046 [details] [diff] [review] Implement AudioContext.destination.maxChannelCount. r= Review of attachment 756046 [details] [diff] [review]: ----------------------------------------------------------------- Nice!

Mass moving Web Audio bugs to the Web Audio component. Filter on duckityduck.

https://hg.mozilla.org/integration/mozilla-inbound/rev/8340c7da287d https://hg.mozilla.org/integration/mozilla-inbound/rev/116e84abedca https://hg.mozilla.org/integration/mozilla-inbound/rev/f223d663d691 https://hg.mozilla.org/integration/mozilla-inbound/rev/3443f1733c22 Updated https://wiki.mozilla.org/WebAudio_API_Rollout_Status.

Fix bustage on android: https://hg.mozilla.org/integration/mozilla-inbound/rev/efa5667ea807

Fix another bustage on another android: https://hg.mozilla.org/integration/mozilla-inbound/rev/6053305ba2b3

https://hg.mozilla.org/mozilla-central/rev/8340c7da287d https://hg.mozilla.org/mozilla-central/rev/116e84abedca https://hg.mozilla.org/mozilla-central/rev/f223d663d691 https://hg.mozilla.org/mozilla-central/rev/3443f1733c22 https://hg.mozilla.org/mozilla-central/rev/efa5667ea807 https://hg.mozilla.org/mozilla-central/rev/d7ec56f8eace https://hg.mozilla.org/mozilla-central/rev/6053305ba2b3