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 12 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
16 changes: 16 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,21 @@ 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) {
let partTargetDuration = (manifest.targetDuration / parts.length);

if (manifest.segments && manifest.segments.length) {
partTargetDuration = parts.reduce((acc, p) => Math.max(acc, p.duration), 0);
}
gkatsev marked this conversation as resolved.
Show resolved Hide resolved

if (onwarn) {
onwarn(`manifest has no partTargetDuration defaulting to ${partTargetDuration}`);
}
manifest.partTargetDuration = partTargetDuration;
}

return manifest;
};

Expand Down
153 changes: 117 additions & 36 deletions src/playlist-loader.js
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,12 @@ export const updateSegment = (a, b) => {
}
}

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

return result;
};

Expand All @@ -69,15 +75,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 i = offset; i < length; i++) {
const newIndex = i - offset;
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];

result[newIndex] = updateSegment(oldSegments[i], result[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;
}

result.push(newSegment);

}
}
return result;
};
Expand Down Expand Up @@ -115,12 +136,22 @@ export const resolveSegmentUris = (segment, baseUri) => {

const getAllSegments = function(media) {
const segments = media.segments || [];
const preloadSegment = 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
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

// 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);
segments.push(preloadSegment);
}

return segments;
Expand All @@ -146,28 +177,36 @@ 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.


media.segments = getAllSegments(media);
const mergedPlaylist = mergeOptions(oldMedia, newMedia);

// 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 this new segment
brandonocasey marked this conversation as resolved.
Show resolved Hide resolved
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 @@ -180,13 +219,13 @@ 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;

return result;
};
Expand Down Expand Up @@ -255,11 +294,48 @@ export default class PlaylistLoader extends EventTarget {
// only refresh the media playlist if no other activity is going on
return;
}
const media = this.media();

let uri = resolveUrl(this.master.uri, media.uri);

if (this.experimentalLLHLS) {
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 is all new, basically use query directives and server side blocking if available.

Copy link
Member

Choose a reason for hiding this comment

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

question: do we want to skip these two server-control checks if we have an end list tag? The spec says that the server MUST ignore these, so, we may as well not do the calculations in those cases, if we don't already.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

sure I will make this an && !endList

const query = [];

if (media.serverControl && media.serverControl.canSkipUntil) {
query.push('_HLS_skip=' + (media.serverControl.canSkipDateranges ? 'v2' : 'YES'));
}

if (media.serverControl && media.serverControl.canBlockReload) {
const preloadSegment = media.preloadSegment;
// next msn will be for preload segment, if we have a preload segment.
let nextMSN = media.mediaSequence + media.segments.length;
let nextPart;

if (preloadSegment && preloadSegment.parts && preloadSegment.parts.length) {
nextPart = preloadSegment.parts.length + 1;
nextMSN--;
} else {
nextMSN++;
}

query.push(`_HLS_msn=${nextMSN}`);

if (nextPart) {
query.push(`_HLS_part=${nextPart}`);
}
}

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

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

}
this.state = 'HAVE_CURRENT_METADATA';

this.request = this.vhs_.xhr({
uri: resolveUrl(this.master.uri, this.media().uri),
uri,
withCredentials: this.withCredentials
}, (error, req) => {
// disposed
Expand Down Expand Up @@ -304,6 +380,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 @@ -321,13 +408,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 @@ -632,11 +715,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 Expand Up @@ -677,7 +758,7 @@ export default class PlaylistLoader extends EventTarget {
manifest.playlists.forEach((playlist) => {
playlist.segments = getAllSegments(playlist);

playlist.segments.forEach((segment) => {
playlist.segments.forEach(function(segment) {
resolveSegmentUris(segment, playlist.resolvedUri);
});
});
Expand Down
9 changes: 7 additions & 2 deletions src/playlist.js
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,12 @@ const getPartsAndSegments = (playlist) => (playlist.segments || []).reduce((acc,
return acc;
}, []);

export const getLastParts = (media) => {
const lastSegment = media.segments && media.segments.length && media.segments[media.segments.length - 1];

return lastSegment && lastSegment.parts || [];
};

/**
* Get the number of seconds to delay from the end of a
* live playlist.
Expand All @@ -47,8 +53,7 @@ export const liveEdgeDelay = (master, media) => {
return master.suggestedPresentationDelay;
}

const lastSegment = media.segments && media.segments.length && media.segments[media.segments.length - 1];
const hasParts = lastSegment && lastSegment.parts && lastSegment.parts.length;
const hasParts = getLastParts(media).length > 0;

// look for "part" delays from ll-hls first
if (hasParts && media.serverControl && media.serverControl.partHoldBack) {
Expand Down
Loading