Bug 759964 added an allowMedia flag to the docshell to disable <video>/<audio>. We can use this same flag for WebAudio. (Quoting Robert O'Callahan (:roc) (Mozilla Corporation) from comment #9) > You probably want to disable WebAudio based on this flag as well. Add some > check to AudioContext...

This seems to work... I don't know how to write a test for it.

Comment on attachment 767042 [details] [diff] [review] patch? Review of attachment 767042 [details] [diff] [review]: ----------------------------------------------------------------- Testing this will be very hard if not impossible given the way that this patch works... ::: content/media/webaudio/AudioContext.cpp @@ +58,5 @@ > // Actually play audio > + nsCOMPtr<nsIDocShell> docShell = do_GetInterface(aWindow); > + if (!docShell || docShell->GetAllowMedia()) { > + mDestination->Stream()->AddAudioOutput(&gWebAudioOutputKey); > + } What if the docshell's state changes later on? Also, offline audio contexts should not be affected by this as they never play back anything.

So we talked about this on IRC, and decided that muting the audio probably makes more sense. Here's how you can implement that. In Web Audio, every audio node has an associated engine, which takes an AudioChunk as input and produces one as output. The easiest way to mute all audio output for a given AudioContext is to set the mVolume member of the output AudioChunk to 0 (mVolume is a multiplier that will get applied to every audio sample upon playback.) The engine for a given node usually inherits from a base class called AudioNodeEngine, which just passes its input through its output. For AudioDestinationNode, we already have an engine for the offline case (which, you don't need to worry about here, as I said in comment 2.) You should derive a new engine class from AudioNodeEngine, maintain a float mVolume member in it, and then in its ProduceAudioBlock, do something like: *aOutput = aInput; aOutput->mVolume *= mVolume; Then, you should add an AudioContext::Mute function which calls the AudioDestinationNode::Mute function which you should also add, and in that function use AudioNode::SendDoubleParameterToStream to send 0 to the engine class. Then inside the engine, you can initialize mVolume to 1.0, override AudioNodeEngine::SetDoubleParameter, and receive the volume parameter and put it in mVolume. This way, when you call Mute(), you end up sending the 0 value to the engine, which will be used as mVolume as soon as it's received, which guarantees that the engine will produce silence from that point on. With this setup, implementing an Unmute function will be really simple, all you have to do is to use AudioNode::SendDoubleParameterToStream to send 1 to the engine so reset mVolume to 1.0. Now, once you have all of that in place, see nsGlobalWindow. There we have a list of all AudioContexts associated with the current window. You need to iterate over them all and call Mute() on each one of them when the allowMedia flag is set. It's *as simple* as that! :-)

Thanks, Ehsan. So I'm working on this and I notice there are AudioContext::Suspend and Resume, and that nsGlobalWindow calls Suspend when it suspends timeouts, and Suspend calls MediaStream::ChangeExplicitBlockerCount, whose comment [1] is: > // Explicitly block. Useful for example if a media element is pausing > // and we need to stop its stream emitting its buffered data. Sounds useful! I looked some more at mExplicitBlockerCount and TimeVarying, and it looks like blocking counts can be set at certain times and apply thereafter, and if the count is > 0 the stream is "blocked," but I don't quite understand. Ehsan, can we not use that here? I'm sure you would have said so if so, but I thought I'd make sure. [1] http://mxr.mozilla.org/mozilla-central/source/content/media/MediaStreamGraph.h#316

I'll attach a test separately once I figure out how to write it.

Ehsan, I'm having a hard time inspecting the destination node, since its max number of outputs is 0, so it can't connect to anything. I was hoping to verify that all the samples (frames?) in its buffer are zero when the context is muted. Any ideas?

(In reply to Drew Willcoxon :adw from comment #4) > Thanks, Ehsan. > > So I'm working on this and I notice there are AudioContext::Suspend and > Resume, and that nsGlobalWindow calls Suspend when it suspends timeouts, and > Suspend calls MediaStream::ChangeExplicitBlockerCount, whose comment [1] is: > > > // Explicitly block. Useful for example if a media element is pausing > > // and we need to stop its stream emitting its buffered data. > > Sounds useful! I looked some more at mExplicitBlockerCount and TimeVarying, > and it looks like blocking counts can be set at certain times and apply > thereafter, and if the count is > 0 the stream is "blocked," but I don't > quite understand. > > Ehsan, can we not use that here? I'm sure you would have said so if so, but > I thought I'd make sure. Suspend() pauses the audio, so it's not useful here.

(In reply to Drew Willcoxon :adw from comment #6) > Ehsan, I'm having a hard time inspecting the destination node, since its max > number of outputs is 0, so it can't connect to anything. I was hoping to > verify that all the samples (frames?) in its buffer are zero when the > context is muted. Any ideas? What gets to the destination node is invisible from the point of view of web content. I don't think there's any good way to automatically test this... :(

Comment on attachment 769868 [details] [diff] [review] patch Review of attachment 769868 [details] [diff] [review]: ----------------------------------------------------------------- ::: content/media/webaudio/AudioContext.cpp @@ +532,5 @@ > > +void > +AudioContext::Mute() const > +{ > + mDestination->Mute(); Please MOZ_ASSERT(!mIsOffline) here and below. ::: content/media/webaudio/AudioDestinationNode.cpp @@ +182,5 @@ > +{ > +public: > + explicit DestinationNodeEngine(AudioDestinationNode* aNode) > + : AudioNodeEngine(aNode) > + , mVolume(1) Nit: 1.0f. @@ +191,5 @@ > + const AudioChunk& aInput, > + AudioChunk* aOutput, > + bool* aFinished) MOZ_OVERRIDE > + { > + AudioNodeEngine::ProduceAudioBlock(aStream, aInput, aOutput, aFinished); Hmm, please just do |*aOutput = aInput;| here, it would be much easier to read. @@ +201,5 @@ > + if (aIndex == VolumeParameter) { > + mVolume = aParam; > + return; > + } > + AudioNodeEngine::SetDoubleParameter(aIndex, aParam); You don't need to call the base class function here. @@ +205,5 @@ > + AudioNodeEngine::SetDoubleParameter(aIndex, aParam); > + } > + > + enum { > + VolumeParameter, Nit: please use the Parameters convention we have in other engines here, such as DelayNodeEngine, for example. @@ +256,5 @@ > > void > +AudioDestinationNode::Mute() > +{ > + SendDoubleParameterToStream(DestinationNodeEngine::VolumeParameter, 0); Please MOZ_ASSERT that we're not on an offline context.

Also, please backport this to Aurora after it lands on central. Thanks!

(In reply to :Ehsan Akhgari (needinfo? me!) from comment #10) > Also, please backport this to Aurora after it lands on central. Thanks! Why, out of curiosity?

(In reply to comment #11) > (In reply to :Ehsan Akhgari (needinfo? me!) from comment #10) > > Also, please backport this to Aurora after it lands on central. Thanks! > > Why, out of curiosity? Because we are planning to ship Web Audio in 24.

Thanks for your help, Ehsan! https://tbpl.mozilla.org/?tree=Try&rev=266b926ba639 https://hg.mozilla.org/integration/mozilla-inbound/rev/c9978562db88

Comment on attachment 769868 [details] [diff] [review] patch [Approval Request Comment] Ehsan wants this on Aurora because we are planning to ship Web Audio in 24 (see comment 12). Bug caused by (feature/regressing bug #): bug 759964 User impact if declined: minimal Testing completed (on m-c, etc.): no automated testing Risk to taking this patch (and alternatives if risky): low String or IDL/UUID changes made by this patch: none

Comment on attachment 769868 [details] [diff] [review] patch Low risk enough and we are still in a phase to take new feature enhancements in Aurora.Approving the patch. Ehsan, given there are no automated tests please give us some feeedback on how best this can be tested.If there are any testcases qa can help with, please add qawanted keyword here.

Drew, is there a testcase QA can use to verify this is fixed?

This is similar to the page I was testing with. You can press the Play button, and after a tone starts playing, run this in the browser console (Shift+Ctrl+J -- not the web console): gBrowser.selectedBrowser.docShell.allowMedia = false; The tone should then stop. Make sure your volume is really down!

Thank you Drew.

Verified as fixed on Firefox 25 beta 1 - I used the instructions provided in Comment 19. Verified on Windows 7, Ubuntu 13.04 and Mac OS X 10.8: Mozilla/5.0 (Windows NT 6.1; WOW64; rv:25.0) Gecko/20100101 Firefox/25.0 Mozilla/5.0 (X11; Linux i686; rv:25.0) Gecko/20100101 Firefox/25.0 Mozilla/5.0 (Macintosh; Intel Mac OS X 10.8; rv:25.0) Gecko/20100101 Firefox/25.0 Build ID: 20130917123208