From 28fa98e7f8841f70e84aef773829ca78f3515ea4 Mon Sep 17 00:00:00 2001 From: Matt Ward Date: Fri, 21 Sep 2018 09:36:25 -0700 Subject: [PATCH 1/7] add an option to ignore player size in selection logic --- README.md | 10 ++++++++++ src/config.js | 2 ++ src/playlist-selectors.js | 22 ++++++++++++++++++--- src/videojs-http-streaming.js | 10 ++++------ test/configuration.test.js | 5 +++++ test/playlist-selectors.test.js | 34 ++++++++++++++++++++++++++++++++- 6 files changed, 73 insertions(+), 10 deletions(-) diff --git a/README.md b/README.md index 45b682b48..dba7d8780 100644 --- a/README.md +++ b/README.md @@ -333,6 +333,16 @@ When `enableLowInitialPlaylist` is set to true, it will be used to select the lowest bitrate playlist initially. This helps to decrease playback start time. This setting is `false` by default. +##### ignorePlayerSize +* Type: `boolean` +* can be used as an initialization option + +When `ignorePlayerSize` is set to true, selection logic will ignore the +player size and rendition resolutions when making a decision. This should +be used when doing per-title encoding which makes non-traditional +rendition resolution decisions. +This setting is `false` by default. + ### Runtime Properties Runtime properties are attached to the tech object when HLS is in use. You can get a reference to the HLS source handler like this: diff --git a/src/config.js b/src/config.js index 1c812a01f..ed95983b7 100644 --- a/src/config.js +++ b/src/config.js @@ -2,6 +2,8 @@ export default { GOAL_BUFFER_LENGTH: 30, MAX_GOAL_BUFFER_LENGTH: 60, GOAL_BUFFER_LENGTH_RATE: 1, + // 0.5 MB/s + INITIAL_BANDWIDTH: 4194304, // A fudge factor to apply to advertised playlist bitrates to account for // temporary flucations in client bandwidth BANDWIDTH_VARIANCE: 1.2, diff --git a/src/playlist-selectors.js b/src/playlist-selectors.js index 6a707ad51..662855e3a 100644 --- a/src/playlist-selectors.js +++ b/src/playlist-selectors.js @@ -124,6 +124,8 @@ export const comparePlaylistResolution = function(left, right) { * Current width of the player element * @param {Number} playerHeight * Current height of the player element + * @param {Boolean} ignorePlayerSize + * True if the player width and height should be ignored during the selection, false otherwise * @return {Playlist} the highest bitrate playlist less than the * currently detected bandwidth, accounting for some amount of * bandwidth variance @@ -131,7 +133,8 @@ export const comparePlaylistResolution = function(left, right) { export const simpleSelector = function(master, playerBandwidth, playerWidth, - playerHeight) { + playerHeight, + ignorePlayerSize) { // convert the playlists to an intermediary representation to make comparisons easier let sortedPlaylistReps = master.playlists.map((playlist) => { let width; @@ -190,6 +193,16 @@ export const simpleSelector = function(master, (rep) => rep.bandwidth === highestRemainingBandwidthRep.bandwidth )[0]; + // if we are ignoring the player size, make an early decision + if (ignorePlayerSize) { + let chosenRep = ( + bandwidthBestRep || + enabledPlaylistReps[0] || + sortedPlaylistReps[0] + ); + return chosenRep ? chosenRep.playlist : null; + } + // filter out playlists without resolution information let haveResolution = bandwidthPlaylistReps.filter((rep) => rep.width && rep.height); @@ -217,6 +230,7 @@ export const simpleSelector = function(master, resolutionPlusOneList = haveResolution.filter( (rep) => rep.width > playerWidth || rep.height > playerHeight ); + console.log('resolutionPlusOneList', haveResolution, resolutionPlusOneList, playerWidth, playerHeight); // find all the variants have the same smallest resolution resolutionPlusOneSmallest = resolutionPlusOneList.filter( @@ -261,7 +275,8 @@ export const lastBandwidthSelector = function() { return simpleSelector(this.playlists.master, this.systemBandwidth, parseInt(safeGetComputedStyle(this.tech_.el(), 'width'), 10), - parseInt(safeGetComputedStyle(this.tech_.el(), 'height'), 10)); + parseInt(safeGetComputedStyle(this.tech_.el(), 'height'), 10), + this.ignorePlayerSize); }; /** @@ -294,7 +309,8 @@ export const movingAverageBandwidthSelector = function(decay) { return simpleSelector(this.playlists.master, average, parseInt(safeGetComputedStyle(this.tech_.el(), 'width'), 10), - parseInt(safeGetComputedStyle(this.tech_.el(), 'height'), 10)); + parseInt(safeGetComputedStyle(this.tech_.el(), 'height'), 10), + this.ignorePlayerSize); }; }; diff --git a/src/videojs-http-streaming.js b/src/videojs-http-streaming.js index 557ecc4c9..ef8d7a607 100644 --- a/src/videojs-http-streaming.js +++ b/src/videojs-http-streaming.js @@ -44,9 +44,6 @@ const Hls = { xhr: xhrFactory() }; -// 0.5 MB/s -const INITIAL_BANDWIDTH = 4194304; - // Define getter/setters for config properites [ 'GOAL_BUFFER_LENGTH', @@ -335,6 +332,7 @@ class HlsHandler extends Component { setOptions_() { // defaults this.options_.withCredentials = this.options_.withCredentials || false; + this.options_.ignorePlayerSize = this.options_.ignorePlayerSize || false; if (typeof this.options_.blacklistDuration !== 'number') { this.options_.blacklistDuration = 5 * 60; @@ -343,17 +341,17 @@ class HlsHandler extends Component { // start playlist selection at a reasonable bandwidth for // broadband internet (0.5 MB/s) or mobile (0.0625 MB/s) if (typeof this.options_.bandwidth !== 'number') { - this.options_.bandwidth = INITIAL_BANDWIDTH; + this.options_.bandwidth = Config.INITIAL_BANDWIDTH; } // If the bandwidth number is unchanged from the initial setting // then this takes precedence over the enableLowInitialPlaylist option this.options_.enableLowInitialPlaylist = this.options_.enableLowInitialPlaylist && - this.options_.bandwidth === INITIAL_BANDWIDTH; + this.options_.bandwidth === Config.INITIAL_BANDWIDTH; // grab options passed to player.src - ['withCredentials', 'bandwidth'].forEach((option) => { + ['withCredentials', 'ignorePlayerSize', 'bandwidth'].forEach((option) => { if (typeof this.source_[option] !== 'undefined') { this.options_[option] = this.source_[option]; } diff --git a/test/configuration.test.js b/test/configuration.test.js index 6ca0410db..662a920ed 100644 --- a/test/configuration.test.js +++ b/test/configuration.test.js @@ -23,6 +23,11 @@ const options = [{ default: false, test: true, alt: false +}, { + name: 'ignorePlayerSize', + default: false, + test: true, + alt: false }, { name: 'bandwidth', default: 4194304, diff --git a/test/playlist-selectors.test.js b/test/playlist-selectors.test.js index 2fb31cab9..73d6d4c83 100644 --- a/test/playlist-selectors.test.js +++ b/test/playlist-selectors.test.js @@ -163,7 +163,39 @@ test('simpleSelector switches up even without resolution information', function( { attributes: { BANDWIDTH: 1000 } } ]; - const selectedPlaylist = simpleSelector(master, 2000, 1, 1); + const selectedPlaylist = simpleSelector(master, 2000, 1, 1, false); assert.equal(selectedPlaylist, master.playlists[1], 'selected the correct playlist'); }); + +// A set of playlists that were defined using non-traditional encoding. +// The resolutions were selected using a per-title encoding technique +// that ensures the resolution maximizes quality at a given bitrate. +const trickyPlaylists = [ + { attributes: { BANDWIDTH: 2362080, RESOLUTION: { width: 1280, height: 720 } } }, + { attributes: { BANDWIDTH: 1390830, RESOLUTION: { width: 1280, height: 720 } } }, + { attributes: { BANDWIDTH: 866114, RESOLUTION: { width: 1024, height: 576 } } }, + { attributes: { BANDWIDTH: 573028, RESOLUTION: { width: 768, height: 432 } } }, + { attributes: { BANDWIDTH: 3482070, RESOLUTION: { width: 1920, height: 1080 } } }, + { attributes: { BANDWIDTH: 6151620, RESOLUTION: { width: 1920, height: 1080 } } } +]; + +test('simpleSelector accounts for resolution information when it exists', function(assert) { + let master = this.hls.playlists.master; + + master.playlists = trickyPlaylists; + + const selectedPlaylist = simpleSelector(master, Config.INITIAL_BANDWIDTH, 444, 790, false); + + assert.equal(selectedPlaylist, master.playlists[3], 'selected the playlist with the lowest bandwidth higher than player resolution'); +}); + +test('simpleSelector can ignore player size and resolutions', function(assert) { + let master = this.hls.playlists.master; + + master.playlists = trickyPlaylists; + + const selectedPlaylist = simpleSelector(master, Config.INITIAL_BANDWIDTH, 444, 790, true); + + assert.equal(selectedPlaylist, master.playlists[4], 'selected a playlist based solely on bandwidth'); +}); From 077d9c25486424ef683127653b48f68df0403b25 Mon Sep 17 00:00:00 2001 From: Matt Ward Date: Fri, 21 Sep 2018 11:32:20 -0700 Subject: [PATCH 2/7] review comments --- README.md | 6 ++---- src/playlist-selectors.js | 2 +- 2 files changed, 3 insertions(+), 5 deletions(-) diff --git a/README.md b/README.md index dba7d8780..baf8dbc0d 100644 --- a/README.md +++ b/README.md @@ -337,10 +337,8 @@ This setting is `false` by default. * Type: `boolean` * can be used as an initialization option -When `ignorePlayerSize` is set to true, selection logic will ignore the -player size and rendition resolutions when making a decision. This should -be used when doing per-title encoding which makes non-traditional -rendition resolution decisions. +When `ignorePlayerSize` is set to true, rendition selection logic will ignore +the player size and rendition resolutions when making a decision. This setting is `false` by default. ### Runtime Properties diff --git a/src/playlist-selectors.js b/src/playlist-selectors.js index 662855e3a..61ddb729f 100644 --- a/src/playlist-selectors.js +++ b/src/playlist-selectors.js @@ -200,6 +200,7 @@ export const simpleSelector = function(master, enabledPlaylistReps[0] || sortedPlaylistReps[0] ); + return chosenRep ? chosenRep.playlist : null; } @@ -230,7 +231,6 @@ export const simpleSelector = function(master, resolutionPlusOneList = haveResolution.filter( (rep) => rep.width > playerWidth || rep.height > playerHeight ); - console.log('resolutionPlusOneList', haveResolution, resolutionPlusOneList, playerWidth, playerHeight); // find all the variants have the same smallest resolution resolutionPlusOneSmallest = resolutionPlusOneList.filter( From d29f14b3ee454ef1b2bd184db91b42caf2615648 Mon Sep 17 00:00:00 2001 From: Matt Ward Date: Fri, 21 Sep 2018 13:45:33 -0700 Subject: [PATCH 3/7] so shall it be limitRenditionByPlayerDimensions --- README.md | 9 +++++---- src/playlist-selectors.js | 14 +++++++------- src/videojs-http-streaming.js | 4 ++-- test/configuration.test.js | 4 ++-- test/playlist-selectors.test.js | 8 ++++---- 5 files changed, 20 insertions(+), 19 deletions(-) diff --git a/README.md b/README.md index baf8dbc0d..e420d1a7f 100644 --- a/README.md +++ b/README.md @@ -333,13 +333,14 @@ When `enableLowInitialPlaylist` is set to true, it will be used to select the lowest bitrate playlist initially. This helps to decrease playback start time. This setting is `false` by default. -##### ignorePlayerSize +##### limitRenditionByPlayerDimensions * Type: `boolean` * can be used as an initialization option -When `ignorePlayerSize` is set to true, rendition selection logic will ignore -the player size and rendition resolutions when making a decision. -This setting is `false` by default. +When `limitRenditionByPlayerDimensions` is set to true, rendition +selection logic will take into account the player size and rendition +resolutions when making a decision. +This setting is `true` by default. ### Runtime Properties Runtime properties are attached to the tech object when HLS is in diff --git a/src/playlist-selectors.js b/src/playlist-selectors.js index 61ddb729f..e9c10df18 100644 --- a/src/playlist-selectors.js +++ b/src/playlist-selectors.js @@ -124,8 +124,8 @@ export const comparePlaylistResolution = function(left, right) { * Current width of the player element * @param {Number} playerHeight * Current height of the player element - * @param {Boolean} ignorePlayerSize - * True if the player width and height should be ignored during the selection, false otherwise + * @param {Boolean} limitRenditionByPlayerDimensions + * True if the player width and height should be used during the selection, false otherwise * @return {Playlist} the highest bitrate playlist less than the * currently detected bandwidth, accounting for some amount of * bandwidth variance @@ -134,7 +134,7 @@ export const simpleSelector = function(master, playerBandwidth, playerWidth, playerHeight, - ignorePlayerSize) { + limitRenditionByPlayerDimensions) { // convert the playlists to an intermediary representation to make comparisons easier let sortedPlaylistReps = master.playlists.map((playlist) => { let width; @@ -193,8 +193,8 @@ export const simpleSelector = function(master, (rep) => rep.bandwidth === highestRemainingBandwidthRep.bandwidth )[0]; - // if we are ignoring the player size, make an early decision - if (ignorePlayerSize) { + // if we not going to limit renditions by player size, make an early decision. + if (!limitRenditionByPlayerDimensions) { let chosenRep = ( bandwidthBestRep || enabledPlaylistReps[0] || @@ -276,7 +276,7 @@ export const lastBandwidthSelector = function() { this.systemBandwidth, parseInt(safeGetComputedStyle(this.tech_.el(), 'width'), 10), parseInt(safeGetComputedStyle(this.tech_.el(), 'height'), 10), - this.ignorePlayerSize); + this.limitRenditionByPlayerDimensions); }; /** @@ -310,7 +310,7 @@ export const movingAverageBandwidthSelector = function(decay) { average, parseInt(safeGetComputedStyle(this.tech_.el(), 'width'), 10), parseInt(safeGetComputedStyle(this.tech_.el(), 'height'), 10), - this.ignorePlayerSize); + this.limitRenditionByPlayerDimensions); }; }; diff --git a/src/videojs-http-streaming.js b/src/videojs-http-streaming.js index ef8d7a607..21cae0008 100644 --- a/src/videojs-http-streaming.js +++ b/src/videojs-http-streaming.js @@ -332,7 +332,7 @@ class HlsHandler extends Component { setOptions_() { // defaults this.options_.withCredentials = this.options_.withCredentials || false; - this.options_.ignorePlayerSize = this.options_.ignorePlayerSize || false; + this.options_.limitRenditionByPlayerDimensions = this.options_.limitRenditionByPlayerDimensions || true; if (typeof this.options_.blacklistDuration !== 'number') { this.options_.blacklistDuration = 5 * 60; @@ -351,7 +351,7 @@ class HlsHandler extends Component { this.options_.bandwidth === Config.INITIAL_BANDWIDTH; // grab options passed to player.src - ['withCredentials', 'ignorePlayerSize', 'bandwidth'].forEach((option) => { + ['withCredentials', 'limitRenditionByPlayerDimensions', 'bandwidth'].forEach((option) => { if (typeof this.source_[option] !== 'undefined') { this.options_[option] = this.source_[option]; } diff --git a/test/configuration.test.js b/test/configuration.test.js index 662a920ed..42ce716ca 100644 --- a/test/configuration.test.js +++ b/test/configuration.test.js @@ -24,8 +24,8 @@ const options = [{ test: true, alt: false }, { - name: 'ignorePlayerSize', - default: false, + name: 'limitRenditionByPlayerDimensions', + default: true, test: true, alt: false }, { diff --git a/test/playlist-selectors.test.js b/test/playlist-selectors.test.js index 73d6d4c83..f60b6056e 100644 --- a/test/playlist-selectors.test.js +++ b/test/playlist-selectors.test.js @@ -180,22 +180,22 @@ const trickyPlaylists = [ { attributes: { BANDWIDTH: 6151620, RESOLUTION: { width: 1920, height: 1080 } } } ]; -test('simpleSelector accounts for resolution information when it exists', function(assert) { +test('simpleSelector limits using resolution information when it exists', function(assert) { let master = this.hls.playlists.master; master.playlists = trickyPlaylists; - const selectedPlaylist = simpleSelector(master, Config.INITIAL_BANDWIDTH, 444, 790, false); + const selectedPlaylist = simpleSelector(master, Config.INITIAL_BANDWIDTH, 444, 790, true); assert.equal(selectedPlaylist, master.playlists[3], 'selected the playlist with the lowest bandwidth higher than player resolution'); }); -test('simpleSelector can ignore player size and resolutions', function(assert) { +test('simpleSelector can not limit based on resolution information', function(assert) { let master = this.hls.playlists.master; master.playlists = trickyPlaylists; - const selectedPlaylist = simpleSelector(master, Config.INITIAL_BANDWIDTH, 444, 790, true); + const selectedPlaylist = simpleSelector(master, Config.INITIAL_BANDWIDTH, 444, 790, false); assert.equal(selectedPlaylist, master.playlists[4], 'selected a playlist based solely on bandwidth'); }); From 8a97633ad3fd98a415b5ab17901aba30325dd872 Mon Sep 17 00:00:00 2001 From: Matt Ward Date: Fri, 21 Sep 2018 14:53:14 -0700 Subject: [PATCH 4/7] we to we're --- src/playlist-selectors.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/playlist-selectors.js b/src/playlist-selectors.js index e9c10df18..2617a5bee 100644 --- a/src/playlist-selectors.js +++ b/src/playlist-selectors.js @@ -193,7 +193,7 @@ export const simpleSelector = function(master, (rep) => rep.bandwidth === highestRemainingBandwidthRep.bandwidth )[0]; - // if we not going to limit renditions by player size, make an early decision. + // if we're not going to limit renditions by player size, make an early decision. if (!limitRenditionByPlayerDimensions) { let chosenRep = ( bandwidthBestRep || From 89c2de42c9c8569b5dc05123c7f426c1dc0d0d14 Mon Sep 17 00:00:00 2001 From: Matt Ward Date: Tue, 2 Oct 2018 08:15:59 -0700 Subject: [PATCH 5/7] more clear and explicit false check --- src/playlist-selectors.js | 2 +- src/videojs-http-streaming.js | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/playlist-selectors.js b/src/playlist-selectors.js index 2617a5bee..286930f0d 100644 --- a/src/playlist-selectors.js +++ b/src/playlist-selectors.js @@ -194,7 +194,7 @@ export const simpleSelector = function(master, )[0]; // if we're not going to limit renditions by player size, make an early decision. - if (!limitRenditionByPlayerDimensions) { + if (limitRenditionByPlayerDimensions === false) { let chosenRep = ( bandwidthBestRep || enabledPlaylistReps[0] || diff --git a/src/videojs-http-streaming.js b/src/videojs-http-streaming.js index 21cae0008..20f0c2b99 100644 --- a/src/videojs-http-streaming.js +++ b/src/videojs-http-streaming.js @@ -332,7 +332,7 @@ class HlsHandler extends Component { setOptions_() { // defaults this.options_.withCredentials = this.options_.withCredentials || false; - this.options_.limitRenditionByPlayerDimensions = this.options_.limitRenditionByPlayerDimensions || true; + this.options_.limitRenditionByPlayerDimensions = this.options_.limitRenditionByPlayerDimensions === false ? false : true; if (typeof this.options_.blacklistDuration !== 'number') { this.options_.blacklistDuration = 5 * 60; From 9b242340fd91545e31ed710ba663ac34e6fc3b34 Mon Sep 17 00:00:00 2001 From: Matt Ward Date: Tue, 2 Oct 2018 08:24:06 -0700 Subject: [PATCH 6/7] fix the test to test false --- test/configuration.test.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/configuration.test.js b/test/configuration.test.js index 42ce716ca..e85110fba 100644 --- a/test/configuration.test.js +++ b/test/configuration.test.js @@ -26,7 +26,7 @@ const options = [{ }, { name: 'limitRenditionByPlayerDimensions', default: true, - test: true, + test: false, alt: false }, { name: 'bandwidth', From abc3eab7d93f9731d8e21e6177c581d71efd5964 Mon Sep 17 00:00:00 2001 From: Matt Ward Date: Tue, 2 Oct 2018 10:19:20 -0700 Subject: [PATCH 7/7] plumbing --- src/videojs-http-streaming.js | 1 + 1 file changed, 1 insertion(+) diff --git a/src/videojs-http-streaming.js b/src/videojs-http-streaming.js index 20f0c2b99..27e0f699c 100644 --- a/src/videojs-http-streaming.js +++ b/src/videojs-http-streaming.js @@ -358,6 +358,7 @@ class HlsHandler extends Component { }); this.bandwidth = this.options_.bandwidth; + this.limitRenditionByPlayerDimensions = this.options_.limitRenditionByPlayerDimensions; } /** * called when player.src gets called, handle a new source