Remove MediaStreamListener

RESOLVED FIXED in Firefox 65

Status

()

enhancement
P2
normal
Rank:
13
RESOLVED FIXED
2 years ago
3 days ago

People

(Reporter: pehrsons, Assigned: pehrsons)

Tracking

(Blocks 2 bugs, Regressed 1 bug)

58 Branch
mozilla65
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox65 fixed)

Details

Attachments

(37 attachments)

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
Assignee

Description

2 years ago
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.
Assignee

Updated

2 years ago
Rank: 13
Priority: -- → P2
Assignee

Updated

2 years ago
Blocks: 1423253
Assignee

Updated

Last year
Blocks: 1454998
Assignee

Updated

9 months ago
Assignee: nobody → apehrson
Status: NEW → ASSIGNED
Assignee

Updated

9 months ago
Blocks: 1493613
Assignee

Updated

9 months ago
No longer blocks: 1454998
Assignee

Updated

9 months ago
No longer blocks: 1423253
Assignee

Updated

9 months ago
Depends on: 1302379
Assignee

Updated

9 months ago
No longer depends on: 1302379
Assignee

Comment 6

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

Comment 9

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

Comment 17

7 months ago
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.
Assignee

Comment 20

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

Comment 21

7 months ago
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

Comment 38

7 months ago
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

Comment 39

7 months ago
bugherder
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: 7 months ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla65
Assignee

Updated

7 months ago
Depends on: 1306999
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.
Assignee

Comment 41

7 months ago
I agree. MediaStreamListener is entirely internal.
Assignee

Updated

6 months ago
Depends on: 1512958
(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
Assignee

Updated

6 months ago
Depends on: 1515068
Assignee

Updated

3 months ago
Depends on: 1536766
Assignee

Updated

2 months ago
No longer depends on: 1536766
Regressions: 1536766
Assignee

Updated

2 months ago
Regressions: 1544650

Updated

3 days ago
Regressions: 1559568
You need to log in before you can comment on or make changes to this bug.