Conversation
…eNumbers Playlists whose segments come from a SegmentBase with an indexRange have an empty segments list until the referenced SIDX is resolved. When such a playlist is merged with a previous manifest (e.g. subtitle playlists, for which the SIDX is never resolved), updateSequenceNumbers would attempt to mark a discontinuity on a nonexistent first segment, throwing "Cannot set properties of undefined (setting 'discontinuity')". Skip sequence merging for playlists without segments, and fall back to an empty list when reading segments from the previous manifest's playlists. Fixes videojs#180
formatVttPlaylist dropped the sidx property carried by SegmentBase playlists, so text tracks could never resolve segment references from the sidxMapping (addSidxSegmentsToPlaylist was a no-op for them) and they bypassed the sidx early-return in updateSequenceNumbers, which assumed every remaining playlist has at least one segment. Keep sidx on the formatted vtt playlist like the audio and video formatters do. Refs videojs#180
This branch has not been deployed
This file contains hidden or 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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
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.
Playing a DASH manifest with an unresolved SIDX throws
Cannot set properties of undefined (setting 'discontinuity').updateSequenceNumbersinsrc/playlist-merge.jsreadsplaylist.segments[0],and a playlist whose SIDX has not been parsed yet has an empty segment list, so the read is undefined
and the assignment onto it throws.
The comment above that line said playlists with no segments need no support because early available
timelines are not supported. That reasoning holds for early available timelines and misses this case:
a SIDX playlist has segments, they just do not exist until the SIDX is fetched and its references are
generated.
A playlist with no segments is now skipped rather than dereferenced, and the old playlist's segments
default to an empty array for the same reason on the other side of the comparison. Skipping is right
here because there is nothing to renumber yet; the sequence numbers are assigned when the segments
arrive.
Tests cover a new playlist with no segments and an old one, both of which threw.
Fixes #180