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 15 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
180 changes: 148 additions & 32 deletions src/playlist-loader.js
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ import {
masterForMedia,
setupMediaPlaylist
} from './manifest';
import {getKnownPartCount} from './playlist.js';

const { mergeOptions, EventTarget } = videojs;

Expand All @@ -34,6 +35,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 @@ -49,6 +56,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 @@ -69,15 +88,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];

for (let i = offset; i < length; i++) {
const newIndex = i - offset;
if (oldSegment) {
currentMap = oldSegment.map || currentMap;

result[newIndex] = updateSegment(oldSegments[i], result[newIndex]);
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 +149,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 @@ -146,28 +195,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.


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

// always use the new medias preload segment.
brandonocasey marked this conversation as resolved.
Show resolved Hide resolved
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 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 +242,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 +317,60 @@ 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.canBlockReload) {
const {preloadSegment} = media;
// next msn is a zero based value, length is not.
let nextMSN = media.mediaSequence + media.segments.length;

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}`);
}

// if we are requesting a nextPart or preload segment was added
// to our segment list. We are requesting a part of the preload segment
// or the full preload segment. Either way we need to go down by 1
// in nextMSN
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this because the nextMSN would be (and normally is, outside of this case) reflecting the predicted next media sequence number, rather than one that's reported in the playlist (the preload segment)?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes its based on sequence number and has to take into account weather we our requesting something that was a preload segment, a part of a current segment, or the next segment after what we currently have.

Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe we can add those notes as a comment?

if (nextPart > -1 || parts.length) {
nextMSN--;
}
}

// add _HLS_msn= infront of any _HLS_part query
brandonocasey marked this conversation as resolved.
Show resolved Hide resolved
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'));
gesinger marked this conversation as resolved.
Show resolved Hide resolved
}

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 +415,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 +443,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 +750,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
21 changes: 19 additions & 2 deletions src/playlist.js
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,24 @@ 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 || [];
};

export const getKnownPartCount = ({preloadSegment}) => {
if (!preloadSegment) {
return;
}
const {parts, preloadHints} = preloadSegment;
let partCount = (preloadHints || [])
.reduce((acc, hint) => acc + (hint.type === 'PART' ? 1 : 0), 0);
brandonocasey marked this conversation as resolved.
Show resolved Hide resolved

partCount += (parts && parts.length) ? parts.length : 0;

return partCount;
};
/**
* Get the number of seconds to delay from the end of a
* live playlist.
Expand All @@ -47,8 +65,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
13 changes: 6 additions & 7 deletions src/segment-loader.js
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ import shallowEqual from './util/shallow-equal.js';
import { QUOTA_EXCEEDED_ERR } from './error-codes';
import { timeRangesToArray } from './ranges';
import {lastBufferedEnd} from './ranges.js';
import {getKnownPartCount} from './playlist.js';

/**
* The segment loader has no recourse except to fetch a segment in the
Expand Down Expand Up @@ -135,11 +136,7 @@ const segmentInfoString = (segmentInfo) => {
const {
startOfSegment,
duration,
segment: {
start,
end,
parts
},
segment,
playlist: {
mediaSequence: seq,
id,
Expand All @@ -158,11 +155,13 @@ const segmentInfoString = (segmentInfo) => {
} else if (segmentInfo.isSyncRequest) {
selection = 'getSyncSegmentCandidate (isSyncRequest)';
}

const {start, end} = segment;
const hasPartIndex = typeof partIndex === 'number';
const name = segmentInfo.segment.uri ? 'segment' : 'pre-segment';
const totalParts = hasPartIndex ? getKnownPartCount({preloadSegment: segment}) - 1 : 0;

return `${name} [${index}/${segmentLen}]` +
(partIndex ? ` part [${partIndex}/${parts.length - 1}]` : '') +
(hasPartIndex ? ` part [${partIndex}/${totalParts}]` : '') +
` mediaSequenceNumber [${seq}/${seq + segmentLen}]` +
` start/end [${start} => ${end}]` +
` startOfSegment [${startOfSegment}]` +
Expand Down
Loading