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 8 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
4 changes: 2 additions & 2 deletions src/master-playlist-controller.js
Original file line number Diff line number Diff line change
Expand Up @@ -1303,7 +1303,7 @@ export class MasterPlaylistController extends videojs.EventTarget {
const mainSeekable = Vhs.Playlist.seekable(
media,
expired,
Vhs.Playlist.getHoldBack(master, media)
Vhs.Playlist.liveEdgeDelay(master, media)
);

if (mainSeekable.length === 0) {
Expand All @@ -1321,7 +1321,7 @@ export class MasterPlaylistController extends videojs.EventTarget {
audioSeekable = Vhs.Playlist.seekable(
media,
expired,
Vhs.Playlist.getHoldBack(master, media)
Vhs.Playlist.liveEdgeDelay(master, media)
);

if (audioSeekable.length === 0) {
Expand Down
30 changes: 16 additions & 14 deletions src/playlist-loader.js
Original file line number Diff line number Diff line change
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,14 +160,7 @@ export const updateMaster = (master, media, unchangedCheck = isPlaylistUnchanged

const mergedPlaylist = mergeOptions(playlist, media);

media.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) {
media.segments.push(media.preloadSegment);
}
media.segments = getAllSegments(media);

// if the update could overlap existing segment information, merge the two segment lists
if (playlist.segments) {
Expand Down Expand Up @@ -197,7 +203,7 @@ export const updateMaster = (master, media, unchangedCheck = isPlaylistUnchanged
*/
export const refreshDelay = (media, update) => {
const lastSegment = media.segments[media.segments.length - 1];
const lastPart = lastSegment && lastSegment.parts && lastSegment.parts[lastSegment.parts - 1];
const lastPart = lastSegment && lastSegment.parts && lastSegment.parts[lastSegment.parts.length - 1];
const lastDuration = lastPart && lastPart.duration || lastSegment && lastSegment.duration;

if (update && lastDuration) {
Expand Down Expand Up @@ -669,11 +675,7 @@ 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) => {
playlist.segments = playlist.segments || [];

if (playlist.preloadSegment && playlist.preloadSegment.parts) {
playlist.segments.push(playlist.preloadSegment);
}
playlist.segments = getAllSegments(playlist);

playlist.segments.forEach((segment) => {
resolveSegmentUris(segment, playlist.resolvedUri);
Expand Down
62 changes: 35 additions & 27 deletions src/playlist.js
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ const {createTimeRange} = videojs;
*
* @return {Array} The part/segment list.
*/
const getPartSegments = (playlist) => (playlist.segments || []).reduce((acc, segment, si) => {
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});
Expand All @@ -30,34 +30,39 @@ const getPartSegments = (playlist) => (playlist.segments || []).reduce((acc, seg
}, []);

/**
* Get the number of seconds to hold back from the end of a
* 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 getHoldBack = (master, media) => {
export const liveEdgeDelay = (master, media) => {
if (media.endList) {
return 0;
}

const partSegments = getPartSegments(media);
const hasParts = partSegments.length &&
typeof partSegments[partSegments.length - 1].partIndex === 'number';
const partsAndSegments = getPartsAndSegments(media);
const hasParts = partsAndSegments.length &&
typeof partsAndSegments[partsAndSegments.length - 1].partIndex === 'number';

// get the last three part/segment durations
let lastThreeDurations = 0;

if (partSegments.length >= 3) {
for (let i = 0; i < 3; i++) {
lastThreeDurations += partSegments[partSegments.length - 1 - i].duration;
if (partsAndSegments.length >= 3) {
for (let i = partsAndSegments.length - 1; i > partsAndSegments.length - 4; i--) {
brandonocasey marked this conversation as resolved.
Show resolved Hide resolved
// segment missing a duration, we cannot calculate
if (!partsAndSegments[i].duration) {
lastThreeDurations = 0;
break;
}
lastThreeDurations += partsAndSegments[i].duration;
}
}

// dash suggestedPresentationDelay trumps everything
if (master && master.suggestedPresentationDelay) {
return master.suggestedPresentationDelay;

// look for "part" delays from ll-hls first
} else if (hasParts && media.serverControl && media.serverControl.partHoldBack) {
return media.serverControl.partHoldBack;
Expand All @@ -72,10 +77,13 @@ export const getHoldBack = (master, media) => {
} 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.
const lastPartSegment = partSegments[partSegments.length - 1];
const lastPartSegment = partsAndSegments[partsAndSegments.length - 1];
const lastPartDuration = lastPartSegment && lastPartSegment.duration || media.targetDuration;

return lastPartDuration + media.targetDuration * 2;
// we shouldn't ever end up using lastThreeDurations as targetDuration
// is usually required, but if we somehow get here, with a missing
// targetDuration we should handle it
} else if (lastThreeDurations) {
return lastThreeDurations;
}
Copy link
Contributor

@gesinger gesinger Mar 19, 2021

Choose a reason for hiding this comment

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

Since we shouldn't end up here, it's probably worth pulling all of that logic down into an else here and return either 0 or whatever we calculate as the last three. Might also be worth warning (if we don't warn elsewhere for missing target duration), or logging anyway to help us determine what's happening in the event of a missing target duration.

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 lastThreeDurations in the case where we have parts, but no partTargetDuration so that we get a more accurate hold back for ll-hls. We warn about target duration via the manifest parser warnings that now go to the debug log.

Expand Down Expand Up @@ -307,24 +315,24 @@ export const sumDurations = function(playlist, startIndex, endIndex) {
* @function safeLiveIndex
*/
export const safeLiveIndex = function(playlist, liveEdgePadding) {
const partSegments = getPartSegments(playlist);
const partsAndSegments = getPartsAndSegments(playlist);

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

if (typeof liveEdgePadding !== 'number') {
liveEdgePadding = getHoldBack(null, playlist);
liveEdgePadding = liveEdgeDelay(null, playlist);
}

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

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

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

Expand Down Expand Up @@ -420,15 +428,15 @@ export const getMediaInfoForTime = function(
startTime
) {

const partSegments = getPartSegments(playlist);
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 (let i = startIndex - 1; i >= 0; i--) {
const segment = partSegments[i];
const segment = partsAndSegments[i];

time += (segment.duration + TIME_FUDGE_FACTOR);

Expand All @@ -445,8 +453,8 @@ export const getMediaInfoForTime = function(
// We were unable to find a good segment within the playlist
// so select the first segment
return {
mediaIndex: partSegments[0] && partSegments[0].segmentIndex || 0,
partIndex: partSegments[0] && partSegments[0].partIndex || null,
mediaIndex: partsAndSegments[0] && partsAndSegments[0].segmentIndex || 0,
partIndex: partsAndSegments[0] && partsAndSegments[0].partIndex || null,
startTime: currentTime
};
}
Expand All @@ -459,7 +467,7 @@ export const getMediaInfoForTime = function(
time -= playlist.targetDuration;
if (time < 0) {
return {
mediaIndex: partSegments[0].segmentIndex,
mediaIndex: partsAndSegments[0].segmentIndex,
startTime: currentTime
};
}
Expand All @@ -469,8 +477,8 @@ 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 (let i = startIndex; i < partSegments.length; i++) {
const partSegment = partSegments[i];
for (let i = startIndex; i < partsAndSegments.length; i++) {
const partSegment = partsAndSegments[i];

time -= partSegment.duration + TIME_FUDGE_FACTOR;

Expand All @@ -485,8 +493,8 @@ export const getMediaInfoForTime = function(

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

// exports
export default {
getHoldBack,
liveEdgeDelay,
duration,
seekable,
safeLiveIndex,
Expand Down
2 changes: 1 addition & 1 deletion src/segment-loader.js
Original file line number Diff line number Diff line change
Expand Up @@ -112,7 +112,7 @@ const segmentInfoString = (segmentInfo) => {
return [
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Better logging in general, but especially needed now that we have parts.

`${name} [${index}/${segments.length - 1}]`,
(partIndex ? `part [${partIndex}/${parts.length - 1}]` : ''),
`msn [${seq}/${seq + segments.length - 1}]`,
`mediaSequenceNumber [${seq}/${seq + segments.length - 1}]`,
`playlist [${id}]`,
`start/end [${start} => ${end}]`,
`timeline [${timeline}]`
Expand Down
50 changes: 34 additions & 16 deletions test/playlist.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -575,7 +575,7 @@ QUnit.test('safeLiveIndex accounts for liveEdgePadding in simple case', function

assert.equal(
Playlist.safeLiveIndex(playlist, 0), 5,
'returns 6 when liveEdgePadding is 0 and duration is 6'
'returns 5 when liveEdgePadding is 0 and duration is 6'
);
});

Expand Down Expand Up @@ -651,11 +651,11 @@ QUnit.test('safeLiveIndex accounts for liveEdgePadding in non-simple case', func

assert.equal(
Playlist.safeLiveIndex(playlist, 0), 5,
'returns 6 when liveEdgePadding is 0'
'returns 5 when liveEdgePadding is 0'
);
});

QUnit.test('getHoldBack works as expected', function(assert) {
QUnit.test('liveEdgeDelay works as expected', function(assert) {
const media = {
endList: true,
targetDuration: 5,
Expand All @@ -665,13 +665,17 @@ QUnit.test('getHoldBack works as expected', function(assert) {
partHoldBack: 2
},
segments: [
{duration: 4},
{duration: 3},
{duration: 4, parts: [
{duration: 1},
{duration: 0.5}
]},
{duration: 3, parts: [
{duration: 1},
{duration: 0.5}
]},
{duration: 4, parts: [
{duration: 1},
{duration: 0.5},
{duration: 0.5}
]}
]
Expand All @@ -681,66 +685,80 @@ QUnit.test('getHoldBack works as expected', function(assert) {
};

assert.equal(
Playlist.getHoldBack(master, media),
Playlist.liveEdgeDelay(master, media),
0,
'returns 0 with endlist'
);

delete media.endList;
assert.equal(
Playlist.getHoldBack(master, media),
Playlist.liveEdgeDelay(master, media),
master.suggestedPresentationDelay,
'uses suggestedPresentationDelay'
);

delete master.suggestedPresentationDelay;
assert.equal(
Playlist.getHoldBack(master, media),
Playlist.liveEdgeDelay(master, media),
media.serverControl.partHoldBack,
'uses part hold back'
);

media.serverControl.partHoldBack = null;
assert.equal(
Playlist.getHoldBack(master, media),
Playlist.liveEdgeDelay(master, media),
media.partTargetDuration * 3,
'uses part target duration * 3'
);

media.partTargetDuration = null;
assert.equal(
Playlist.getHoldBack(master, media),
Playlist.liveEdgeDelay(master, media),
2,
'uses last three part durations'
);

Copy link
Contributor

Choose a reason for hiding this comment

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

May want another test here with fewer than 3 parts available.

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 a real scenario the last 3 segments will always have >= 2 parts. as having only one part is the same as not using parts at all. I will update the test for that.

media.segments[media.segments.length - 1].parts = null;
media.segments = media.segments.map((s) => {
s.parts = null;
return s;
});
assert.equal(
Playlist.getHoldBack(master, media),
Playlist.liveEdgeDelay(master, media),
media.serverControl.holdBack,
'uses hold back'
);

media.serverControl.holdBack = null;
assert.equal(
Playlist.getHoldBack(master, media),
Playlist.liveEdgeDelay(master, media),
(media.targetDuration * 2) + media.segments[media.segments.length - 1].duration,
'uses (targetDuration * 2) + last segment duration'
);

media.targetDuration = null;
assert.equal(
Playlist.getHoldBack(master, media),
Playlist.liveEdgeDelay(master, media),
11,
'uses last three segment durations'
);

media.segments = media.segments.map((s) => {
s.duration = null;
return s;
});

assert.equal(
Playlist.liveEdgeDelay(master, media),
0,
'no segment durations live delay can be calculated'
brandonocasey marked this conversation as resolved.
Show resolved Hide resolved
);

media.segments.length = 0;

assert.equal(
Playlist.getHoldBack(master, media),
Playlist.liveEdgeDelay(master, media),
0,
'no possible holdback can be calculated'
'no segments live delay can be calculated'
brandonocasey marked this conversation as resolved.
Show resolved Hide resolved
);
});

Expand Down