Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: Use ll-hls query directives and support skipping segments #1079

Merged
merged 30 commits into from
May 26, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
30 commits
Select commit Hold shift + click to select a range
3d39612
feat: Use ll-hls query directives and support skipping segments
brandonocasey Feb 11, 2021
5e46a1e
Merge branch 'main' into feat/llhls-3
brandonocasey Apr 7, 2021
df957d4
Merge branch 'main' into feat/llhls-3
brandonocasey Apr 7, 2021
ff40332
unified manifest parse function
brandonocasey Apr 8, 2021
53ec980
tests for llhls and query directives in playlist-loader
brandonocasey Apr 8, 2021
addf841
Merge branch 'main' into feat/llhls-3
brandonocasey Apr 8, 2021
285a4ae
Merge branch 'main' into feat/llhls-3
brandonocasey Apr 26, 2021
a46f0a7
remove unused option
brandonocasey Apr 26, 2021
9d1fc06
add duration to preload segment
brandonocasey Apr 27, 2021
00a8e25
always add a part target duration when we have parts
brandonocasey Apr 28, 2021
1ff48b0
query directive fixes
brandonocasey Apr 28, 2021
3ff694a
small logging fix, bring map to skipped segments
brandonocasey Apr 28, 2021
d3fb477
pare down changes
brandonocasey Apr 28, 2021
2854967
cover all scenarios
brandonocasey Apr 29, 2021
a842b84
fix logging and merging issues
brandonocasey Apr 29, 2021
7898b4f
Merge branch 'main' into feat/llhls-3
brandonocasey May 24, 2021
3ff79bc
check for endlist before trying query directives
brandonocasey May 24, 2021
8ca1629
code review
brandonocasey May 24, 2021
941881f
use new media
brandonocasey May 25, 2021
9288f11
Update src/playlist-loader.js
brandonocasey May 25, 2021
adb47d2
Update src/playlist-loader.js
brandonocasey May 25, 2021
9744999
Update src/playlist-loader.js
brandonocasey May 25, 2021
1d2b8a2
Update test/playlist-loader.test.js
brandonocasey May 25, 2021
ee09077
remove uneeded comment and duplicate test
brandonocasey May 25, 2021
a1d3176
separate code
brandonocasey May 25, 2021
c555607
add before/afterEach
brandonocasey May 25, 2021
44b8830
log error on missing PART-TARGET
brandonocasey May 26, 2021
2aa89ab
add spec, capitalize llhls
brandonocasey May 26, 2021
bba29c4
add comment for _HLS_msn= logic
brandonocasey May 26, 2021
e8cd59d
Update src/playlist-loader.js
brandonocasey May 26, 2021
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 13 additions & 0 deletions src/manifest.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ import videojs from 'video.js';
import window from 'global/window';
import { Parser as M3u8Parser } from 'm3u8-parser';
import { resolveUrl } from './resolve-url';
import { getLastParts } from './playlist.js';

const { log } = videojs;

Expand Down Expand Up @@ -92,6 +93,18 @@ export const parseManifest = ({
manifest.targetDuration = targetDuration;
}

const parts = getLastParts(manifest);
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

set a default part target duration for manifests with parts that do not have one.


if (parts.length && !manifest.partTargetDuration) {
const partTargetDuration = parts.reduce((acc, p) => Math.max(acc, p.duration), 0);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since the spec says:

4.4.3.7.  EXT-X-PART-INF

   The EXT-X-PART-INF tag provides information about the Partial
   Segments in the Playlist.  It is REQUIRED if a Playlist contains one
   or more EXT-X-PART tags.

https://datatracker.ietf.org/doc/html/draft-pantos-hls-rfc8216bis-09#section-4.4.3.7

I think we may be better off throwing an error on the manifest if it's not available and parts are present.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In an ideal world I think we would, but we already guess at target duration above this so I think it makes sense to guess at partTargetDuration as well. Might be ok to log an error here though?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think an error would be good in that case. Maybe we can reference the spec too.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We do log onwarn below. Is that sufficient?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think a warning log is sufficient, as this goes against a requirement of the spec.


if (onwarn) {
onwarn(`manifest has no partTargetDuration defaulting to ${partTargetDuration}`);
log.error('LL-HLS manifest has parts but lacks required #EXT-X-PART-INF:PART-TARGET value. See https://datatracker.ietf.org/doc/html/draft-pantos-hls-rfc8216bis-09#section-4.4.3.7. Playback is not guaranteed.');
}
manifest.partTargetDuration = partTargetDuration;
}

return manifest;
};

Expand Down
246 changes: 191 additions & 55 deletions src/playlist-loader.js
Original file line number Diff line number Diff line change
Expand Up @@ -16,9 +16,69 @@ import {
setupMediaPlaylist,
forEachMediaGroup
} from './manifest';
import {getKnownPartCount} from './playlist.js';

const { mergeOptions, EventTarget } = videojs;

const addLLHLSQueryDirectives = (uri, media) => {
if (media.endList) {
return uri;
}
const query = [];

if (media.serverControl && media.serverControl.canBlockReload) {
const {preloadSegment} = media;
// next msn is a zero based value, length is not.
let nextMSN = media.mediaSequence + media.segments.length;

// If preload segment has parts then it is likely
// that we are going to request a part of that preload segment.
// the logic below is used to determine that.
if (preloadSegment) {
const parts = preloadSegment.parts || [];
// _HLS_part is a zero based index
const nextPart = getKnownPartCount(media) - 1;

// if nextPart is > -1 and not equal to just the
// length of parts, then we know we had part preload hints
// and we need to add the _HLS_part= query
if (nextPart > -1 && nextPart !== (parts.length - 1)) {
// add existing parts to our preload hints
query.push(`_HLS_part=${nextPart}`);
}

// this if statement makes sure that we request the msn
// of the preload segment if:
// 1. the preload segment had parts (and was not yet a full segment)
// but was added to our segments array
// 2. the preload segment had preload hints for parts that are not in
// the manifest yet.
// in all other cases we want the segment after the preload segment
// which will be given by using media.segments.length because it is 1 based
// rather than 0 based.
if (nextPart > -1 || parts.length) {
nextMSN--;
}
}

// add _HLS_msn= in front of any _HLS_part query
query.unshift(`_HLS_msn=${nextMSN}`);
}

if (media.serverControl && media.serverControl.canSkipUntil) {
// add _HLS_skip= infront of all other queries.
query.unshift('_HLS_skip=' + (media.serverControl.canSkipDateranges ? 'v2' : 'YES'));
}

query.forEach(function(str, i) {
const symbol = i === 0 ? '?' : '&';

uri += `${symbol}${str}`;
});

return uri;
};

/**
* Returns a new segment object with properties and
* the parts array merged.
Expand All @@ -35,6 +95,12 @@ export const updateSegment = (a, b) => {

const result = mergeOptions(a, b);

// if only the old segment has preload hints
// and the new one does not, remove preload hints.
if (a.preloadHints && !b.preloadHints) {
delete result.preloadHints;
}
gkatsev marked this conversation as resolved.
Show resolved Hide resolved

// if only the old segment has parts
// then the parts are no longer valid
if (a.parts && !b.parts) {
Expand All @@ -50,6 +116,18 @@ export const updateSegment = (a, b) => {
}
}

// set skipped to false for segments that have
// have had information merged from the old segment.
if (!a.skipped && b.skipped) {
result.skipped = false;
}

// set preload to false for segments that have
// had information added in the new segment.
if (a.preload && !b.preload) {
result.preload = false;
}

return result;
};

Expand All @@ -70,15 +148,30 @@ export const updateSegment = (a, b) => {
*/
export const updateSegments = (original, update, offset) => {
const oldSegments = original.slice();
const result = update.slice();
const newSegments = update.slice();

offset = offset || 0;
const length = Math.min(original.length, update.length + offset);
const result = [];

let currentMap;

for (let newIndex = 0; newIndex < newSegments.length; newIndex++) {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This function was completely changed. Previously we used to create a new array from newSegments and then loop over old segments, merging in old timing information and properties.

Now create an empty array for the result. Loop over every new segment and merge the accompanying old segment if we have one. Otherwise we just push the new segment into the list.

const oldSegment = oldSegments[newIndex + offset];
const newSegment = newSegments[newIndex];

if (oldSegment) {
currentMap = oldSegment.map || currentMap;

result.push(updateSegment(oldSegment, newSegment));
} else {
// carry over map to new segment if it is missing
if (currentMap && !newSegment.map) {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The big push for the refactor of this function was:

using HLS_skip=YES also skips init segment maps that have already been seen, so we need to make sure that the map that was in use for the last old segment is merged into new segments.

newSegment.map = currentMap;
}

for (let i = offset; i < length; i++) {
const newIndex = i - offset;
result.push(newSegment);

result[newIndex] = updateSegment(oldSegments[i], result[newIndex]);
}
}
return result;
};
Expand Down Expand Up @@ -116,12 +209,27 @@ export const resolveSegmentUris = (segment, baseUri) => {

const getAllSegments = function(media) {
const segments = media.segments || [];
const preloadSegment = media.preloadSegment;

// a preloadSegment with only preloadHints is not currently
// a usable segment, only include a preloadSegment that has
// parts.
if (media.preloadSegment && media.preloadSegment.parts) {
segments.push(media.preloadSegment);
if (preloadSegment && preloadSegment.parts && preloadSegment.parts.length) {
// if preloadHints has a MAP that means that the
// init segment is going to change. We cannot use any of the parts
// from this preload segment.
if (preloadSegment.preloadHints) {
for (let i = 0; i < preloadSegment.preloadHints.length; i++) {
if (preloadSegment.preloadHints[i].type === 'MAP') {
return segments;
gkatsev marked this conversation as resolved.
Show resolved Hide resolved
}
}
}
// set the duration for our preload segment to target duration.
preloadSegment.duration = media.targetDuration;
gesinger marked this conversation as resolved.
Show resolved Hide resolved
preloadSegment.preload = true;

segments.push(preloadSegment);
}

return segments;
Expand All @@ -147,28 +255,41 @@ export const isPlaylistUnchanged = (a, b) => a === b ||
* master playlist with the updated media playlist merged in, or
* null if the merge produced no change.
*/
export const updateMaster = (master, media, unchangedCheck = isPlaylistUnchanged) => {
export const updateMaster = (master, newMedia, unchangedCheck = isPlaylistUnchanged) => {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I continuously got confused by media and playlist here and realized that they are just terrible names. Most of the changes in this function, other than those noted, are for this rename.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thought (non-blocking): yeah, I'm sure have a lot of places where the terminology is confusing. We probably want to write up some guidelines around naming, particularly around words like media, playlist, etc.

const result = mergeOptions(master, {});
const playlist = result.playlists[media.id];
const oldMedia = result.playlists[newMedia.id];

if (!playlist) {
if (!oldMedia) {
return null;
}

if (unchangedCheck(playlist, media)) {
if (unchangedCheck(oldMedia, newMedia)) {
return null;
}

const mergedPlaylist = mergeOptions(playlist, media);
newMedia.segments = getAllSegments(newMedia);
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

preserve preloadSegment from newMedia during the merge that will happen below.


const mergedPlaylist = mergeOptions(oldMedia, newMedia);

media.segments = getAllSegments(media);
// always use the new media's preload segment
if (mergedPlaylist.preloadSegment && !newMedia.preloadSegment) {
delete mergedPlaylist.preloadSegment;
}

// if the update could overlap existing segment information, merge the two segment lists
if (playlist.segments) {
if (oldMedia.segments) {
if (newMedia.skip) {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if we have skipped segments, add them back in so that we can merge information from the old media segments into them.

newMedia.segments = newMedia.segments || [];
// add back in objects for skipped segments, so that we merge
// old properties into the new segments
for (let i = 0; i < newMedia.skip.skippedSegments; i++) {
newMedia.segments.unshift({skipped: true});
gesinger marked this conversation as resolved.
Show resolved Hide resolved
}
}
mergedPlaylist.segments = updateSegments(
playlist.segments,
media.segments,
media.mediaSequence - playlist.mediaSequence
oldMedia.segments,
newMedia.segments,
newMedia.mediaSequence - oldMedia.mediaSequence
);
}

Expand All @@ -181,22 +302,22 @@ export const updateMaster = (master, media, unchangedCheck = isPlaylistUnchanged
// that is referenced by index, and one by URI. The index reference may no longer be
// necessary.
for (let i = 0; i < result.playlists.length; i++) {
if (result.playlists[i].id === media.id) {
if (result.playlists[i].id === newMedia.id) {
result.playlists[i] = mergedPlaylist;
}
}
result.playlists[media.id] = mergedPlaylist;
result.playlists[newMedia.id] = mergedPlaylist;
// URI reference added for backwards compatibility
result.playlists[media.uri] = mergedPlaylist;
result.playlists[newMedia.uri] = mergedPlaylist;

// update media group playlist references.
forEachMediaGroup(master, (properties, mediaType, groupKey, labelKey) => {
brandonocasey marked this conversation as resolved.
Show resolved Hide resolved
if (!properties.playlists) {
return;
}
for (let i = 0; i < properties.playlists.length; i++) {
if (media.id === properties.playlists[i].id) {
properties.playlists[i] = media;
if (newMedia.id === properties.playlists[i].id) {
properties.playlists[i] = newMedia;
}
}
});
Expand Down Expand Up @@ -263,34 +384,44 @@ export default class PlaylistLoader extends EventTarget {
this.state = 'HAVE_NOTHING';

// live playlist staleness timeout
this.on('mediaupdatetimeout', () => {
if (this.state !== 'HAVE_METADATA') {
// only refresh the media playlist if no other activity is going on
return;
}
this.handleMediaupdatetimeout_ = this.handleMediaupdatetimeout_.bind(this);
this.on('mediaupdatetimeout', this.handleMediaupdatetimeout_);
}

this.state = 'HAVE_CURRENT_METADATA';
handleMediaupdatetimeout_() {
if (this.state !== 'HAVE_METADATA') {
// only refresh the media playlist if no other activity is going on
return;
}
const media = this.media();

this.request = this.vhs_.xhr({
uri: resolveUrl(this.master.uri, this.media().uri),
withCredentials: this.withCredentials
}, (error, req) => {
// disposed
if (!this.request) {
return;
}
let uri = resolveUrl(this.master.uri, media.uri);

if (error) {
return this.playlistRequestError(this.request, this.media(), 'HAVE_METADATA');
}
if (this.experimentalLLHLS) {
uri = addLLHLSQueryDirectives(uri, media);
}
this.state = 'HAVE_CURRENT_METADATA';

this.haveMetadata({
playlistString: this.request.responseText,
url: this.media().uri,
id: this.media().id
});
this.request = this.vhs_.xhr({
uri,
withCredentials: this.withCredentials
}, (error, req) => {
// disposed
if (!this.request) {
return;
}

if (error) {
return this.playlistRequestError(this.request, this.media(), 'HAVE_METADATA');
}

this.haveMetadata({
playlistString: this.request.responseText,
url: this.media().uri,
id: this.media().id
});
});

}

playlistRequestError(xhr, playlist, startingState) {
Expand All @@ -317,6 +448,17 @@ export default class PlaylistLoader extends EventTarget {
this.trigger('error');
}

parseManifest_({url, manifestString}) {
return parseManifest({
onwarn: ({message}) => this.logger_(`m3u8-parser warn for ${url}: ${message}`),
oninfo: ({message}) => this.logger_(`m3u8-parser info for ${url}: ${message}`),
manifestString,
customTagParsers: this.customTagParsers,
customTagMappers: this.customTagMappers,
experimentalLLHLS: this.experimentalLLHLS
});
}

/**
* Update the playlist loader's state in response to a new or updated playlist.
*
Expand All @@ -334,13 +476,9 @@ export default class PlaylistLoader extends EventTarget {
this.request = null;
this.state = 'HAVE_METADATA';

const playlist = playlistObject || parseManifest({
onwarn: ({message}) => this.logger_(`m3u8-parser warn for ${id}: ${message}`),
oninfo: ({message}) => this.logger_(`m3u8-parser info for ${id}: ${message}`),
manifestString: playlistString,
customTagParsers: this.customTagParsers,
customTagMappers: this.customTagMappers,
experimentalLLHLS: this.experimentalLLHLS
const playlist = playlistObject || this.parseManifest_({
url,
manifestString: playlistString
});

playlist.lastRequest = Date.now();
Expand Down Expand Up @@ -647,11 +785,9 @@ export default class PlaylistLoader extends EventTarget {

this.src = resolveManifestRedirect(this.handleManifestRedirects, this.src, req);

const manifest = parseManifest({
const manifest = this.parseManifest_({
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

move parseManifest to this.parseManifest as we weren't previously passing all the options in both calls.

manifestString: req.responseText,
customTagParsers: this.customTagParsers,
customTagMappers: this.customTagMappers,
experimentalLLHLS: this.experimentalLLHLS
url: this.src
});

this.setupInitialPlaylist(manifest);
Expand Down
Loading