STR: - Run the demo <http://greweb.me/zpeech/> - Wait a bit (or trigger CC in about:memory) Expected: - We keep seeing the mic input spectrum moving in the canvas Actual: - Everything stop. The "camera icon" indicating gUM activity disappear. According to the attached stack, and mccr8, we should be addref-ing something, somewhere.

Comment on attachment 826796 [details] Stack for the MSG thread, that shows the unlink The documentation for DOMMediaStream says "Add an nsISupports object that this stream will keep alive as long as the stream is not finished", but the object is added to the traversal, so it stays alive only while something not in the same cycle keeps the DOMMediaStream alive. http://hg.mozilla.org/mozilla-central/rev/11b2cd90a51a#l2.34 #0 mozilla::DOMMediaStream::cycleCollection::Unlink ( this=0x7f50223d86b0 <mozilla::DOMMediaStream::_cycleCollectorGlobal>, p=0xed0560) at /home/karl/moz/dev/content/media/DOMMediaStream.cpp:30 #1 0x00007f501f586310 in nsCycleCollector::CollectWhite (this=0x907b30) at /home/karl/moz/dev/xpcom/base/nsCycleCollector.cpp:2365 #2 0x00007f501f586d91 in nsCycleCollector::Collect (this=0x907b30, aCCType=ScheduledCC, aWhiteNodes=0x7fffca5d55f0, aResults=0x7fffca5dd480, aManualListener=0x0) at /home/karl/moz/dev/xpcom/base/nsCycleCollector.cpp:2688 #3 0x00007f501f5883c0 in nsCycleCollector_collect (aManuallyTriggered=false, aResults=0x7fffca5dd480, aManualListener=0x0) at /home/karl/moz/dev/xpcom/base/nsCycleCollector.cpp:3159 #4 0x00007f501dfef75e in nsJSContext::CycleCollectNow (aListener=0x0, aExtraForgetSkippableCalls=0, aManuallyTriggered=false) at /home/karl/moz/dev/dom/base/nsJSEnvironment.cpp:2048 I don't know what is meant to keep the DOMMediaStream alive.

If the DOMMediaStream has a SourceMediaStream, then the references should keep the MediaStreamAudioSourceNode alive until the stream is finished or graph is shutdown, and so they should not be traversed. If it has a ProcessedMediaStream, then the stream won't finish and MediaStreamAudioSourceNode should not be kept alive unless the DOMMediaStream has an external reference. I think we can make whether mConsumersToKeepAlive are traversed depend on the type of stream.

The changes to the test don't test a SourceMediaStream, but they do confirm that AddConsumerToKeepAlive() is already effective for a ProcessedMediaStream from Audio::mozCaptureStreamUntilEnded(). However, the change for SourceMediaStream is ineffective here because nsDOMUserMediaStream hides the SourceMediaStream behind a track union ProcessedMediaStream.

Bump. Any progress on this? In the meantime, the following workaround fixes the issue in my testing: context = new AudioContext(); navigator.getUserMedia({ audio: true }, function(stream) { // the important thing is to save a reference to the MediaStreamAudioSourceNode // thus, *window*.source or any other object reference will do window.source = context.createMediaStreamSource(stream); source.connect(context.destination); }, alert);

Karl, is the work for this happening elsewhere? If not, how important is it?

Bug 897796 is for providing a framework to fix with without introducing a leak. That is a sizeable chunk of work. Perhaps doing a leaking solution first is an option. I don't know exactly what would be involved to do that. It would be less work than fixing bug 897796, but perhaps dead work, because I suspect it would still need to be re-done properly once bug 897796 is fixed. I consider this is a bad bug, and many have noticed it, as can be seen by the number of duplicates. There is a non-intuitive workaround, but the bug means that many getUserMedia() demos developed for / tested in other browsers fail to work on Gecko.

Just hit this when writing an example for MediaRecorder. Definitely a very easy thing to hit, and very puzzling as well: http://mozdevs.github.io/mediarecorder/mediadevices-to-mediastreamaudiosourcenode.html

This is a really bad bug, and there's zero indication of what's going on to the developer. If we cannot fix this bug (two years old!), then we should at least scream really loud on the console about it until we can. I added event listeners on the MediaStream and *none* of them fire - onended, oninactive, onerror, onremovechannel. There's just *no* way to know what failed. I spent so long assuming this was something *I* did wrong. Adding dev-doc-needed so that we can add huge warnings on MDN about this, if we haven't already.

Maire, can we bump priority of this?

MDN folks, a quick review of these pages found no references to this bug. We should probably note it on all of them. https://developer.mozilla.org/en-US/docs/Web/API/MediaDevices/getUserMedia https://developer.mozilla.org/en-US/docs/Web/API/Navigator/getUserMedia https://developer.mozilla.org/en-US/docs/Web/API/AudioContext/createMediaStreamSource https://developer.mozilla.org/en-US/docs/Web/API/MediaStreamAudioSourceNode https://developer.mozilla.org/en-US/docs/Web/API/Web_Audio_API/Using_Web_Audio_API Interestingly, the example on that last URL *doesn't* exhibit the problem, even though it's not using the workaround in comment #5. https://github.com/mdn/voice-change-o-matic/blob/gh-pages/scripts/app.js#L102 I haven't narrowed down what exactly it's doing that keeps the object alive.

I've bumped the priority to a P1 (Rank:15). I think we want to focus on finding a shorter term solution to ease the pain (assuming that's possible) and file another bug for a longer term solution. I'm particularly concerned about an app relying on GC for a MediaStream to shutdown. So let me call in Randell and Karl... Karl, Randell -- You two know the most about the DOM side of MSG. What makes sense to ease the pain in the short term? Are there any good answers? And (more importantly) are we going down the right path for a solution? Karl -- I know you're the current owner of this bug. Are you likely to have time to work on this soon? If not, I may need to find someone else. Thanks!

My concern is that if we do the "leaking" solution, that the capture won't shut down until you navigate away - the user will see the recording indicator constantly. Conversely, if we solve this per the spec, and hold it alive until someone disconnects or destroys the audiocontext, the app is still relying on GC to stop the media capture, which may be immediate - or may be a long time. In general, users of getUserMedia streams have had to learn to hold references and call stream.stop() to ensure capture ends immediately. Chrome has been side-stepping this by ending capture when a stream is not connected somehow to a media element -- but this has major privacy concerns, as it lets a site 'hide' that they can restart capture at any time, perhaps faster than you can notice the indicator. So we can make it work as the spec indicates, and should -- but applications relying on that behavior likely have a bug (perhaps merely latent, like on a site which has no plan to ever shut down capture after it's started -- depending on the user to navigate away or hit reload). If we can find *some* way to detect this behavior we can warn the dev in the console (even perhaps after we make it spec-compliant).

What about the lack of events being emitted? Is that a separate bug?

(In reply to Dietrich Ayala (:dietrich) from comment #21) > What about the lack of events being emitted? Is that a separate bug? Which events are you expecting and not getting? Almost certainly this is a separate bug/bugs, though

See comment #16. It seems related to this particular issue, possibly a side-effect of it.

> I added event listeners on the MediaStream and *none* of them fire - > onended, oninactive, onerror, onremovechannel. The only events currently defined on MediaStream are onaddtrack and onremovetrack. MediaStreamTracks supports ended (and mute/unmute and overconstrained). We do fire ended on stop() for a getUserMedia MediaStreamTrack (or when a remote track from PeerConnection ends). The spec says nothing about firing it when a Track is gc'd (and I'm not certain how that would work anyways). oninactive on a MediaStream may have been in an earlier spec, or perhaps it's part of ORTC (Edge apparently supports it). I'm not sure what "onremovechannel" is from; I wouldn't expect onremovetrack to fire here either, note - that only fires when a track is removed from the stream. Here's the current spec draft: http://w3c.github.io/mediacapture-main/getusermedia.html

Sorry, I meant onremovetrack. I was working from https://developer.mozilla.org/en-US/docs/Web/API/MediaStream#Event_handlers.

(In reply to Dietrich Ayala (:dietrich) from comment #25) > Sorry, I meant onremovetrack. > > I was working from > https://developer.mozilla.org/en-US/docs/Web/API/MediaStream#Event_handlers. active/inactive were removed recently, and we never implemented them. (I think we knew they would be removed for quite a while.) ended I believe we also never implemented on the stream; and it's now moved to the tracks. Right now I wouldn't envision getting any events for the stream in this case. Even for the tracks, I think there would be no events per the spec (ended comes close, but the spec language wouldn't cover it I think).

Chris, see comment #26 for developer documentation changes. We've documented these events on MDN, but they've either been removed, never supported, or changed from stream to tracks.

(In reply to Dietrich Ayala (:dietrich) from comment #27) > Chris, see comment #26 for developer documentation changes. We've documented > these events on MDN, but they've either been removed, never supported, or > changed from stream to tracks. Ok, confirmed that these handlers (onactive/oninactive/onended) have been removed from the spec; since they were never implemented (at least by us), I've removed the page stubs.

I will work on adding the warnings needed to MDN. I need to review the bug here a bit to be sure I get it right, but I'll get it done this afternoon.

Thanks Sheppy and Chris :D Randell, do you know why this bug does not occur with audio+video or video-only streams?

Hard to say without code, but if you separately are putting the stream into a <video> element, that keeps it alive. If this is a video+audio capture fed into createMediaStreamSource (and no other refs), it might have to do with the cycle-handling boilerplate/code, but that's all speculation until someone gets up to their elbows in it. Can you add a simple testcase since it's clear you've tried it? or find me on IRC (#media). Thanks!

I ran across this now and took a deep dive. It's not harder than that the only things keeping a MediaStreamAudioSourceNode alive are the DOMMediaStream used as input to said node, and an external reference to the node directly. The AudioContext keeps nodes alive as long as they're active (internal notion), but MediaStreamAudioSourceNode never gets active. It's probably because we cannot easily say when it stops being active, so we handed the lifetime decision to the DOMMediaStream instead. I think we can make a reasonable short-term fix by marking the MediaStreamAudioSourceNode active as long as there is a live AudioStreamTrack in the input stream. It should really be active forever, and disconnected when it cannot be connected to an output anymore (bug 897796), and the references to it are gone. Tracks may be added to a MediaStream at any time, so we shouldn't mark it inactive just because there are no live tracks. Even if all references to a MediaStream are gone, a producer like HTMLMediaElement::CaptureStream or a receiving RTCPeerConnection may dynamically create new tracks that will appear in the MediaStream, so can also not just say that all MediaStreams go inactive after their references are gone and there are no live audio tracks.

Review commit: https://reviewboard.mozilla.org/r/61268/diff/#index_header See other reviews: https://reviewboard.mozilla.org/r/61268/

Review commit: https://reviewboard.mozilla.org/r/61270/diff/#index_header See other reviews: https://reviewboard.mozilla.org/r/61270/

Karl, Paul wanted me to switch review over to you. There's a bit of background/discussion on IRC at [1] that you might want to look at. [1] http://logs.glob.uno/?c=mozilla%23media&s=29%20Jun%202016&e=29%20Jun%202016#c268725

Comment on attachment 8766290 [details] Bug 934512 - Test that MediaStreamAudioSourceNode does not get GCed while it has live audio tracks. https://reviewboard.mozilla.org/r/61268/#review59020

Comment on attachment 8766291 [details] Bug 934512 - Mark MediaStream source node as active while it has live audio tracks. https://reviewboard.mozilla.org/r/61270/#review59022

Try push: https://treeherder.mozilla.org/#/jobs?repo=try&revision=e10ddbd28c4d

Pushed by pehrsons@gmail.com: https://hg.mozilla.org/integration/mozilla-inbound/rev/88377287b2a5 Test that MediaStreamAudioSourceNode does not get GCed while it has live audio tracks. r=padenot https://hg.mozilla.org/integration/mozilla-inbound/rev/750df2678990 Mark MediaStream source node as active while it has live audio tracks. r=padenot

https://hg.mozilla.org/mozilla-central/rev/88377287b2a5 https://hg.mozilla.org/mozilla-central/rev/750df2678990

Paul -- Andreas is on PTO for one more week. As the reviewer, can you tell me if it makes sense to ask for uplift on the fix for this into Fx 49 (Dev Edition)? The actual fix is small.

Developers have learned to work around this, we can let it ride.

Documentation had already been partially updated, but I've finished things out: Everything that's part of MediaStream seems to have been brought up to date on the main page here: https://developer.mozilla.org/en-US/docs/Web/API/MediaStream -- I also removed some obsolete or possibly once Chrome-specific (but no longer true) information about exceptions thrown by addTrack()/removeTrack(). Added new pages: https://developer.mozilla.org/en-US/docs/Web/API/MediaStream/onremovetrack https://developer.mozilla.org/en-US/docs/Web/API/MediaStream/ended (labeled as obsolete but present for completeness and to guide in updating older code) And I also made a few other corrections and tweaks here and there. I may also get removeTrack() documented (it isn't currently), but that's not part of this bug, so I'm considering this done.