feat: support multi-period live DASH with changing periods #152
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
In order to handle live DASH manifests where periods are changed, added, and removed, merging is used to ensure that the new manifest is a continuation of the last parsed manifest. In order to handle live cases, the last MPD must be provided via the `lastMpd` option. BREAKING CHANGE: period start time is now used as the timeline instead of period index
Codecov Report
@@ Coverage Diff @@
## main #152 +/- ##
==========================================
+ Coverage 93.17% 94.30% +1.12%
==========================================
Files 17 18 +1
Lines 674 772 +98
Branches 224 240 +16
==========================================
+ Hits 628 728 +100
+ Misses 46 44 -2
Continue to review full report at Codecov.
|
| playlist.discontinuitySequence = findIndex(timelineStarts, ({ timeline }) => timeline === playlist.timeline); | ||
|
|
||
| if (!playlist.segments) { | ||
| return; |
There was a problem hiding this comment.
might as well add a test here
gkatsev
reviewed
Nov 3, 2021
gkatsev
reviewed
Nov 15, 2021
README.md
Outdated
| @@ -44,6 +44,17 @@ const manifest = await res.text(); | |||
| var parsedManifest = mpdParser.parse(manifest, { manifestUri }); | |||
| ``` | |||
|
|
|||
| If dealing with a live stream, then on subsequent calls to parse, the last parsed manifest | |||
There was a problem hiding this comment.
Suggested change
| If dealing with a live stream, then on subsequent calls to parse, the last parsed manifest | |
| If dealing with a live stream, then on subsequent calls to parse, the previously parsed manifest |
|
|
||
| const SUPPORTED_MEDIA_TYPES = ['AUDIO', 'SUBTITLES']; | ||
| // allow one 60fps frame as leniency (arbitrarily chosen) | ||
| const TIME_FUDGE = 1 / 60; |
src/playlist-merge.js
Outdated
Comment on lines
16
to
17
| return union(timelineStarts, ({ timeline }) => timeline).sort((a, b) => | ||
| (a.timeline > b.timeline) ? 1 : -1); |
There was a problem hiding this comment.
Splitting on the .sort, I think, makes it a lot easier to understand that we call union and then sort on the response from union. At first, I thought that we were passing a method and the sort as a second argument to union.
The linter should be fine with this, I checked.
Suggested change
| return union(timelineStarts, ({ timeline }) => timeline).sort((a, b) => | |
| (a.timeline > b.timeline) ? 1 : -1); | |
| return union(timelineStarts, ({ timeline }) => timeline) | |
| .sort((a, b) => (a.timeline > b.timeline) ? 1 : -1); |
Comment on lines
+64
to
+66
| playlist.segments.forEach((segment, index) => { | ||
| segment.number = playlist.mediaSequence + index; | ||
| }); |
There was a problem hiding this comment.
I guess this is part of the magic that makes thing whole thing work?
There was a problem hiding this comment.
Along with the passed-in media sequence calculations in updateSequenceNumber below.
Multiperiod SIDX (in both live and VOD) and live SIDX where the SIDX properties change on refresh, are not yet supported in mpd-parser. To prevent exceptions, return early from merging logic on SIDX detection, and add a TODO comment for the future.
Updates old start references to periodStart, as periodStart should not be generated where the start attribute was missing. Fixes durationTimeParser segment time values by using the number rather than a 0 based index for calcuating time. Add comments in durationTimeParser to make logic clearer.
gkatsev
previously approved these changes
Dec 6, 2021
gesinger
added a commit
to gesinger/http-streaming
that referenced
this pull request
Dec 6, 2021
In order to support the new mpd-parser API, whenever a DASH manifest refreshes, the prior parsed manifest object needs to be passed in as `previousManifest`. This change adds support for that. NOTE: this is to support an upcoming feature in mpd-parser: videojs/mpd-parser#152
6 tasks
gkatsev
approved these changes
Dec 15, 2021
gkatsev
approved these changes
Dec 17, 2021
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.
In order to handle live DASH manifests where periods are changed, added, and removed,
merging is used to ensure that the new manifest is a continuation of the last parsed
manifest.
In order to handle live cases, the last MPD must be provided via the
lastMpdoption.BREAKING CHANGE: period start time is now used as the timeline instead of period index