1509548 - Reenable the start time assertion in StreamTracks.h

Status: RESOLVED FIXED

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

Description

[Paul Adenot (:padenot)]

Disable to keep the Stream -> Tracks refactoring in the tree, but we should have a look.

Comment 1

[Paul Adenot (:padenot)]

Attachment: Bug 1509548 - Temporarily disable an assertion following 1423241.

Comment 2

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/a95519247ae7
Temporarily disable an assertion following 1423241.

Comment 3

[Paul Adenot (:padenot)]

Andreas, if you could have a look in a timely manner it would be great, thanks!

Comment 4

[Bogdan Tara[:bogdan_tara | bogdant]]

https://hg.mozilla.org/mozilla-central/rev/a95519247ae7

Comment 5

[Paul Adenot (:padenot)]

Oops I meant to mark this one leave-open.

Comment 6

[Andreas Pehrson [:pehrsons]]

Since DecodedStream pushes data ahead of the MediaStreamGraph's currentTime we have some data buffered in StreamTracks.

When seeking, the original media element stops playback and starts seeking. The captureStream will continue to play the buffered data instead (breaking spec).

When seeking is finished the original media element starts playback again, from the new position. DecodedStream on the other hand creates new tracks for playing from the new position (breaking spec). If the old tracks haven't ended yet, there's an overlap in the stream's timeline, and the new tracks are added to the graph *before* the knownTracksTime (which is the end of the pushed data) and we fail the assert.

This ties a bit into bug 1172394 since we should not create new tracks when seeking. Fixing that should be the long-term solution for this bug.

Short-term I feel inclined to remove knownTracksTime altogether. It is used as a measure outside of end times of track's to keep a stream's currentTime from advancing. With it removed, a stream's currentTime would instead be capped at the earliest end time of its non-ended tracks. That seems fine.
We'd also lose this assert, but that seems fine to me too.


Also note that of all users of SourceMediaStream, only DecodedStream is currently using AdvanceKnownTracksTime, https://searchfox.org/mozilla-central/search?q=SourceMediaStream%3A%3AAdvanceKnownTracksTime&case=false&regexp=false&path=

Comment 7

[Andreas Pehrson [:pehrsons]]

Attachment: Bug 1509548 - Remove the concept of a known tracks time from MediaStreamGraph. r?padenot

Comment 8

[Andreas Pehrson [:pehrsons]]

Attachment: Bug 1509548 - Rename track-end-time methods in StreamTracks. r?padenot

They deserve descriptive names.

Comment 9

[Andreas Pehrson [:pehrsons]]

Attachment: Bug 1509548 - Clean up what appears to be wip-leftover gunk in DecodedStream. r?jya

Comment 10

[Andreas Pehrson [:pehrsons]]

Attachment: Bug 1509548 - Enable pulling after adding listeners in MediaManager. r?padenot

Comment 11

[Andreas Pehrson [:pehrsons]]

Attachment: Bug 1509548 - Remove early PullNewData return. r?padenot

Without knownTracksTime, StreamTracks::GetFirstTrackEnd() returns
STREAM_TIME_MAX for an empty StreamTracks, so PullNewData() thinks no new data
is needed.

This circumvents that by always checking whether tracks need data.

Comment 12

[Andreas Pehrson [:pehrsons]]

Attachment: Bug 1509548 - Make MediaStreamGraph pull data per track instead of per stream. r?padenot

Comment 13

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/1d1b47e23537
Remove the concept of a known tracks time from MediaStreamGraph. r=padenot
https://hg.mozilla.org/integration/autoland/rev/8a064f0dbb2f
Rename track-end-time methods in StreamTracks. r=padenot
https://hg.mozilla.org/integration/autoland/rev/dfde7b2c53a6
Clean up what appears to be wip-leftover gunk in DecodedStream. r=jya
https://hg.mozilla.org/integration/autoland/rev/311cee86bc66
Remove early PullNewData return. r=padenot
https://hg.mozilla.org/integration/autoland/rev/daaecb62f373
Make MediaStreamGraph pull data per track instead of per stream. r=padenot

Comment 14

[Razvan Maries]

https://hg.mozilla.org/mozilla-central/rev/1d1b47e23537
https://hg.mozilla.org/mozilla-central/rev/8a064f0dbb2f
https://hg.mozilla.org/mozilla-central/rev/dfde7b2c53a6
https://hg.mozilla.org/mozilla-central/rev/311cee86bc66
https://hg.mozilla.org/mozilla-central/rev/daaecb62f373