1423241 - Remove MediaStreamListener

Status: RESOLVED FIXED

(Core :: Audio/Video: MediaStreamGraph, enhancement, P2)

Description

[Andreas Pehrson [:pehrsons]]

Currently there are some callbacks that only exist in MediaStreamListener (NotifyPull, for instance). These will have to be ported to MediaStreamTrackListener.

Then all users of MediaStreamListener can be ported to MediaStreamTrackListener and we can remove MediaStreamListener altogether.

Comment 1

[Andreas Pehrson [:pehrsons]]

Attachment: Bug 1423241 - Implement MediaStreamTrackListener::NotifyOutput. r?padenot

Comment 2

[Andreas Pehrson [:pehrsons]]

Attachment: Bug 1423241 - Implement HTMLMediaElement::IsPlaybackEnded and IsEnded properly for MediaStreams. r?jya

Comment 3

[Andreas Pehrson [:pehrsons]]

Attachment: Bug 1423241 - Rename StreamSizeListener to VideoFrameListener and minor cleanup. r?jya

Comment 4

[Andreas Pehrson [:pehrsons]]

Attachment: Bug 1423241 - Remove HTMLMediaElement::StreamListener::NotifyHasCurrentData. r?jya

Comment 5

[Andreas Pehrson [:pehrsons]]

Attachment: Bug 1423241 - Remove a rawptr in HTMLMediaElement. r?jya

Comment 6

[Andreas Pehrson [:pehrsons]]

Attachment: Bug 1423241 - Remove drawImage exception handling from captureStream_common.js. r?jib

This can conceal real bugs. Tests should be fixed so they don't risk calling
getPixel in invalid states instead.

Comment 7

[Andreas Pehrson [:pehrsons]]

Attachment: Bug 1423241 - Expose MSG's GraphTime through main-thread-Watchable and move media element to it. r?padenot

Comment 8

[Andreas Pehrson [:pehrsons]]

Attachment: Bug 1423241 - Remove screen-wakelock code that no longer has an effect. r?alwu

Comment 9

[Andreas Pehrson [:pehrsons]]

Attachment: Bug 1423241 - Hold back readyState while no frame has been displayed as intended. r?jya

HasVideo() might be false even though there is a video track present as it will
only look at the resolution of a VideoTrack.

Comment 10

[Andreas Pehrson [:pehrsons]]

Attachment: Bug 1423241 - Move MediaPipeline from MediaStreamListener to MediaStreamTrackListener. r?padenot

Comment 11

[Andreas Pehrson [:pehrsons]]

Attachment: Bug 1423241 - Move CanvasCaptureMediaStream from stream listener to track listener. r?padenot

Comment 12

[Andreas Pehrson [:pehrsons]]

Attachment: Bug 1423241 - Move special media element captureStream handling of inactive stream out of MediaStreamGraph. r?jib

Comment 13

[Andreas Pehrson [:pehrsons]]

Attachment: Bug 1423241 - Move CanvasCaptureMediaStream MSG cleanup to MediaStreamTrackListener. r?padenot

Comment 14

[Andreas Pehrson [:pehrsons]]

Attachment: Bug 1423241 - Move SpeechRecognition from stream listener to track listener. r?padenot

Comment 15

[Andreas Pehrson [:pehrsons]]

Attachment: Bug 1423241 - Implement NotifyPull for MediaStreamTrackListener. r?padenot

Comment 16

[Andreas Pehrson [:pehrsons]]

Attachment: Bug 1423241 - Move MediaManager from a stream listener to track listeners. r?padenot

Comment 17

[Andreas Pehrson [:pehrsons]]

Attachment: Bug 1423241 - Fix MediaStreamTrackListener::NotifyEnded. r?padenot

Without this, NotifyEnded() happens before the track has been played out, at the
time it's marked ended by its producer. This change will actually make us wait
until the last chunk has been played out and then notify listeners.

Comment 18

[Andreas Pehrson [:pehrsons]]

Attachment: Bug 1423241 - Ensure audio loads after video to make drawImage happy. r?jib

https://github.com/whatwg/html/issues/4128 filed to cover this case in the spec.

Comment 19

[Andreas Pehrson [:pehrsons]]

Attachment: Bug 1423241 - Always add tracks at the stream's current time. r?padenot

Comment 20

[Andreas Pehrson [:pehrsons]]

Attachment: Bug 1423241 - Return appended StreamTime samples appended through AppendToTrack. r?padenot

This allows DecodedStream to accurately track how many samples have been
appended to a track, even with resampling enabled.

Comment 21

[Andreas Pehrson [:pehrsons]]

Attachment: Bug 1423241 - Refactor DecodedStream. r?jya

This removes DecodedStream's use of MediaStreamListener in favor of
MediaStreamTrackListener. This change has however rippled through to a lot
more cleanup, per below.

This moves the MediaStreamTrack lifetime ownership for captured
HTMLMediaElements from the media element to DecodedStream, where the
MediaStreamGraph-side tracks are already created and ended today.

This makes MediaStreamTrack creation explicit across the entire codebase and
lets us remove the MediaStreamTrackSourceGetter class and the infrastructure
of adding MediaStreamTracks after they've already been created in the graph
from DOMMediaStream.

With track ownership, and thus TrackID allocation ownership, happening
exclusively in DecodedStream for its output tracks, we also stop throwing
away and recreating the SourceMediaStream to which we feed data on seek.
This is one step closer to fixing bug 1172394 and spec compliance of
HTMLMediaElement.captureStream().

Comment 22

[Andreas Pehrson [:pehrsons]]

Attachment: Bug 1423241 - Remove a rawptr in MediaStreamTrack. r?padenot

Comment 23

[Andreas Pehrson [:pehrsons]]

Attachment: Bug 1423241 - Move track-ended notifications from DOMMediaStream to MediaStreamTrack. r?padenot

Comment 24

[Andreas Pehrson [:pehrsons]]

Attachment: Bug 1423241 - Remove DOMMediaStream::OwnedStreamListener. r?padenot

Comment 25

[Andreas Pehrson [:pehrsons]]

Attachment: Bug 1423241 - Remove OnTracksAvailableCallback from HTMLMediaElement. r?padenot

Comment 26

[Andreas Pehrson [:pehrsons]]

Attachment: Bug 1423241 - Remove OnTracksAvailableCallback from MediaManager. r?padenot

Comment 27

[Andreas Pehrson [:pehrsons]]

Attachment: Bug 1423241 - Remove OnTracksAvailableCallback from MediaRecorder. r?padenot

Comment 28

[Andreas Pehrson [:pehrsons]]

Attachment: Bug 1423241 - Ignore runnables for main thread after next stream state update, after entering shutdown. r?padenot

They may hang on to references causing leaks.

Comment 29

[Andreas Pehrson [:pehrsons]]

Attachment: Bug 1423241 - Remove DOMMediaStream::PlaybackStreamListener. r?padenot

Comment 30

[Andreas Pehrson [:pehrsons]]

Attachment: Bug 1423241 - Async/await-ify test_gUM_bug1223696.html. r?jib

Comment 31

[Andreas Pehrson [:pehrsons]]

Attachment: Bug 1423241 - Remove MediaStreamListener \o/. r?padenot

Comment 32

[Andreas Pehrson [:pehrsons]]

Attachment: Bug 1423241 - Remove fake-class-based webrtc unittests. r?dminor

Comment 33

[Andreas Pehrson [:pehrsons]]

Attachment: Bug 1423241 - Add logging for a captured MediaDecoder. r?jya

Comment 34

[Andreas Pehrson [:pehrsons]]

Attachment: Bug 1423241 - Unify decoder and stream paths in HTMLMediaElement::AddRemoveSelfReference. r?jya

Comment 35

[Andreas Pehrson [:pehrsons]]

Attachment: Bug 1423241 - Ignore ended tracks when checking for audio tracks in an MSG. r?padenot

Comment 36

[Andreas Pehrson [:pehrsons]]

Attachment: Bug 1423241 - Clean up iframes in between subtests in test_gUM_audioConstraints_concurrentIframes. r?padenot

Comment 37

[Andreas Pehrson [:pehrsons]]

Attachment: Bug 1423241 - Handle DOMMediaStream destroying its input stream before we can end its track. r?padenot

Comment 38

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/6eda96338365
Move special media element captureStream handling of inactive stream out of MediaStreamGraph. r=jib
https://hg.mozilla.org/integration/autoland/rev/24d6a327d6a2
Move CanvasCaptureMediaStream MSG cleanup to MediaStreamTrackListener. r=padenot
https://hg.mozilla.org/integration/autoland/rev/acd70715684a
Move SpeechRecognition from stream listener to track listener. r=padenot
https://hg.mozilla.org/integration/autoland/rev/f7fc271746a2
Implement NotifyPull for MediaStreamTrackListener. r=padenot
https://hg.mozilla.org/integration/autoland/rev/12f6bb0888a2
Move MediaManager from a stream listener to track listeners. r=padenot
https://hg.mozilla.org/integration/autoland/rev/befba547fb58
Fix MediaStreamTrackListener::NotifyEnded. r=padenot
https://hg.mozilla.org/integration/autoland/rev/de66629a4a20
Implement MediaStreamTrackListener::NotifyOutput. r=padenot
https://hg.mozilla.org/integration/autoland/rev/5bd5b88080df
Implement HTMLMediaElement::IsPlaybackEnded and IsEnded properly for MediaStreams. r=jya
https://hg.mozilla.org/integration/autoland/rev/d53898abd647
Rename StreamSizeListener to VideoFrameListener and minor cleanup. r=jya
https://hg.mozilla.org/integration/autoland/rev/7db08b6156f2
Remove HTMLMediaElement::StreamListener::NotifyHasCurrentData. r=jya
https://hg.mozilla.org/integration/autoland/rev/69551bbbf203
Remove a rawptr in HTMLMediaElement. r=jya
https://hg.mozilla.org/integration/autoland/rev/7964b0b2f836
Ensure audio loads after video to make drawImage happy. r=jib
https://hg.mozilla.org/integration/autoland/rev/82afe83c9c4f
Remove drawImage exception handling from captureStream_common.js. r=jib
https://hg.mozilla.org/integration/autoland/rev/430c57c4fb18
Expose MSG's GraphTime through main-thread-Watchable and move media element to it. r=padenot
https://hg.mozilla.org/integration/autoland/rev/720f409b83f2
Remove screen-wakelock code that no longer has an effect. r=alwu
https://hg.mozilla.org/integration/autoland/rev/e93cc0185c24
Hold back readyState while no frame has been displayed as intended. r=jya
https://hg.mozilla.org/integration/autoland/rev/fafb582ffcbc
Move MediaPipeline from MediaStreamListener to MediaStreamTrackListener. r=padenot
https://hg.mozilla.org/integration/autoland/rev/926b591bb0cf
Move CanvasCaptureMediaStream from stream listener to track listener. r=padenot
https://hg.mozilla.org/integration/autoland/rev/d39a3f7454a1
Always add tracks at the stream's current time. r=padenot
https://hg.mozilla.org/integration/autoland/rev/12f91d8a1f69
Return appended StreamTime samples appended through AppendToTrack. r=padenot
https://hg.mozilla.org/integration/autoland/rev/243a33803d16
Refactor DecodedStream. r=jya
https://hg.mozilla.org/integration/autoland/rev/897c956c3cf3
Remove a rawptr in MediaStreamTrack. r=padenot
https://hg.mozilla.org/integration/autoland/rev/76c2bf0ca8a6
Move track-ended notifications from DOMMediaStream to MediaStreamTrack. r=padenot
https://hg.mozilla.org/integration/autoland/rev/2b4b43f378d8
Remove DOMMediaStream::OwnedStreamListener. r=padenot
https://hg.mozilla.org/integration/autoland/rev/ed7c9d7a635d
Remove OnTracksAvailableCallback from HTMLMediaElement. r=padenot
https://hg.mozilla.org/integration/autoland/rev/7fef8af7ad93
Remove OnTracksAvailableCallback from MediaManager. r=padenot
https://hg.mozilla.org/integration/autoland/rev/3c5df9713725
Remove OnTracksAvailableCallback from MediaRecorder. r=padenot
https://hg.mozilla.org/integration/autoland/rev/d4767402cbb3
Ignore runnables for main thread after next stream state update, after entering shutdown. r=padenot
https://hg.mozilla.org/integration/autoland/rev/47958cf495da
Remove DOMMediaStream::PlaybackStreamListener. r=padenot
https://hg.mozilla.org/integration/autoland/rev/929905c7782b
Async/await-ify test_gUM_bug1223696.html. r=jib
https://hg.mozilla.org/integration/autoland/rev/ed8ffdb9c33a
Remove MediaStreamListener \o/. r=padenot
https://hg.mozilla.org/integration/autoland/rev/3ec97b5fccd7
Remove fake-class-based webrtc unittests. r=dminor
https://hg.mozilla.org/integration/autoland/rev/0e6607a8aaf6
Add logging for a captured MediaDecoder. r=jya
https://hg.mozilla.org/integration/autoland/rev/3db9a18c3061
Unify decoder and stream paths in HTMLMediaElement::AddRemoveSelfReference. r=jya
https://hg.mozilla.org/integration/autoland/rev/7ef7361d4849
Ignore ended tracks when checking for audio tracks in an MSG. r=padenot
https://hg.mozilla.org/integration/autoland/rev/0c83ce85257f
Clean up iframes in between subtests in test_gUM_audioConstraints_concurrentIframes. r=padenot
https://hg.mozilla.org/integration/autoland/rev/6be5d281ae92
Handle DOMMediaStream destroying its input stream before we can end its track. r=padenot

Comment 39

[Bogdan Tara[:bogdan_tara | bogdant]]

https://hg.mozilla.org/mozilla-central/rev/6eda96338365
https://hg.mozilla.org/mozilla-central/rev/24d6a327d6a2
https://hg.mozilla.org/mozilla-central/rev/acd70715684a
https://hg.mozilla.org/mozilla-central/rev/f7fc271746a2
https://hg.mozilla.org/mozilla-central/rev/12f6bb0888a2
https://hg.mozilla.org/mozilla-central/rev/befba547fb58
https://hg.mozilla.org/mozilla-central/rev/de66629a4a20
https://hg.mozilla.org/mozilla-central/rev/5bd5b88080df
https://hg.mozilla.org/mozilla-central/rev/d53898abd647
https://hg.mozilla.org/mozilla-central/rev/7db08b6156f2
https://hg.mozilla.org/mozilla-central/rev/69551bbbf203
https://hg.mozilla.org/mozilla-central/rev/7964b0b2f836
https://hg.mozilla.org/mozilla-central/rev/82afe83c9c4f
https://hg.mozilla.org/mozilla-central/rev/430c57c4fb18
https://hg.mozilla.org/mozilla-central/rev/720f409b83f2
https://hg.mozilla.org/mozilla-central/rev/e93cc0185c24
https://hg.mozilla.org/mozilla-central/rev/fafb582ffcbc
https://hg.mozilla.org/mozilla-central/rev/926b591bb0cf
https://hg.mozilla.org/mozilla-central/rev/d39a3f7454a1
https://hg.mozilla.org/mozilla-central/rev/12f91d8a1f69
https://hg.mozilla.org/mozilla-central/rev/243a33803d16
https://hg.mozilla.org/mozilla-central/rev/897c956c3cf3
https://hg.mozilla.org/mozilla-central/rev/76c2bf0ca8a6
https://hg.mozilla.org/mozilla-central/rev/2b4b43f378d8
https://hg.mozilla.org/mozilla-central/rev/ed7c9d7a635d
https://hg.mozilla.org/mozilla-central/rev/7fef8af7ad93
https://hg.mozilla.org/mozilla-central/rev/3c5df9713725
https://hg.mozilla.org/mozilla-central/rev/d4767402cbb3
https://hg.mozilla.org/mozilla-central/rev/47958cf495da
https://hg.mozilla.org/mozilla-central/rev/929905c7782b
https://hg.mozilla.org/mozilla-central/rev/ed8ffdb9c33a
https://hg.mozilla.org/mozilla-central/rev/3ec97b5fccd7
https://hg.mozilla.org/mozilla-central/rev/0e6607a8aaf6
https://hg.mozilla.org/mozilla-central/rev/3db9a18c3061
https://hg.mozilla.org/mozilla-central/rev/7ef7361d4849
https://hg.mozilla.org/mozilla-central/rev/0c83ce85257f
https://hg.mozilla.org/mozilla-central/rev/6be5d281ae92

Comment 40

[Eric Shepherd [:sheppy]]

This doesn't appear anywhere on MDN, so it seems that it has never been documented. I'm inclined to just let it stay that way.

Comment 41

[Andreas Pehrson [:pehrsons]]

I agree. MediaStreamListener is entirely internal.

Comment 42

[Chris Mills [:cmills]]

(In reply to Andreas Pehrson [:pehrsons] from comment #41)
> I agree. MediaStreamListener is entirely internal.

OK, removing dev-doc-needed then.