759908 - MediaStreams need Notify callbacks

Status: RESOLVED FIXED

(Core :: WebRTC, defect)

Description

[Anant Narayanan [:anant]]

When content assgins (or de-assigns) a stream on media elements like <video> and <audio>, the underlying media engine should be notified via "NotifyStartConsuming" and "NotifyStopConsuming" callbacks.

Comment 1

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Attachment: fix

This is actually totally untested, but it's pretty simple so I expect it to work.

Comment 2

[Randell Jesup [:jesup] (needinfo me)]

Comment on attachment 628608 [details] [diff] [review]
fix

Looks good to me.   Pretty simple and direct.  r+ from me (though I wasn't asked) assuming it meets all of anant's needs (and I strongly suspect it will).

Comment 3

[Anant Narayanan [:anant]]

Comment on attachment 628608 [details] [diff] [review]
fix

I updated the DOM patch to use this. Though it doesn't quite work yet (main thread weirdness), the API is great! I'll post on the bug again when I get it work for real.

Comment 4

[Anant Narayanan [:anant]]

I just tested this out, and it works. Let's land it!

Comment 5

[Robert O'Callahan (:roc) (email my personal email if necessary)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/2074a7b72bd1

Comment 6

[Anant Narayanan [:anant]]

NotfyConsumptionChanged does not get called when a window with a live media stream is closed. Is that expected behaviour?

Comment 7

[(no longer active)]

https://hg.mozilla.org/mozilla-central/rev/2074a7b72bd1

Comment 8

[Robert O'Callahan (:roc) (email my personal email if necessary)]

(In reply to Anant Narayanan [:anant] from comment #6)
> NotfyConsumptionChanged does not get called when a window with a live media
> stream is closed. Is that expected behaviour?

Yes.

If you want to stop producing video/audio immediately when your window is closed, you should track whether your document is active or not.

Comment 9

[Robert O'Callahan (:roc) (email my personal email if necessary)]

We could change NotfyConsumptionChanged so that we say "no consumers" under more conditions, but it's not clear to me what the right definition would be. One issue is that it's not easy to distinguish between "page going into bfcache and may be revisited and resume playing" from "window closed". In fact it is possible to close a window containing a playing media element, but keep a reference to that element from another window, and then pull the media element out into a document that's still visible and resume it playing!

Comment 10

[Anant Narayanan [:anant]]

(In reply to Robert O'Callahan (:roc) (Mozilla Corporation) from comment #9)
> We could change NotfyConsumptionChanged so that we say "no consumers" under
> more conditions, but it's not clear to me what the right definition would
> be. One issue is that it's not easy to distinguish between "page going into
> bfcache and may be revisited and resume playing" from "window closed". In
> fact it is possible to close a window containing a playing media element,
> but keep a reference to that element from another window, and then pull the
> media element out into a document that's still visible and resume it playing!

Ah, that's an interesting use-case. We might want to support resuming a video call from an accidentally closed tab or going back from accidental page navigation. On the other hand, there might be other security issues with this behaviour. Regardless, you're right that the current behaviour is more flexible, not tying a stream's lifetime to a window is the right thing to do.

I've changed the DOM bindings to watch for window navigation and closures at which point we release our reference to the stream. At a later point, we may want to keep the stream around to enable some interesting use cases, but this works for now.