Skip to content

Commit

Permalink
code review
Browse files Browse the repository at this point in the history
  • Loading branch information
@brandonocasey
brandonocasey committed May 26, 2021
1 parent 99dd646 commit ac9de31
Show file tree
Hide file tree
Showing 5 changed files with 52 additions and 58 deletions.
12 changes: 6 additions & 6 deletions src/playlist.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -372,16 +372,16 @@ export const seekable = function(playlist, expired, liveEdgePadding) {
* @param {number} options.currentTime The number of seconds since the earliest
* possible position to determine the containing segment for
* @param {number} options.startTime the time when the segment/part starts
* @param {number} options.segmentIndex the segment index to start looking at.
* @param {number?} [options.partIndex] the part index to look at within the segment.
* @param {number} options.startingSegmentIndex the segment index to start looking at.
* @param {number?} [options.startingPartIndex] the part index to look at within the segment.
*
* @return {Object} an object with partIndex, segmentIndex, and startTime.
*/
export const getMediaInfoForTime = function({
playlist,
currentTime,
segmentIndex,
partIndex,
startingSegmentIndex,
startingPartIndex,
startTime
}) {

Expand All @@ -393,12 +393,12 @@ export const getMediaInfoForTime = function({
for (let i = 0; i < partsAndSegments.length; i++) {
const partAndSegment = partsAndSegments[i];

if (segmentIndex !== partAndSegment.segmentIndex) {
if (startingSegmentIndex !== partAndSegment.segmentIndex) {
continue;
}

// skip this if part index does not match.
if (typeof partIndex === 'number' && typeof partAndSegment.partIndex === 'number' && partIndex !== partAndSegment.partIndex) {
if (typeof startingPartIndex === 'number' && typeof partAndSegment.partIndex === 'number' && startingPartIndex !== partAndSegment.partIndex) {
continue;
}

Expand Down
4 changes: 2 additions & 2 deletions src/segment-loader.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1399,8 +1399,8 @@ export default class SegmentLoader extends videojs.EventTarget {
const {segmentIndex, startTime, partIndex} = Playlist.getMediaInfoForTime({
playlist: this.playlist_,
currentTime: this.fetchAtBuffer_ ? bufferedEnd : this.currentTime_(),
partIndex: this.syncPoint_.partIndex,
segmentIndex: this.syncPoint_.segmentIndex,
startingPartIndex: this.syncPoint_.partIndex,
startingSegmentIndex: this.syncPoint_.segmentIndex,
startTime: this.syncPoint_.time
});

Expand Down
4 changes: 4 additions & 0 deletions src/sync-controller.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,8 @@ export const syncPointStrategies = [

currentTime = currentTime || 0;
for (let i = 0; i < partsAndSegments.length; i++) {
// start from the end and loop backwards for live
// or start from the front and loop forwards for non-live
const index = (playlist.endList || currentTime === 0) ? i : partsAndSegments.length - (i + 1);
const partAndSegment = partsAndSegments[index];
const segment = partAndSegment.segment;
Expand Down Expand Up @@ -89,6 +91,8 @@ export const syncPointStrategies = [
const partsAndSegments = getPartsAndSegments(playlist);

for (let i = 0; i < partsAndSegments.length; i++) {
// start from the end and loop backwards for live
// or start from the front and loop forwards for non-live
const index = (playlist.endList || currentTime === 0) ? i : partsAndSegments.length - (i + 1);
const partAndSegment = partsAndSegments[index];
const segment = partAndSegment.segment;
Expand Down
70 changes: 35 additions & 35 deletions test/playlist.test.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1001,8 +1001,8 @@ QUnit.module('Playlist', function() {
this.defaults = {
playlist: media,
currentTime: -1,
segmentIndex: 0,
partIndex: null,
startingSegmentIndex: 0,
startingPartIndex: null,
startTime: 0
};

Expand Down Expand Up @@ -1067,8 +1067,8 @@ QUnit.module('Playlist', function() {
this.defaults = {
playlist: media,
currentTime: 2.1,
segmentIndex: 0,
partIndex: null,
startingSegmentIndex: 0,
startingPartIndex: null,
startTime: 0
};

Expand All @@ -1079,61 +1079,61 @@ QUnit.module('Playlist', function() {
'1 away 2 is correct'
);
assert.deepEqual(
this.getMediaInfoForTime({currentTime: 4.1, segmentIndex: 1, startTime: 2}),
this.getMediaInfoForTime({currentTime: 4.1, startingSegmentIndex: 1, startTime: 2}),
{segmentIndex: 2, startTime: 4, partIndex: null},
'1 away 3 is correct'
);
assert.deepEqual(
this.getMediaInfoForTime({currentTime: 6.1, segmentIndex: 2, startTime: 4}),
this.getMediaInfoForTime({currentTime: 6.1, startingSegmentIndex: 2, startTime: 4}),
{segmentIndex: 3, startTime: 6, partIndex: null},
'1 away 4 is correct'
);
assert.deepEqual(
this.getMediaInfoForTime({currentTime: 8.1, segmentIndex: 3, startTime: 6}),
this.getMediaInfoForTime({currentTime: 8.1, startingSegmentIndex: 3, startTime: 6}),
{segmentIndex: 4, startTime: 8, partIndex: null},
'1 away 5 is correct'
);
assert.deepEqual(
this.getMediaInfoForTime({currentTime: 10.1, segmentIndex: 4, startTime: 8}),
this.getMediaInfoForTime({currentTime: 10.1, startingSegmentIndex: 4, startTime: 8}),
{segmentIndex: 5, startTime: 10, partIndex: null},
'1 away 6 is correct'
);

// 2 segments away
assert.deepEqual(
this.getMediaInfoForTime({currentTime: 4.1, segmentIndex: 0, startTime: 0}),
this.getMediaInfoForTime({currentTime: 4.1, startingSegmentIndex: 0, startTime: 0}),
{segmentIndex: 2, startTime: 4, partIndex: null},
'2 away 3 is correct'
);
assert.deepEqual(
this.getMediaInfoForTime({currentTime: 6.1, segmentIndex: 1, startTime: 2}),
this.getMediaInfoForTime({currentTime: 6.1, startingSegmentIndex: 1, startTime: 2}),
{segmentIndex: 3, startTime: 6, partIndex: null},
'2 away 4 is correct'
);
assert.deepEqual(
this.getMediaInfoForTime({currentTime: 8.1, segmentIndex: 2, startTime: 4}),
this.getMediaInfoForTime({currentTime: 8.1, startingSegmentIndex: 2, startTime: 4}),
{segmentIndex: 4, startTime: 8, partIndex: null},
'2 away 5 is correct'
);
assert.deepEqual(
this.getMediaInfoForTime({currentTime: 10.1, segmentIndex: 3, startTime: 6}),
this.getMediaInfoForTime({currentTime: 10.1, startingSegmentIndex: 3, startTime: 6}),
{segmentIndex: 5, startTime: 10, partIndex: null},
'2 away 6 is correct'
);

// 3 segments away
assert.deepEqual(
this.getMediaInfoForTime({currentTime: 6.1, segmentIndex: 0, startTime: 0}),
this.getMediaInfoForTime({currentTime: 6.1, startingSegmentIndex: 0, startTime: 0}),
{segmentIndex: 3, startTime: 6, partIndex: null},
'3 away 4 is correct'
);
assert.deepEqual(
this.getMediaInfoForTime({currentTime: 8.1, segmentIndex: 1, startTime: 2}),
this.getMediaInfoForTime({currentTime: 8.1, startingSegmentIndex: 1, startTime: 2}),
{segmentIndex: 4, startTime: 8, partIndex: null},
'3 away 5 is correct'
);
assert.deepEqual(
this.getMediaInfoForTime({currentTime: 10.1, segmentIndex: 2, startTime: 4}),
this.getMediaInfoForTime({currentTime: 10.1, startingSegmentIndex: 2, startTime: 4}),
{segmentIndex: 5, startTime: 10, partIndex: null},
'3 away 6 is correct'
);
Expand Down Expand Up @@ -1168,74 +1168,74 @@ QUnit.module('Playlist', function() {
this.defaults = {
playlist: media,
currentTime: 2.1,
segmentIndex: 0,
partIndex: null,
startingSegmentIndex: 0,
startingPartIndex: null,
startTime: 0
};

// 1 segment away
assert.deepEqual(
this.getMediaInfoForTime({currentTime: 0, segmentIndex: 1, startTime: 2}),
this.getMediaInfoForTime({currentTime: 0, startingSegmentIndex: 1, startTime: 2}),
{segmentIndex: 0, startTime: 0, partIndex: null},
'1 away 1 is correct'
);
assert.deepEqual(
this.getMediaInfoForTime({currentTime: 2.1, segmentIndex: 2, startTime: 4}),
this.getMediaInfoForTime({currentTime: 2.1, startingSegmentIndex: 2, startTime: 4}),
{segmentIndex: 1, startTime: 2, partIndex: null},
'1 away 2 is correct'
);
assert.deepEqual(
this.getMediaInfoForTime({currentTime: 4.1, segmentIndex: 3, startTime: 6}),
this.getMediaInfoForTime({currentTime: 4.1, startingSegmentIndex: 3, startTime: 6}),
{segmentIndex: 2, startTime: 4, partIndex: null},
'1 away 3 is correct'
);
assert.deepEqual(
this.getMediaInfoForTime({currentTime: 6.1, segmentIndex: 4, startTime: 8}),
this.getMediaInfoForTime({currentTime: 6.1, startingSegmentIndex: 4, startTime: 8}),
{segmentIndex: 3, startTime: 6, partIndex: null},
'1 away 4 is correct'
);
assert.deepEqual(
this.getMediaInfoForTime({currentTime: 8.1, segmentIndex: 5, startTime: 10}),
this.getMediaInfoForTime({currentTime: 8.1, startingSegmentIndex: 5, startTime: 10}),
{segmentIndex: 4, startTime: 8, partIndex: null},
'1 away 5 is correct'
);

// 2 segments away
assert.deepEqual(
this.getMediaInfoForTime({currentTime: 0, segmentIndex: 2, startTime: 4}),
this.getMediaInfoForTime({currentTime: 0, startingSegmentIndex: 2, startTime: 4}),
{segmentIndex: 0, startTime: 0, partIndex: null},
'2 away 1 is correct'
);
assert.deepEqual(
this.getMediaInfoForTime({currentTime: 2.1, segmentIndex: 3, startTime: 6}),
this.getMediaInfoForTime({currentTime: 2.1, startingSegmentIndex: 3, startTime: 6}),
{segmentIndex: 1, startTime: 2, partIndex: null},
'2 away 2 is correct'
);
assert.deepEqual(
this.getMediaInfoForTime({currentTime: 4.1, segmentIndex: 4, startTime: 8}),
this.getMediaInfoForTime({currentTime: 4.1, startingSegmentIndex: 4, startTime: 8}),
{segmentIndex: 2, startTime: 4, partIndex: null},
'2 away 3 is correct'
);
assert.deepEqual(
this.getMediaInfoForTime({currentTime: 6.1, segmentIndex: 5, startTime: 10}),
this.getMediaInfoForTime({currentTime: 6.1, startingSegmentIndex: 5, startTime: 10}),
{segmentIndex: 3, startTime: 6, partIndex: null},
'2 away 4 is correct'
);

// 3 segments away
assert.deepEqual(
this.getMediaInfoForTime({currentTime: 0, segmentIndex: 3, startTime: 6}),
this.getMediaInfoForTime({currentTime: 0, startingSegmentIndex: 3, startTime: 6}),
{segmentIndex: 0, startTime: 0, partIndex: null},
'3 away 1 is correct'
);
assert.deepEqual(
this.getMediaInfoForTime({currentTime: 2.1, segmentIndex: 4, startTime: 8}),
this.getMediaInfoForTime({currentTime: 2.1, startingSegmentIndex: 4, startTime: 8}),
{segmentIndex: 1, startTime: 2, partIndex: null},
'3 away 2 is correct'
);

assert.deepEqual(
this.getMediaInfoForTime({currentTime: 4.1, segmentIndex: 5, startTime: 10}),
this.getMediaInfoForTime({currentTime: 4.1, startingSegmentIndex: 5, startTime: 10}),
{segmentIndex: 2, startTime: 4, partIndex: null},
'3 away 3 is correct'
);
Expand Down Expand Up @@ -1264,8 +1264,8 @@ QUnit.module('Playlist', function() {
this.defaults = {
playlist: media,
currentTime: 0,
segmentIndex: 0,
partIndex: null,
startingSegmentIndex: 0,
startingPartIndex: null,
startTime: 0
};

Expand Down Expand Up @@ -1310,8 +1310,8 @@ QUnit.module('Playlist', function() {
this.defaults = {
playlist: media,
currentTime: 0,
segmentIndex: 0,
partIndex: null,
startingSegmentIndex: 0,
startingPartIndex: null,
startTime: 0
};

Expand Down Expand Up @@ -1394,8 +1394,8 @@ QUnit.module('Playlist', function() {
this.defaults = {
playlist: media,
currentTime: 0,
segmentIndex: 0,
partIndex: null,
startingSegmentIndex: 0,
startingPartIndex: null,
startTime: 0
};

Expand Down
20 changes: 5 additions & 15 deletions test/sync-controller.test.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -246,30 +246,20 @@ QUnit.test('ProgramDateTime strategy finds nearest llhls sync point', function(a

this.syncController.setDateTimeMappingForStart(playlist);

const newPlaylist = playlistWithDuration(200, {llhls: true});

syncPoint = strategy.run(this.syncController, newPlaylist, duration, timeline);

assert.equal(syncPoint, null, 'no syncpoint when datetimeObject not set on playlist');

newPlaylist.segments.forEach((segment, index) => {
segment.dateTimeObject = new Date(2012, 11, 12, 12, 12, 22 + (index * 10));
});

syncPoint = strategy.run(this.syncController, newPlaylist, duration, timeline, 194);
syncPoint = strategy.run(this.syncController, playlist, duration, timeline, 194);

assert.deepEqual(syncPoint, {
time: 192,
segmentIndex: 18,
segmentIndex: 19,
partIndex: 1
}, 'syncpoint found for ProgramDateTime');

syncPoint = strategy.run(this.syncController, newPlaylist, duration, timeline, 204);
syncPoint = strategy.run(this.syncController, playlist, duration, timeline, 204);

assert.deepEqual(syncPoint, {
time: 202,
time: 198,
segmentIndex: 19,
partIndex: 1
partIndex: 4
}, 'syncpoint found for ProgramDateTime');
});

Expand Down

0 comments on commit ac9de31

Please sign in to comment.