Closed
Bug 1509548
Opened 5 years ago
Closed 5 years ago
Reenable the start time assertion in StreamTracks.h
Categories
(Core :: WebRTC: Audio/Video, enhancement, P2)
Core
WebRTC: Audio/Video
Tracking
()
RESOLVED
FIXED
mozilla65
Tracking | Status | |
---|---|---|
firefox65 | --- | fixed |
People
(Reporter: padenot, Assigned: pehrsons)
References
Details
Attachments
(6 files, 1 obsolete file)
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 |
Disable to keep the Stream -> Tracks refactoring in the tree, but we should have a look.
Reporter | ||
Comment 1•5 years ago
|
||
Pushed by padenot@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/a95519247ae7 Temporarily disable an assertion following 1423241.
Reporter | ||
Comment 3•5 years ago
|
||
Andreas, if you could have a look in a timely manner it would be great, thanks!
Blocks: 1423241
Flags: needinfo?(apehrson)
Comment 4•5 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/a95519247ae7
Status: NEW → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla65
Reporter | ||
Comment 5•5 years ago
|
||
Oops I meant to mark this one leave-open.
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Assignee | ||
Updated•5 years ago
|
Status: REOPENED → ASSIGNED
Flags: needinfo?(apehrson)
Assignee | ||
Comment 6•5 years ago
|
||
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®exp=false&path=
Assignee | ||
Comment 7•5 years ago
|
||
Assignee | ||
Comment 8•5 years ago
|
||
They deserve descriptive names.
Assignee | ||
Comment 9•5 years ago
|
||
Assignee | ||
Comment 10•5 years ago
|
||
Assignee | ||
Comment 11•5 years ago
|
||
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.
Updated•5 years ago
|
Attachment #9027576 -
Attachment is obsolete: true
Assignee | ||
Comment 12•5 years ago
|
||
Updated•5 years ago
|
Rank: 15
Priority: -- → P2
Comment 13•5 years ago
|
||
Pushed by pehrsons@gmail.com: 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•5 years ago
|
||
bugherder |
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
Status: ASSIGNED → RESOLVED
Closed: 5 years ago → 5 years ago
Resolution: --- → FIXED
You need to log in
before you can comment on or make changes to this bug.
Description
•