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.

Sign up for GitHub

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

Jump to bottom

feat: Update minimumUpdatePeriod handling #942

Merged
merged 5 commits into from
Sep 23, 2020
Merged

feat: Update minimumUpdatePeriod handling #942

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
abf2b41
refresh when MUP changes, differentiate between MUP=0 and MUP=undefined
alex-barstow Sep 3, 2020
b6785bd
update mpd-parser
alex-barstow Sep 3, 2020
2d05724
Merge branch 'main' into update-minimumUpdatePeriod-handling
alex-barstow Sep 3, 2020
bf3b8c0
break out duplicate minimumUpdatePeriod logic into function
alex-barstow Sep 11, 2020
99b416b
clear timeout and add comment
alex-barstow Sep 23, 2020
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
6 changes: 3 additions & 3 deletions package-lock.json
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,7 @@
"aes-decrypter": "3.0.1",
"global": "^4.3.2",
"m3u8-parser": "4.4.3",
"mpd-parser": "0.11.0",
"mpd-parser": "0.12.0",
"mux.js": "5.6.6",
"video.js": "^6 || ^7"
},
Expand Down
24 changes: 12 additions & 12 deletions src/dash-playlist-loader.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -94,6 +94,10 @@ export const updateMaster = (oldMaster, newMaster) => {
}
});

if (newMaster.minimumUpdatePeriod !== oldMaster.minimumUpdatePeriod) {
noChanges = false;
}

if (noChanges) {
return null;
}
Expand Down Expand Up @@ -657,16 +661,10 @@ export default class DashPlaylistLoader extends EventTarget {
this.media(this.master.playlists[0]);
}

// TODO: minimumUpdatePeriod can have a value of 0. Currently the manifest will not
// be refreshed when this is the case. The inter-op guide says that when the
// minimumUpdatePeriod is 0, the manifest should outline all currently available
// segments, but future segments may require an update. I think a good solution
// would be to update the manifest at the same rate that the media playlists
// are "refreshed", i.e. every targetDuration.
if (this.master && this.master.minimumUpdatePeriod) {
if (this.master && this.master.minimumUpdatePeriod >= 0) {
this.minimumUpdatePeriodTimeout_ = window.setTimeout(() => {
this.trigger('minimumUpdatePeriod');
}, this.master.minimumUpdatePeriod);
}, this.master.minimumUpdatePeriod || this.media().targetDuration * 1000);
}
}

Expand Down Expand Up @@ -766,10 +764,11 @@ export default class DashPlaylistLoader extends EventTarget {

// Clear & reset timeout with new minimumUpdatePeriod
Copy link
Contributor

@gesinger gesinger Sep 10, 2020 •
edited
Loading

Choose a reason for hiding this comment

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

It may be worth creating a updateMinimumUpdatePeriodTimeout_() function to centralize the logic of the three uses (even can include the clearing) and add a comment around why we use the target duration when the minimumUpdatePeriod is 0.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yup makes sense. Done

window.clearTimeout(this.minimumUpdatePeriodTimeout_);
if (this.master.minimumUpdatePeriod) {

if (this.master.minimumUpdatePeriod >= 0) {
this.minimumUpdatePeriodTimeout_ = window.setTimeout(() => {
this.trigger('minimumUpdatePeriod');
}, this.master.minimumUpdatePeriod);
}, this.master.minimumUpdatePeriod || playlist.targetDuration * 1000);
}

// TODO: do we need to reload the current playlist?
Expand All @@ -789,10 +788,11 @@ export default class DashPlaylistLoader extends EventTarget {

// Clear & reset timeout with new minimumUpdatePeriod
window.clearTimeout(this.minimumUpdatePeriodTimeout_);
if (this.master.minimumUpdatePeriod) {

if (this.master.minimumUpdatePeriod >= 0) {
this.minimumUpdatePeriodTimeout_ = window.setTimeout(() => {
this.trigger('minimumUpdatePeriod');
}, this.master.minimumUpdatePeriod);
}, this.master.minimumUpdatePeriod || this.media().targetDuration * 1000);
}
});
}
Expand Down
107 changes: 95 additions & 12 deletions test/dash-playlist-loader.test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -320,6 +320,62 @@ QUnit.test('updateMaster: updates playlists and mediaGroups', function(assert) {
);
});

QUnit.test('updateMaster: updates minimumUpdatePeriod', function(assert) {
const master = {
playlists: {
length: 1,
0: {
uri: '0',
id: '0',
segments: []
}
},
mediaGroups: {
AUDIO: {},
SUBTITLES: {}
},
duration: 0,
minimumUpdatePeriod: 0
};

const update = {
playlists: {
length: 1,
0: {
uri: '0',
id: '0',
segments: []
}
},
mediaGroups: {
AUDIO: {},
SUBTITLES: {}
},
duration: 0,
minimumUpdatePeriod: 2
};

assert.deepEqual(
updateMaster(master, update),
{
playlists: {
length: 1,
0: {
uri: '0',
id: '0',
segments: []
}
},
mediaGroups: {
AUDIO: {},
SUBTITLES: {}
},
duration: 0,
minimumUpdatePeriod: 2
}
);
});

QUnit.test('generateSidxKey: generates correct key', function(assert) {
const sidxInfo = {
byterange: {
Expand Down Expand Up @@ -2324,7 +2380,7 @@ QUnit.test('refreshes the xml if there is a minimumUpdatePeriod', function(asser
assert.equal(minimumUpdatePeriods, 1, 'refreshed manifest');
});

QUnit.test('stop xml refresh if minimumUpdatePeriod changes from `mUP > 0` to `mUP == 0`', function(assert) {
QUnit.test('stop xml refresh if minimumUpdatePeriod is removed', function(assert) {
const loader = new DashPlaylistLoader('dash-live.mpd', this.fakeVhs);
let minimumUpdatePeriods = 0;

Expand All @@ -2337,25 +2393,52 @@ QUnit.test('stop xml refresh if minimumUpdatePeriod changes from `mUP > 0` to `m
this.standardXHRResponse(this.requests.shift());
assert.equal(minimumUpdatePeriods, 0, 'no refreshes immediately after response');

// First Refresh Tick
// First Refresh Tick: MPD loaded
this.clock.tick(4 * 1000);
this.standardXHRResponse(this.requests[0], loader.masterXml_);
assert.equal(this.requests.length, 1, 'refreshed manifest');
assert.equal(this.requests[0].uri, 'dash-live.mpd', 'refreshed manifest');
assert.equal(minimumUpdatePeriods, 1, 'total minimumUpdatePeriods');

// Second Refresh Tick: MinimumUpdatePeriod Removed
this.clock.tick(4 * 1000);
this.standardXHRResponse(this.requests[1], loader.masterXml_.replace('minimumUpdatePeriod="PT4S"', ''));
this.standardXHRResponse(this.requests[0], loader.masterXml_.replace('minimumUpdatePeriod="PT4S"', ''));

// Second Refresh Tick: MUP removed
this.clock.tick(4 * 1000);
this.standardXHRResponse(this.requests[2]);
assert.equal(this.requests.length, 3, 'final manifest refresh');
assert.equal(minimumUpdatePeriods, 3, 'final minimumUpdatePeriods');
assert.equal(this.requests.length, 1, 'no more manifest refreshes');
assert.equal(minimumUpdatePeriods, 1, 'no more minimumUpdatePeriods');
});

// Third Refresh Tick: No Additional Requests Expected
QUnit.test('continue xml refresh every targetDuration if minimumUpdatePeriod is 0', function(assert) {
const loader = new DashPlaylistLoader('dash-live.mpd', this.fakeVhs);
let minimumUpdatePeriods = 0;

loader.on('minimumUpdatePeriod', () => minimumUpdatePeriods++);

loader.load();

// Start Request
assert.equal(minimumUpdatePeriods, 0, 'no refreshes to start');
this.standardXHRResponse(this.requests.shift());
assert.equal(minimumUpdatePeriods, 0, 'no refreshes immediately after response');

// First Refresh Tick
this.clock.tick(4 * 1000);
assert.equal(this.requests.length, 3, 'final manifest refresh');
assert.equal(minimumUpdatePeriods, 3, 'final minimumUpdatePeriods');
assert.equal(this.requests.length, 1, 'refreshed manifest');
assert.equal(this.requests[0].uri, 'dash-live.mpd', 'refreshed manifest');
assert.equal(minimumUpdatePeriods, 1, 'total minimumUpdatePeriods');

this.standardXHRResponse(this.requests[0], loader.masterXml_.replace('minimumUpdatePeriod="PT4S"', 'minimumUpdatePeriod="PT0S"'));

// Second Refresh Tick: MinimumUpdatePeriod set to 0
// The manifest should refresh after one target duration, in this case 2 seconds. At this point
// it should not have occurred.
this.clock.tick(1 * 1000);
assert.equal(this.requests.length, 1, 'no 3rd manifest refresh yet');
assert.equal(minimumUpdatePeriods, 1, 'no 3rd minimumUpdatePeriod yet');

// Now the refresh should happen
this.clock.tick(1 * 1000);
assert.equal(this.requests.length, 2, '3rd manifest refresh after targetDuration');
assert.equal(minimumUpdatePeriods, 2, '3rd minimumUpdatePeriod after targetDuration');
});

QUnit.test('media playlists "refresh" by re-parsing master xml', function(assert) {
Expand Down