Use exposed transmuxer time modifications for more accurate conversion between stream and player times #371
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
* Change `appendBuffer` to use a config instead of an optional trailing callback parameter * Add `playerTimeToStreamTime` function to convert player time within a segment to its associated stream time * Add `segmentForStreamTime` function to use EXT-X-PROGRAM-DATE-TIME to determine the segment which should contain the passed stream time * Add `segmentForPlayerTime` function to use video timing info attached to the playlist after transmuxing to determine accurate times, or estimates based on manifest durations, to get the segment which should contain the passed player time * Experiment with using those functions in findSegmentForPlayerTime and findSegmentForStreamTime
* Handle when `done` callback doesn't exist * Change mock function signature and behavior * Use new function signature in source updater tests
* Add transmuxedPresentationEnd to videoTimingInfo to get actual segment duration for calculations * Add TODO for fmp4 timing info, since fmp4 segments won't go through the transmuxer * Rename functions to match old function names * Remove now unused segmentForTime * Add videoTimingInfo where needed in tests, to represent already transmuxed segments * Change tests such that findSegmentForStreamTime can always find an accurate segment unless the segment is the last one, as the program date times will represent accurate ranges * Add targetDurations where needed in tests
* Minor edits to stream time from player time doc * Rename videoTimingInfo to videoSegmentTimingInfo to follow mux.js change * Rename prependedGopDuration to prependedContentDuration to follow mux.js change
* Removed unused timeWithinSegment function * Add SEGMENT_END_FUDGE_PERCENT for estimates of last segment timing when determining a player or stream time from a stream or player time * Account for fudge in findSegmentForStreamTime * Clean up some of the comments and code in time utils * Add TODO for accounting for prepended content duration in time utils * Add more tests for findSegmentForPlayerTime and findSegmentForStreamTime
ldayananda
reviewed
Jan 14, 2019
docs/stream-time-from-player-time.md
Outdated
| const transmuxerPrependedSeconds = segment.videoTimingInfo.transmuxerPrependedSeconds; | ||
| const transmuxedStart = segment.videoTimingInfo.transmuxedPresentationStart; | ||
|
|
||
| // get the proper start of new content (not prepended old content) from the segment, |
There was a problem hiding this comment.
although I get what you mean to say, it might be easier for someone new to follow if you say this is the "start of content before old content is prepended"?
docs/stream-time-from-player-time.md
Outdated
| // segment2: 2 => 4 | ||
| // segment3: 4 => 6 | ||
|
|
||
| const segment2 = { |
There was a problem hiding this comment.
We should also have an example of how to get the time for the first segment, which would need to use the time since beginning of the stream.
Is transmuxerPrependedSeconds guaranteed to be 0 for the first segment?
There was a problem hiding this comment.
For now transmuxerPrependedSeconds is guaranteed to be 0 for the first segment, though it could change in the future (if, for instance, we get into prepending blank video frames to start a video with offset timing).
docs/stream-time-from-player-time.md
Outdated
| playerTimeToStreamTime(2.5, segment2); | ||
| // startOfSegment = 1.7 + 0.3 = 2 | ||
| // offsetFromSegmentStart = 2.5 - 2 = 0.5 | ||
| // return 32.1 + 0.5 = 32.6 |
There was a problem hiding this comment.
for consistency, might want to use the getTime version of 32.1 so it matches the steps and result of the formula
player times * Fix line breaks in docs/stream-time-from-player-time.md * Improve comments in docs/stream-time-from-player-time.md * Add and improve examples in docs/stream-time-from-player-time.md * Treat done callback as required for appendBuffer in source updater * Add jsdoc to playerTimeToStreamTime * Improve some comments in src/util/time.js
immediately available * Add creating-content.md doc file with ffmpeg command to create HLS VOD stream with EXT-X-PROGRAM-DATE-TIME tags * Fix findSegmentForStreamTime to return estimates when the segment is not buffered, and use player time for estimated segment start instead of stream time * Fix findSegmentForStreamTime to return an estimated start of segment using playlist duration function * Fix seekToStreamTime to perform seeks when findSegmentForStreamTime returns estimates * Add mediaSequence values to mock playlists in time module tests * Use mock videoSegmentTimingInfo in a seekToStreamTime test
precise, and update docs
ldayananda
reviewed
Jan 25, 2019
playlists When handling video segment timing info in segment loader, the segment was retrieved using the segment info object's playlist reference and media index. However, if a live playlist refreshed during transmux, then the playlist refrenced would be an old one, leaving the probed start/end times on the updated segment, and the video timing info on the old segment. Instead, use the segment info object's segment reference, which is updated to point to the new segment on playlist refreshes.
ldayananda
previously approved these changes
Jan 31, 2019
This prevents an async video segment timing info callback from being assigned to a different segment than originally requested.
and seekToProgramTime Program time more accurately reflects the times being converted, as EXT-X-PROGRAM-DATE-TIME is used for HLS, and stream time can be ambiguous as to whether it references a real world time or the timestamps present in the stream.
Firefox won't loop if seeking directly to the duration, or to too close to the duration. 100ms from the duration seems to work.
forbesjo
approved these changes
Feb 7, 2019
brandonocasey
added a commit
that referenced
this pull request
May 2, 2019
brandonocasey
added a commit
that referenced
this pull request
Aug 19, 2019
This was caused by a bad merge of #371 into the lhls/tba branch, which is now master.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Requires mux.js master (not yet versioned)
Description
In order to more accurately convert between player times and stream times, the modifications done to a segment's timing values by the transmuxer must be exposed and used. In addition
Requirements Checklist