Skip to content

Commit

Permalink
fix: content steering bug fixes and tests (#1430)
Browse files Browse the repository at this point in the history
  • Loading branch information
@adrums86
adrums86 authored Nov 7, 2023
1 parent 719b7f4 commit 532aa4d
Show file tree
Hide file tree
Showing 6 changed files with 308 additions and 113 deletions.
50 changes: 34 additions & 16 deletions src/content-steering-controller.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,7 @@ export default class ContentSteeringController extends videojs.EventTarget {

this.currentPathway = null;
this.defaultPathway = null;
this.queryBeforeStart = null;
this.queryBeforeStart = false;
this.availablePathways_ = new Set();
this.excludedPathways_ = new Set();
this.steeringManifest = new SteeringManifest();
Expand All @@ -92,7 +92,7 @@ export default class ContentSteeringController extends videojs.EventTarget {
/**
* Assigns the content steering tag properties to the steering controller
*
* @param {string} baseUrl the baseURL from the manifest for resolving the steering manifest url
* @param {string} baseUrl the baseURL from the main manifest for resolving the steering manifest url
* @param {Object} steeringTag the content steering tag from the main manifest
*/
assignTagProperties(baseUrl, steeringTag) {
Expand All @@ -110,24 +110,21 @@ export default class ContentSteeringController extends videojs.EventTarget {
this.decodeDataUriManifest_(steeringUri.substring(steeringUri.indexOf(',') + 1));
return;
}
// With DASH queryBeforeStart, we want to use the steeringUri as soon as possible for the request.
this.steeringManifest.reloadUri = this.queryBeforeStart ? steeringUri : resolveUrl(baseUrl, steeringUri);

// reloadUri is the resolution of the main manifest URL and steering URL.
this.steeringManifest.reloadUri = resolveUrl(baseUrl, steeringUri);
// pathwayId is HLS defaultServiceLocation is DASH
this.defaultPathway = steeringTag.pathwayId || steeringTag.defaultServiceLocation;
// currently only DASH supports the following properties on <ContentSteering> tags.
this.queryBeforeStart = steeringTag.queryBeforeStart || false;
this.proxyServerUrl_ = steeringTag.proxyServerURL || null;
this.queryBeforeStart = steeringTag.queryBeforeStart;
this.proxyServerUrl_ = steeringTag.proxyServerURL;

// trigger a steering event if we have a pathway from the content steering tag.
// this tells VHS which segment pathway to start with.
// If queryBeforeStart is true we need to wait for the steering manifest response.
if (this.defaultPathway && !this.queryBeforeStart) {
this.trigger('content-steering');
}

if (this.queryBeforeStart) {
this.requestSteeringManifest(this.steeringManifest.reloadUri);
}
}

/**
Expand All @@ -136,21 +133,20 @@ export default class ContentSteeringController extends videojs.EventTarget {
*
* @param {string} initialUri The optional uri to make the request with.
* If set, the request should be made with exactly what is passed in this variable.
* This scenario is specific to DASH when the queryBeforeStart parameter is true.
* This scenario should only happen once on initalization.
*/
requestSteeringManifest(initialUri) {
requestSteeringManifest(initial) {
const reloadUri = this.steeringManifest.reloadUri;

if (!initialUri && !reloadUri) {
if (!reloadUri) {
return;
}

// We currently don't support passing MPD query parameters directly to the content steering URL as this requires
// ExtUrlQueryInfo tag support. See the DASH content steering spec section 8.1.

// This request URI accounts for manifest URIs that have been excluded.
const uri = initialUri || this.getRequestURI(reloadUri);
const uri = initial ? reloadUri : this.getRequestURI(reloadUri);

// If there are no valid manifest URIs, we should stop content steering.
if (!uri) {
Expand Down Expand Up @@ -196,8 +192,8 @@ export default class ContentSteeringController extends videojs.EventTarget {
}
const steeringManifestJson = JSON.parse(this.request_.responseText);

this.startTTLTimeout_();
this.assignSteeringProperties_(steeringManifestJson);
this.startTTLTimeout_();
});
}

Expand Down Expand Up @@ -413,13 +409,35 @@ export default class ContentSteeringController extends videojs.EventTarget {
}

/**
* clears all pathways from the available pathways set
* Clears all pathways from the available pathways set
*/
clearAvailablePathways() {
this.availablePathways_.clear();
}

/**
* Removes a pathway from the available pathways set.
*/
excludePathway(pathway) {
return this.availablePathways_.delete(pathway);
}

/**
* Checks the refreshed DASH manifest content steering tag for changes.
*
* @param {string} baseURL new steering tag on DASH manifest refresh
* @param {Object} newTag the new tag to check for changes
* @return a true or false whether the new tag has different values
*/
didDASHTagChange(baseURL, newTag) {
return !newTag && this.steeringManifest.reloadUri ||
newTag && (resolveUrl(baseURL, newTag.serverURL) !== this.steeringManifest.reloadUri ||
newTag.defaultServiceLocation !== this.defaultPathway ||
newTag.queryBeforeStart !== this.queryBeforeStart ||
newTag.proxyServerURL !== this.proxyServerUrl_);
}

getAvailablePathways() {
return this.availablePathways_;
}
}
5 changes: 1 addition & 4 deletions src/dash-playlist-loader.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -325,10 +325,7 @@ export default class DashPlaylistLoader extends EventTarget {

// live playlist staleness timeout
this.on('mediaupdatetimeout', () => {
// We handle live content steering in the playlist controller
if (!this.media().attributes.serviceLocation) {
this.refreshMedia_(this.media().id);
}
this.refreshMedia_(this.media().id);
});

this.state = 'HAVE_NOTHING';
Expand Down
97 changes: 66 additions & 31 deletions src/playlist-controller.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -608,6 +608,8 @@ export class PlaylistController extends videojs.EventTarget {
let updatedPlaylist = this.mainPlaylistLoader_.media();

if (!updatedPlaylist) {
// Add content steering listeners on first load and init.
this.attachContentSteeringListeners_();
this.initContentSteeringController_();
// exclude any variants that are not supported by the browser before selecting
// an initial media as the playlist selectors do not consider browser support
Expand Down Expand Up @@ -2104,51 +2106,84 @@ export class PlaylistController extends videojs.EventTarget {
});
}

/**
* Utility for getting the pathway or service location from an HLS or DASH playlist.
*
* @param {Object} playlist for getting pathway from.
* @return the pathway attribute of a playlist
*/
pathwayAttribute_(playlist) {
return playlist.attributes['PATHWAY-ID'] || playlist.attributes.serviceLocation;
}

/**
* Initialize content steering listeners and apply the tag properties.
* Initialize available pathways and apply the tag properties.
*/
initContentSteeringController_() {
const initialMain = this.main();
const main = this.main();

if (!initialMain.contentSteering) {
if (!main.contentSteering) {
return;
}
for (const playlist of main.playlists) {
this.contentSteeringController_.addAvailablePathway(this.pathwayAttribute_(playlist));
}
this.contentSteeringController_.assignTagProperties(main.uri, main.contentSteering);
// request the steering manifest immediately if queryBeforeStart is set.
if (this.contentSteeringController_.queryBeforeStart) {
// When queryBeforeStart is true, initial request should omit steering parameters.
this.contentSteeringController_.requestSteeringManifest(true);
return;
}
// otherwise start content steering after playback starts
this.tech_.one('canplay', () => {
this.contentSteeringController_.requestSteeringManifest();
});
}

const updateSteeringValues = (main) => {
for (const playlist of main.playlists) {
this.contentSteeringController_.addAvailablePathway(this.pathwayAttribute_(playlist));
}

this.contentSteeringController_.assignTagProperties(main.uri, main.contentSteering);
};

updateSteeringValues(initialMain);
/**
* Reset the content steering controller and re-init.
*/
resetContentSteeringController_() {
this.contentSteeringController_.clearAvailablePathways();
this.contentSteeringController_.dispose();
this.initContentSteeringController_();
}

/**
* Attaches the listeners for content steering.
*/
attachContentSteeringListeners_() {
this.contentSteeringController_.on('content-steering', this.excludeThenChangePathway_.bind(this));

// We need to ensure we update the content steering values when a new
// manifest is loaded in live DASH with content steering.
if (this.sourceType_ === 'dash') {
this.mainPlaylistLoader_.on('mediaupdatetimeout', () => {
this.mainPlaylistLoader_.refreshMedia_(this.mainPlaylistLoader_.media().id);

// clear past values
this.contentSteeringController_.abort();
this.contentSteeringController_.clearTTLTimeout_();
this.contentSteeringController_.clearAvailablePathways();

updateSteeringValues(this.main());
});
}
this.mainPlaylistLoader_.on('loadedplaylist', () => {
const main = this.main();
// check if steering tag or pathways changed.
const didDashTagChange = this.contentSteeringController_.didDASHTagChange(main.uri, main.contentSteering);
const didPathwaysChange = () => {
const availablePathways = this.contentSteeringController_.getAvailablePathways();
const newPathways = [];

for (const playlist of main.playlists) {
const serviceLocation = playlist.attributes.serviceLocation;

if (serviceLocation) {
newPathways.push(serviceLocation);
if (!availablePathways.has(serviceLocation)) {
return true;
}
}
}
// If we have no new serviceLocations and previously had availablePathways
if (!newPathways.length && availablePathways.size) {
return true;
}
return false;
};

// Do this at startup only, after that the steering requests are managed by the Content Steering class.
// DASH queryBeforeStart scenarios will be handled by the Content Steering class.
if (!this.contentSteeringController_.queryBeforeStart) {
this.tech_.one('canplay', () => {
this.contentSteeringController_.requestSteeringManifest();
if (didDashTagChange || didPathwaysChange()) {
this.resetContentSteeringController_();
}
});
}
}
Expand Down
29 changes: 28 additions & 1 deletion test/content-steering-controller.test.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -194,6 +194,33 @@ QUnit.test('Can handle DASH proxyServerURL', function(assert) {
assert.equal(this.requests[0].uri, expectedProxyUrl, 'returns expected proxy server URL');
});

QUnit.test('didDASHTagChange can check if a DASH content steering tag has changed', function(assert) {
const oldSteeringTag = {
serverURL: 'https://content.steering.dash/?old=params',
proxyServerURL: 'https://old.proxy.url',
defaultServiceLocation: 'old-dash-cdn'
};
const newSteeringTag = {
serverURL: 'https://content.steering.dash/?new=params',
proxyServerURL: 'https://new.proxy.url',
defaultServiceLocation: 'new-dash-cdn',
queryBeforeStart: true
};
const ommittedAttributes = {
serverURL: 'https://content.steering.dash/?old=params'
};

this.contentSteeringController.assignTagProperties(this.baseURL, oldSteeringTag);
assert.false(this.contentSteeringController.didDASHTagChange(this.baseURL, oldSteeringTag));
assert.true(this.contentSteeringController.didDASHTagChange(this.baseURL, newSteeringTag));
assert.true(this.contentSteeringController.didDASHTagChange(this.baseURL, ommittedAttributes));
this.contentSteeringController.dispose();
this.contentSteeringController.assignTagProperties(this.baseURL, ommittedAttributes);
assert.false(this.contentSteeringController.didDASHTagChange(this.baseURL, ommittedAttributes));
assert.true(this.contentSteeringController.didDASHTagChange(this.baseURL, newSteeringTag));
assert.true(this.contentSteeringController.didDASHTagChange(this.baseURL, oldSteeringTag));
});

// Common steering manifest tests
QUnit.test('Can handle content steering manifest with VERSION', function(assert) {
const steeringTag = {
Expand Down Expand Up @@ -269,7 +296,7 @@ QUnit.test('Can handle DASH content steering manifest with SERVICE-LOCATION-PRIO

QUnit.test('Can handle DASH content steering manifest with PATHWAY-PRIORITY and tag with pathwayId', function(assert) {
const steeringTag = {
serverUri: 'https://content.steering.hls',
serverUri: 'https://content.steering.dash',
pathwayId: 'dash3'
};

Expand Down
56 changes: 0 additions & 56 deletions test/dash-playlist-loader.test.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2821,62 +2821,6 @@ QUnit.test('pause does not remove minimum update period timeout when not main',
);
});

QUnit.test('Content Steering with Live DASH should NOT update media', function(assert) {
const mainLoader = new DashPlaylistLoader('dash-live.mpd', this.fakeVhs);

const refreshMediaSpy = sinon.stub(mainLoader, 'refreshMedia_');

mainLoader.load();
this.standardXHRResponse(this.requests.shift());
this.clock.tick(1);

const media = mainLoader.main.playlists[0];

mainLoader.media = () => media;

// This means content steering is active on the media.
media.attributes.serviceLocation = 'cdn-a';

mainLoader.media(media);

// This means there was a DASH live update.
mainLoader.trigger('mediaupdatetimeout');

// If refreshMedia_ is only called once, it means it was called on initialization,
// and is expected to be called later by the playlist controller.
assert.equal(
refreshMediaSpy.callCount,
1
);
});

QUnit.test('Live DASH without content steering should update media', function(assert) {
const mainLoader = new DashPlaylistLoader('dash-live.mpd', this.fakeVhs);

const refreshMediaSpy = sinon.stub(mainLoader, 'refreshMedia_');

mainLoader.load();
this.standardXHRResponse(this.requests.shift());
this.clock.tick(1);

const media = mainLoader.main.playlists[0];

mainLoader.media = () => media;

mainLoader.media(media);

// This means there was a DASH live update.
mainLoader.trigger('mediaupdatetimeout');

// If refreshMedia_ is called twice, it means it is was called on initialization,
// and later when there is a live update. This should all be handled by the
// playlist controller.
assert.equal(
refreshMediaSpy.callCount,
2
);
});

QUnit.test('updateMain: merges in top level timelineStarts', function(assert) {
const prev = {
timelineStarts: [0, 1],
Expand Down
Loading

0 comments on commit 532aa4d

Please sign in to comment.