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 serverControl and preloadSegment llhls features behind a flag #1078

Merged
merged 30 commits into from
Apr 5, 2021
Merged
Show file tree
Hide file tree
Changes from 22 commits
Commits
Show all changes
30 commits
Select commit Hold shift + click to select a range
123aba7
feat: Add ll-hls ext-x-part support behind a flag
brandonocasey Jan 26, 2021
32de23e
code review
brandonocasey Mar 1, 2021
3f8ad12
change option from llhls -> experimentalLLHLS
brandonocasey Mar 2, 2021
9c9493a
update to camelcase
brandonocasey Mar 2, 2021
73278fa
cr
brandonocasey Mar 3, 2021
e7598c6
separate updateSegment function
brandonocasey Mar 3, 2021
d3236ca
m3u8-parser update
brandonocasey Mar 4, 2021
c2ebf7b
test fixes with m3u8-parser update
brandonocasey Mar 4, 2021
a8957ff
ie 11 fix
brandonocasey Mar 5, 2021
2ce6e59
feat(llhls): serverControl, preloadSegment, and partTargetDuration
brandonocasey Feb 10, 2021
0fa6dee
getHoldBack -> liveEdgeDelay
brandonocasey Mar 17, 2021
b4e219c
use part length
brandonocasey Mar 17, 2021
a58bcef
getAllSegments helper
brandonocasey Mar 17, 2021
83bf171
partSegments -> partsAndSegments
brandonocasey Mar 17, 2021
43720f0
msn -> mediaSequenceNumber
brandonocasey Mar 17, 2021
6ce8c1e
cr add comments
brandonocasey Mar 18, 2021
c3bef56
test text update
brandonocasey Mar 18, 2021
5d78a1f
better test
brandonocasey Mar 18, 2021
467832f
Merge remote-tracking branch 'origin/main' into feat/llhls-2
brandonocasey Mar 22, 2021
d35fa4d
code review
brandonocasey Mar 22, 2021
4374c0e
remove last three durations logic
brandonocasey Mar 25, 2021
e80c0fb
Merge branch 'main' into feat/llhls-2
brandonocasey Mar 29, 2021
b77c94c
Update src/playlist.js
brandonocasey Mar 30, 2021
dffa291
Merge remote-tracking branch 'origin/main' into feat/llhls-2
brandonocasey Mar 30, 2021
3aa8582
code review
brandonocasey Mar 30, 2021
1aa7785
null check
brandonocasey Mar 30, 2021
b381d96
Merge branch 'main' into feat/llhls-2
brandonocasey Mar 30, 2021
088ee6e
set to largest duration, add test
brandonocasey Apr 5, 2021
d7c30d2
Update test/manifest.test.js
brandonocasey Apr 5, 2021
01a6763
Merge remote-tracking branch 'origin/main' into feat/llhls-2
brandonocasey Apr 5, 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
14 changes: 11 additions & 3 deletions src/master-playlist-controller.js
Original file line number Diff line number Diff line change
Expand Up @@ -1299,8 +1299,12 @@ export class MasterPlaylistController extends videojs.EventTarget {
return;
}

const suggestedPresentationDelay = this.masterPlaylistLoader_.master.suggestedPresentationDelay;
const mainSeekable = Vhs.Playlist.seekable(media, expired, suggestedPresentationDelay);
const master = this.masterPlaylistLoader_.master;
const mainSeekable = Vhs.Playlist.seekable(
media,
expired,
Vhs.Playlist.liveEdgeDelay(master, media)
);

if (mainSeekable.length === 0) {
return;
Expand All @@ -1314,7 +1318,11 @@ export class MasterPlaylistController extends videojs.EventTarget {
return;
}

audioSeekable = Vhs.Playlist.seekable(media, expired, suggestedPresentationDelay);
audioSeekable = Vhs.Playlist.seekable(
media,
expired,
Vhs.Playlist.liveEdgeDelay(master, media)
);

if (audioSeekable.length === 0) {
return;
Expand Down
51 changes: 33 additions & 18 deletions src/playlist-loader.js
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,7 @@ export const updateSegments = (original, update, offset) => {
};

export const resolveSegmentUris = (segment, baseUri) => {
// preloadSegments will not have a uri at all
// preloadSegment will not have a uri at all
// as the segment isn't actually in the manifest yet, only parts
if (!segment.resolvedUri && segment.uri) {
segment.resolvedUri = resolveUrl(baseUri, segment.uri);
Expand Down Expand Up @@ -113,6 +113,19 @@ export const resolveSegmentUris = (segment, baseUri) => {
}
};

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

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

return segments;
};

// consider the playlist unchanged if the playlist object is the same or
// the number of segments is equal, the media sequence number is unchanged,
// and this playlist hasn't become the end of the playlist
Expand Down Expand Up @@ -147,6 +160,8 @@ export const updateMaster = (master, media, unchangedCheck = isPlaylistUnchanged

const mergedPlaylist = mergeOptions(playlist, media);

media.segments = getAllSegments(media);

// if the update could overlap existing segment information, merge the two segment lists
if (playlist.segments) {
mergedPlaylist.segments = updateSegments(
Expand Down Expand Up @@ -188,16 +203,16 @@ export const updateMaster = (master, media, unchangedCheck = isPlaylistUnchanged
*/
export const refreshDelay = (media, update) => {
const lastSegment = media.segments[media.segments.length - 1];
let delay;

if (update && lastSegment && lastSegment.duration) {
delay = lastSegment.duration * 1000;
} else {
// if the playlist is unchanged since the last reload or last segment duration
// cannot be determined, try again after half the target duration
delay = (media.targetDuration || 10) * 500;
const lastPart = lastSegment && lastSegment.parts && lastSegment.parts[lastSegment.parts.length - 1];
const lastDuration = lastPart && lastPart.duration || lastSegment && lastSegment.duration;

if (update && lastDuration) {
return lastDuration * 1000;
}
return delay;

// if the playlist is unchanged since the last reload or last segment duration
// cannot be determined, try again after half the target duration
return (media.partTargetDuration || media.targetDuration || 10) * 500;
Copy link
Contributor Author

Choose a reason for hiding this comment

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

use partTargetDuration or targetDuration

};

/**
Expand Down Expand Up @@ -326,7 +341,7 @@ export default class PlaylistLoader extends EventTarget {
// merge this playlist into the master
const update = updateMaster(this.master, playlist);

this.targetDuration = playlist.targetDuration;
this.targetDuration = playlist.partTargetDuration || playlist.targetDuration;
Copy link
Contributor Author

Choose a reason for hiding this comment

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

We use a playlists targetDuration in a few places outside of these files. In the case where we have partTargetDuration that is always what we will use as out "true" targetDuration, I understand that this is ambiguous though and I think we might want to rename this.targetDuration to something else, but that might be a breaking change. Should we deprecate this name?


if (update) {
this.master = update;
Expand Down Expand Up @@ -405,7 +420,7 @@ export default class PlaylistLoader extends EventTarget {
window.clearTimeout(this.finalRenditionTimeout);

if (shouldDelay) {
const delay = (playlist.targetDuration / 2) * 1000 || 5 * 1000;
const delay = ((playlist.partTargetDuration || playlist.targetDuration) / 2) * 1000 || 5 * 1000;

this.finalRenditionTimeout =
window.setTimeout(this.media.bind(this, playlist, false), delay);
Expand Down Expand Up @@ -538,7 +553,7 @@ export default class PlaylistLoader extends EventTarget {
const media = this.media();

if (shouldDelay) {
const delay = media ? (media.targetDuration / 2) * 1000 : 5 * 1000;
const delay = media ? ((media.partTargetDuration || media.targetDuration) / 2) * 1000 : 5 * 1000;

this.mediaUpdateTimeout = window.setTimeout(() => this.load(), delay);
return;
Expand Down Expand Up @@ -660,11 +675,11 @@ export default class PlaylistLoader extends EventTarget {
// then resolve URIs in advance, as they are usually done after a playlist request,
// which may not happen if the playlist is resolved.
manifest.playlists.forEach((playlist) => {
if (playlist.segments) {
playlist.segments.forEach((segment) => {
resolveSegmentUris(segment, playlist.resolvedUri);
});
}
playlist.segments = getAllSegments(playlist);

playlist.segments.forEach((segment) => {
resolveSegmentUris(segment, playlist.resolvedUri);
});
});
this.trigger('loadedplaylist');
if (!this.request) {
Expand Down
126 changes: 97 additions & 29 deletions src/playlist.js
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,68 @@ import {TIME_FUDGE_FACTOR} from './ranges.js';

const {createTimeRange} = videojs;

/**
* A function to get a combined list of parts and segments with durations
* and indexes.
*
* @param {Playlist} playlist the playlist to get the list for.
*
* @return {Array} The part/segment list.
*/
const getPartsAndSegments = (playlist) => (playlist.segments || []).reduce((acc, segment, si) => {
if (segment.parts) {
segment.parts.forEach(function(part, pi) {
acc.push({duration: part.duration, segmentIndex: si, partIndex: pi});

brandonocasey marked this conversation as resolved.
Show resolved Hide resolved
});
} else {
acc.push({duration: segment.duration, segmentIndex: si, partIndex: null});
}
return acc;
}, []);

/**
* Get the number of seconds to delay from the end of a
* live playlist.
*
* @param {Playlist} master the master playlist
* @param {Playlist} media the media playlist
* @return {number} the hold back in seconds.
*/
export const liveEdgeDelay = (master, media) => {
if (media.endList) {
return 0;
}

const lastSegment = media.segments && media.segments.length && media.segments[media.segments.length - 1];
const hasParts = lastSegment && lastSegment.parts && lastSegment.parts.length;
// by default we use the last three durations of segments if
// part target duration or target duration isn't found.
// see: https://tools.ietf.org/html/draft-pantos-hls-rfc8216bis-08#section-4.4.3.8
Copy link
Contributor

Choose a reason for hiding this comment

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

I may be missing something, but I don't think that section of the spec, if referring to HOLD-BACK, matches the comment here, since that is saying to use part target duration or target duration when the parameter is not available, and doesn't refer to the use of segment durations. Do we just have this in case the required target durations are missing?

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 we use three segment durations in case target durations are not available for whatever reason.

Copy link
Contributor

Choose a reason for hiding this comment

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

I'm wondering if it's worth it to add the logic for this here if those are required attributes. It adds logic to the code that we shouldn't really encounter, and if we were to, then it's something that should be resolved by the creator of the stream, since target duration is always required (https://tools.ietf.org/html/draft-pantos-hls-rfc8216bis-08#section-4.4.3.1) and part target duration is required when parts are used (https://tools.ietf.org/html/draft-pantos-hls-rfc8216bis-08#section-4.4.3.7).

I think if we're trying to account for missing target durations we should do that at a higher level, when we first parse the manifest. It would help to simplify the code.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

ok I will remove the last three durations logic.

Copy link
Member

Choose a reason for hiding this comment

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

What about just default to 10s target duration in case somehow it isn't set or whatever? While I agree that bad streams are bad, we should still try to play as much as possible and having a default, if bad value, is worth it. Plus, it should make things simpler than trying to figure out segment duration and using that for target duation.

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 having a sort of default may be appropriate (along with a warning). We should probably add it at the point where we first parse the stream (i.e., in the loaders).

brandonocasey marked this conversation as resolved.
Show resolved Hide resolved

// dash suggestedPresentationDelay trumps everything
if (master && master.suggestedPresentationDelay) {
return master.suggestedPresentationDelay;
Comment on lines +44 to +46
Copy link
Contributor

Choose a reason for hiding this comment

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

Might be worth pulling this up to short circuit the rest of the function.

// look for "part" delays from ll-hls first
} else if (hasParts && media.serverControl && media.serverControl.partHoldBack) {
return media.serverControl.partHoldBack;
Copy link
Contributor

Choose a reason for hiding this comment

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

Same with this, might be easier to read if exiting before going through other logic and calculations.

} else if (hasParts && media.partTargetDuration) {
return media.partTargetDuration * 3;

// finally look for full segment delays
} else if (media.serverControl && media.serverControl.holdBack) {
return media.serverControl.holdBack;
} else if (media.targetDuration) {
brandonocasey marked this conversation as resolved.
Show resolved Hide resolved
// TODO: this should probably be targetDuration * 3
// but we use this for backwards compatability.
Copy link
Contributor

Choose a reason for hiding this comment

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

Probably worth adding to next major to change this.

const lastPartDuration = lastSegment && lastSegment.duration || media.targetDuration;

return lastPartDuration + media.targetDuration * 2;
}

return 0;
};

/**
* walk backward until we find a duration we can use
* or return a failure
Expand Down Expand Up @@ -233,31 +295,29 @@ export const sumDurations = function(playlist, startIndex, endIndex) {
* @function safeLiveIndex
*/
export const safeLiveIndex = function(playlist, liveEdgePadding) {
if (!playlist.segments.length) {
const partsAndSegments = getPartsAndSegments(playlist);

if (!partsAndSegments.length) {
return 0;
}

let i = playlist.segments.length;
const lastSegmentDuration = playlist.segments[i - 1].duration || playlist.targetDuration;
const safeDistance = typeof liveEdgePadding === 'number' ?
liveEdgePadding :
lastSegmentDuration + playlist.targetDuration * 2;

if (safeDistance === 0) {
return i;
if (typeof liveEdgePadding !== 'number') {
liveEdgePadding = liveEdgeDelay(null, playlist);
}

let i = partsAndSegments.length;
let distanceFromEnd = 0;

while (i--) {
distanceFromEnd += playlist.segments[i].duration;
distanceFromEnd += partsAndSegments[i].duration;

if (distanceFromEnd >= safeDistance) {
break;
if (distanceFromEnd >= liveEdgePadding) {
return partsAndSegments[i].segmentIndex;
}
}

return Math.max(0, i);
// there is nowhere in the playlist that is a safe distance from live.
return 0;
};

/**
Expand Down Expand Up @@ -347,31 +407,34 @@ export const getMediaInfoForTime = function(
startIndex,
startTime
) {
let i;
let segment;
const numSegments = playlist.segments.length;

const partsAndSegments = getPartsAndSegments(playlist);
let time = currentTime - startTime;

if (time < 0) {
// Walk backward from startIndex in the playlist, adding durations
// until we find a segment that contains `time` and return it
if (startIndex > 0) {
for (i = startIndex - 1; i >= 0; i--) {
segment = playlist.segments[i];
for (let i = startIndex - 1; i >= 0; i--) {
const segment = partsAndSegments[i];

time += (segment.duration + TIME_FUDGE_FACTOR);

if (time > 0) {
return {
mediaIndex: i,
startTime: startTime - sumDurations(playlist, startIndex, i)
mediaIndex: segment.segmentIndex,
startTime: startTime - sumDurations(playlist, startIndex, segment.segmentIndex),
partIndex: segment.partIndex
};
}
}
}

// We were unable to find a good segment within the playlist
// so select the first segment
return {
mediaIndex: 0,
mediaIndex: partsAndSegments[0] && partsAndSegments[0].segmentIndex || 0,
partIndex: partsAndSegments[0] && partsAndSegments[0].partIndex || null,
startTime: currentTime
};
}
Expand All @@ -380,11 +443,11 @@ export const getMediaInfoForTime = function(
// adding target durations. If we "run out of time" before getting to
// the first segment, return the first segment
if (startIndex < 0) {
for (i = startIndex; i < 0; i++) {
for (let i = startIndex; i < 0; i++) {
time -= playlist.targetDuration;
if (time < 0) {
return {
mediaIndex: 0,
mediaIndex: partsAndSegments[0].segmentIndex,
startTime: currentTime
};
}
Expand All @@ -394,20 +457,24 @@ export const getMediaInfoForTime = function(

// Walk forward from startIndex in the playlist, subtracting durations
// until we find a segment that contains `time` and return it
for (i = startIndex; i < numSegments; i++) {
segment = playlist.segments[i];
time -= segment.duration + TIME_FUDGE_FACTOR;
for (let i = startIndex; i < partsAndSegments.length; i++) {
const partSegment = partsAndSegments[i];

time -= partSegment.duration + TIME_FUDGE_FACTOR;

if (time < 0) {
return {
mediaIndex: i,
startTime: startTime + sumDurations(playlist, startIndex, i)
mediaIndex: partSegment.segmentIndex,
startTime: startTime + sumDurations(playlist, startIndex, partSegment.segmentIndex),
partIndex: partSegment.partIndex
};
}
}

// We are out of possible candidates so load the last one...
return {
mediaIndex: numSegments - 1,
mediaIndex: partsAndSegments[partsAndSegments.length - 1].segmentIndex,
partIndex: partsAndSegments[partsAndSegments.length - 1].partIndex,
startTime: currentTime
};
};
Expand Down Expand Up @@ -543,6 +610,7 @@ export const isLowestEnabledRendition = (master, media) => {

// exports
export default {
liveEdgeDelay,
duration,
seekable,
safeLiveIndex,
Expand Down
Loading