Closed Bug 1423241 Opened 3 years ago Closed 2 years ago

Remove MediaStreamListener

Categories

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

58 Branch
enhancement

Tracking

()

RESOLVED FIXED
mozilla65
Tracking Status
firefox65 --- fixed

People

(Reporter: pehrsons, Assigned: pehrsons)

References

(Blocks 1 open bug)

Details

Attachments

(37 files)

46 bytes, text/x-phabricator-request
Details | Review
46 bytes, text/x-phabricator-request
Details | Review
46 bytes, text/x-phabricator-request
Details | Review
46 bytes, text/x-phabricator-request
Details | Review
46 bytes, text/x-phabricator-request
Details | Review
46 bytes, text/x-phabricator-request
Details | Review
46 bytes, text/x-phabricator-request
Details | Review
46 bytes, text/x-phabricator-request
Details | Review
46 bytes, text/x-phabricator-request
Details | Review
46 bytes, text/x-phabricator-request
Details | Review
46 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
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.
Rank: 13
Priority: -- → P2
Assignee: nobody → apehrson
Status: NEW → ASSIGNED
No longer depends on: 1302379
This can conceal real bugs. Tests should be fixed so they don't risk calling
getPixel in invalid states instead.
HasVideo() might be false even though there is a video track present as it will
only look at the resolution of a VideoTrack.
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.
This allows DecodedStream to accurately track how many samples have been
appended to a track, even with resampling enabled.
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().
Attachment #9026057 - Attachment description: Bug 1423241 - Move SpeechRecognition from stream listener to track listener. → Bug 1423241 - Move SpeechRecognition from stream listener to track listener. r?padenot
Pushed by pehrsons@gmail.com:
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
Depends on: 1509548
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
Status: ASSIGNED → RESOLVED
Closed: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla65
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.
I agree. MediaStreamListener is entirely internal.
(In reply to Andreas Pehrson [:pehrsons] from comment #41)
> I agree. MediaStreamListener is entirely internal.

OK, removing dev-doc-needed then.
Keywords: dev-doc-needed
No longer depends on: 1536766
Regressions: 1536766
Regressions: 1559568
You need to log in before you can comment on or make changes to this bug.