feat: support multi-period live DASH with changing periods #144
Conversation
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 addition, playlists that need to be excluded are identified in the returned object. BREAKING CHANGE: the top level API of this module no longer returns the manifest alone
README.md
Outdated
| playlistsToExclude, | ||
| mediaGroupPlaylistsToExclude |
There was a problem hiding this comment.
how bad would it be to include these properties on the returned manifest object directly?
There was a problem hiding this comment.
Possibly with noop function even if lastMpd isn't passed to us, so that we are consistent.
src/toPlaylists.js
Outdated
| // TODO Decide if SegmentList be chosen ahead of SegmentBase. | ||
| // | ||
| // Judging by the specification (see the 2012 specification, section 5.3.9.2), | ||
| // SegmentBase seems to only be used as the strategy if there's no SegmentTemplate or | ||
| // SegmentList. However, the historical ordering here will use SegmentBase as the | ||
| // strategy before SegmentList. This ordering should be reconsidered. |
There was a problem hiding this comment.
I believe the DASH profile specifies which of these will be used/available. Something to look into in the future.
src/playlist-merge.js
Outdated
| /** | ||
| * Finds the media group playlist with the matching NAME attribute. | ||
| * | ||
| * @param {Object} config |
There was a problem hiding this comment.
hopefully, to resolve linter issues
| * @param {Object} config | |
| * @param {Object} config options object |
src/playlist-merge.js
Outdated
| /** | ||
| * Returns any old playlists that are no longer available in the new playlists. | ||
| * | ||
| * @param {Object} config |
There was a problem hiding this comment.
hopefully, to resolve linter issues
| * @param {Object} config | |
| * @param {Object} config options object |
src/playlist-merge.js
Outdated
| * Returns any old media group playlists that are no longer available in the new media | ||
| * group playlists. | ||
| * | ||
| * @param {Object} config |
There was a problem hiding this comment.
hopefully, to resolve linter issues
| * @param {Object} config | |
| * @param {Object} config options object |
src/playlist-merge.js
Outdated
| * Goes through the provided segments and updates the appropriate sequence and timeline | ||
| * related attributes. | ||
| * | ||
| * @param {Object} config |
There was a problem hiding this comment.
hopefully, to resolve linter issues
| * @param {Object} config | |
| * @param {Object} config options object |
src/playlist-merge.js
Outdated
| // a segment with a matching presentation time. These values should not change on | ||
| // refreshes. | ||
| const firstNewSegment = newSegments[0]; | ||
| const oldMatchingSegmentIndex = oldSegments.findIndex((oldSegment) => |
There was a problem hiding this comment.
findIndex isn't available on IE11, unfortunately. And we should avoid breaking it if we can.
src/playlist-merge.js
Outdated
| // and the new playlist exists for periods/timelines after a certain point) then it will | ||
| // be available once it is no longer incomplete. | ||
| newManifest.playlists = | ||
| newManifest.playlists.filter((playlist) => !incompletePlaylists.includes(playlist)); |
There was a problem hiding this comment.
.includes isn't available on IE11 either, unfortunately.
src/playlist-merge.js
Outdated
| // matching presentation times in another playlist (or matching presentation time | ||
| // ranges within timelines) and sequencing from there. | ||
| newManifest.playlists = | ||
| newManifest.playlists.filter((playlist) => !addedPlaylists.includes(playlist)); |
There was a problem hiding this comment.
.includes isn't available on IE11 either, unfortunately.
src/playlist-merge.js
Outdated
|
|
||
| forEachMediaGroup(manifest, SUPPORTED_MEDIA_TYPES, (properties, type, group, label) => { | ||
| properties.playlists.forEach((playlist) => { | ||
| if (playlists.includes(playlist)) { |
There was a problem hiding this comment.
.includes isn't available on IE11 either, unfortunately.
There was a problem hiding this comment.
I usually use .some in place of .includes
README.md
Outdated
| playlistsToExclude, | ||
| mediaGroupPlaylistsToExclude |
There was a problem hiding this comment.
Possibly with noop function even if lastMpd isn't passed to us, so that we are consistent.
src/playlist-merge.js
Outdated
|
|
||
| forEachMediaGroup(manifest, SUPPORTED_MEDIA_TYPES, (properties, type, group, label) => { | ||
| properties.playlists.forEach((playlist) => { | ||
| if (playlists.includes(playlist)) { |
There was a problem hiding this comment.
I usually use .some in place of .includes
src/toPlaylists.js
Outdated
| @@ -10,6 +10,12 @@ export const generateSegments = ({ attributes, segmentInfo }) => { | |||
| if (segmentInfo.template) { | |||
| segmentsFn = segmentsFromTemplate; | |||
| segmentAttributes = merge(attributes, segmentInfo.template); | |||
| // TODO Decide if SegmentList be chosen ahead of SegmentBase. | |||
There was a problem hiding this comment.
| // TODO Decide if SegmentList be chosen ahead of SegmentBase. | |
| // TODO: Decide if SegmentList be chosen ahead of SegmentBase. |
So that it shows up in the linter
|
Closing in favor of #152 |
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 addition, playlists that
need to be excluded are identified in the returned object.
BREAKING CHANGE: the top level API of this module no longer returns the
manifest alone