From 3ae779f9f3676f6ea5b5e10a38712653e93057e0 Mon Sep 17 00:00:00 2001 From: brandonocasey Date: Wed, 24 Feb 2021 16:12:58 -0500 Subject: [PATCH 1/9] fix: Add exclude reason and skip duplicate playlist-unchanged --- src/master-playlist-controller.js | 14 +++++++++++++- 1 file changed, 13 insertions(+), 1 deletion(-) diff --git a/src/master-playlist-controller.js b/src/master-playlist-controller.js index d82ebcda6..74c615e10 100644 --- a/src/master-playlist-controller.js +++ b/src/master-playlist-controller.js @@ -472,6 +472,14 @@ export class MasterPlaylistController extends videojs.EventTarget { this.masterPlaylistLoader_.on('playlistunchanged', () => { const updatedPlaylist = this.masterPlaylistLoader_.media(); + + // ignore unchanged playlists that have already been + // excluded for not-changing. We likely just have a really slowly updating + // playlist. + if (updatedPlaylist.lastExcludeReason === 'playlist-unchanged') { + return; + } + const playlistOutdated = this.stuckAtPlaylistEnd_(updatedPlaylist); if (playlistOutdated) { @@ -480,7 +488,8 @@ export class MasterPlaylistController extends videojs.EventTarget { // one is updating (and give the player a chance to re-adjust to the // safe live point). this.blacklistCurrentPlaylist({ - message: 'Playlist no longer updating.' + message: 'Playlist no longer updating.', + reason: 'playlist-unchanged' }); // useful for monitoring QoS this.tech_.trigger('playliststuck'); @@ -1066,6 +1075,9 @@ export class MasterPlaylistController extends videojs.EventTarget { // Blacklist this playlist currentPlaylist.excludeUntil = Date.now() + (blacklistDuration * 1000); + if (error.reason) { + currentPlaylist.lastExcludeReason = error.reason; + } this.tech_.trigger('blacklistplaylist'); this.tech_.trigger({type: 'usage', name: 'vhs-rendition-blacklisted'}); this.tech_.trigger({type: 'usage', name: 'hls-rendition-blacklisted'}); From fae6f77e6fe77d8f36aea37e61e84a75c340e0c7 Mon Sep 17 00:00:00 2001 From: brandonocasey Date: Wed, 24 Feb 2021 16:32:56 -0500 Subject: [PATCH 2/9] better timers --- src/dash-playlist-loader.js | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/src/dash-playlist-loader.js b/src/dash-playlist-loader.js index 1feda8e1c..3bab4ce94 100644 --- a/src/dash-playlist-loader.js +++ b/src/dash-playlist-loader.js @@ -705,6 +705,9 @@ export default class DashPlaylistLoader extends EventTarget { } updateMinimumUpdatePeriodTimeout_() { + if (!this.isMaster_) { + return; + } // Clear existing timeout window.clearTimeout(this.minimumUpdatePeriodTimeout_); @@ -728,7 +731,7 @@ export default class DashPlaylistLoader extends EventTarget { // If we haven't yet selected a playlist, wait until then so we know the // target duration if (!this.media()) { - this.one('loadedplaylist', () => { + this.one('loadedmetadata', () => { createMUPTimeout(this.media().targetDuration * 1000); }); } else { @@ -791,9 +794,10 @@ export default class DashPlaylistLoader extends EventTarget { this.trigger('playlistunchanged'); } - if (!this.media().endList) { + if (!this.media().endList && !this.mediaUpdateTimeout) { this.mediaUpdateTimeout = window.setTimeout(() => { this.trigger('mediaupdatetimeout'); + this.mediaUpdateTimeout = null; }, refreshDelay(this.media(), Boolean(mediaChanged))); } From c05a92bb2c01efab6eee988c6c6db2a3bf16a067 Mon Sep 17 00:00:00 2001 From: brandonocasey Date: Wed, 24 Feb 2021 16:36:48 -0500 Subject: [PATCH 3/9] "private" exclude reason --- src/master-playlist-controller.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/master-playlist-controller.js b/src/master-playlist-controller.js index 74c615e10..ca2ef5560 100644 --- a/src/master-playlist-controller.js +++ b/src/master-playlist-controller.js @@ -476,7 +476,7 @@ export class MasterPlaylistController extends videojs.EventTarget { // ignore unchanged playlists that have already been // excluded for not-changing. We likely just have a really slowly updating // playlist. - if (updatedPlaylist.lastExcludeReason === 'playlist-unchanged') { + if (updatedPlaylist.lastExcludeReason_ === 'playlist-unchanged') { return; } @@ -1076,7 +1076,7 @@ export class MasterPlaylistController extends videojs.EventTarget { // Blacklist this playlist currentPlaylist.excludeUntil = Date.now() + (blacklistDuration * 1000); if (error.reason) { - currentPlaylist.lastExcludeReason = error.reason; + currentPlaylist.lastExcludeReason_ = error.reason; } this.tech_.trigger('blacklistplaylist'); this.tech_.trigger({type: 'usage', name: 'vhs-rendition-blacklisted'}); From ed74bcb3a109044c51a2da9dfa51d106f2b20b19 Mon Sep 17 00:00:00 2001 From: brandonocasey Date: Wed, 24 Feb 2021 16:47:29 -0500 Subject: [PATCH 4/9] better media update timeout again --- src/dash-playlist-loader.js | 18 +++++++++++++----- 1 file changed, 13 insertions(+), 5 deletions(-) diff --git a/src/dash-playlist-loader.js b/src/dash-playlist-loader.js index 3bab4ce94..6d4743deb 100644 --- a/src/dash-playlist-loader.js +++ b/src/dash-playlist-loader.js @@ -794,11 +794,19 @@ export default class DashPlaylistLoader extends EventTarget { this.trigger('playlistunchanged'); } - if (!this.media().endList && !this.mediaUpdateTimeout) { - this.mediaUpdateTimeout = window.setTimeout(() => { - this.trigger('mediaupdatetimeout'); - this.mediaUpdateTimeout = null; - }, refreshDelay(this.media(), Boolean(mediaChanged))); + if (!this.mediaUpdateTimeout) { + const createMediaUpdateTimeout = () => { + if (this.media().endList) { + return; + } + + this.mediaUpdateTimeout = window.setTimeout(() => { + this.trigger('mediaupdatetimeout'); + createMediaUpdateTimeout(); + }, refreshDelay(this.media(), Boolean(mediaChanged))); + }; + + createMediaUpdateTimeout(); } this.trigger('loadedplaylist'); From cf6776daa091e1f720684ca61849372b5756cc10 Mon Sep 17 00:00:00 2001 From: brandonocasey Date: Thu, 25 Feb 2021 11:15:11 -0500 Subject: [PATCH 5/9] better minimum update period timeout --- src/dash-playlist-loader.js | 71 +++++++++++++++++++------------------ 1 file changed, 37 insertions(+), 34 deletions(-) diff --git a/src/dash-playlist-loader.js b/src/dash-playlist-loader.js index 6d4743deb..1e96d0dc0 100644 --- a/src/dash-playlist-loader.js +++ b/src/dash-playlist-loader.js @@ -415,6 +415,9 @@ export default class DashPlaylistLoader extends EventTarget { window.clearTimeout(this.minimumUpdatePeriodTimeout_); window.clearTimeout(this.mediaRequest_); window.clearTimeout(this.mediaUpdateTimeout); + this.mediaUpdateTimeout = null; + this.mediaRequest_ = null; + this.minimumUpdatePeriodTimeout_ = null; this.off(); } @@ -507,7 +510,9 @@ export default class DashPlaylistLoader extends EventTarget { pause() { this.stopRequest(); window.clearTimeout(this.mediaUpdateTimeout); - window.clearTimeout(this.minimumUpdatePeriodTimeout_); + window.clearTimeout(this.masterPlaylistLoader_.minimumUpdatePeriodTimeout_); + this.masterPlaylistLoader_.minimumUpdatePeriodTimeout_ = null; + this.mediaUpdateTimeout = null; if (this.state === 'HAVE_NOTHING') { // If we pause the loader before any data has been retrieved, its as if we never // started, so reset to an unstarted state. @@ -517,7 +522,7 @@ export default class DashPlaylistLoader extends EventTarget { load(isFinalRendition) { window.clearTimeout(this.mediaUpdateTimeout); - window.clearTimeout(this.minimumUpdatePeriodTimeout_); + this.mediaUpdateTimeout = null; const media = this.media(); @@ -696,48 +701,46 @@ export default class DashPlaylistLoader extends EventTarget { this.masterPlaylistLoader_.srcUrl = location; } - // if the minimumUpdatePeriod was changed, update the minimumUpdatePeriodTimeout_ - if (!oldMaster || (newMaster && oldMaster.minimumUpdatePeriod !== newMaster.minimumUpdatePeriod)) { - this.updateMinimumUpdatePeriodTimeout_(); - } + // create a minimumUpdatePeriodTimeout_ if needed + this.updateMinimumUpdatePeriodTimeout_(); return Boolean(newMaster); } updateMinimumUpdatePeriodTimeout_() { - if (!this.isMaster_) { + const mpl = this.masterPlaylistLoader_; + + if (mpl.minimumUpdatePeriodTimeout_) { return; } - // Clear existing timeout - window.clearTimeout(this.minimumUpdatePeriodTimeout_); + const createMUPTimeout = () => { + let minimumUpdatePeriod = mpl.master && mpl.master.minimumUpdatePeriod; + + // If the minimumUpdatePeriod has a value of 0, that indicates that the current + // MPD has no future validity, so a new one will need to be acquired when new + // media segments are to be made available. Thus, we use the target duration + // in this case + if (minimumUpdatePeriod === 0) { + // If we haven't yet selected a playlist, wait until then so we know the + // target duration + if (!mpl.media()) { + mpl.one('loadedmetadata', createMUPTimeout); + } else { + minimumUpdatePeriod = mpl.media().targetDuration * 1000; + } + } + + if (typeof minimumUpdatePeriod !== 'number') { + return; + } - const createMUPTimeout = (mup) => { - this.minimumUpdatePeriodTimeout_ = window.setTimeout(() => { - this.trigger('minimumUpdatePeriod'); - createMUPTimeout(mup); - }, mup); + mpl.minimumUpdatePeriodTimeout_ = window.setTimeout(() => { + mpl.trigger('minimumUpdatePeriod'); + createMUPTimeout(); + }, minimumUpdatePeriod); }; - const minimumUpdatePeriod = this.masterPlaylistLoader_.master && this.masterPlaylistLoader_.master.minimumUpdatePeriod; - - if (minimumUpdatePeriod > 0) { - createMUPTimeout(minimumUpdatePeriod); - - // If the minimumUpdatePeriod has a value of 0, that indicates that the current - // MPD has no future validity, so a new one will need to be acquired when new - // media segments are to be made available. Thus, we use the target duration - // in this case - } else if (minimumUpdatePeriod === 0) { - // If we haven't yet selected a playlist, wait until then so we know the - // target duration - if (!this.media()) { - this.one('loadedmetadata', () => { - createMUPTimeout(this.media().targetDuration * 1000); - }); - } else { - createMUPTimeout(this.media().targetDuration * 1000); - } - } + createMUPTimeout(); } /** From b6cf366899a043a11fa0416aebb573b70007fe9e Mon Sep 17 00:00:00 2001 From: brandonocasey Date: Fri, 26 Feb 2021 11:09:38 -0500 Subject: [PATCH 6/9] faster mup updates --- src/dash-playlist-loader.js | 82 ++++++++++++++++++++++++------------- 1 file changed, 54 insertions(+), 28 deletions(-) diff --git a/src/dash-playlist-loader.js b/src/dash-playlist-loader.js index 1e96d0dc0..53f52c9ef 100644 --- a/src/dash-playlist-loader.js +++ b/src/dash-playlist-loader.js @@ -419,6 +419,11 @@ export default class DashPlaylistLoader extends EventTarget { this.mediaRequest_ = null; this.minimumUpdatePeriodTimeout_ = null; + if (this.masterPlaylistLoader_.createMupOnMedia_) { + this.off('loadedmetadata', this.masterPlaylistLoader_.createMupOnMedia_); + this.masterPlaylistLoader_.createMupOnMedia_ = null; + } + this.off(); } @@ -508,6 +513,10 @@ export default class DashPlaylistLoader extends EventTarget { } pause() { + if (this.masterPlaylistLoader_.createMupOnMedia_) { + this.off('loadedmetadata', this.masterPlaylistLoader_.createMupOnMedia_); + this.masterPlaylistLoader_.createMupOnMedia_ = null; + } this.stopRequest(); window.clearTimeout(this.mediaUpdateTimeout); window.clearTimeout(this.masterPlaylistLoader_.minimumUpdatePeriodTimeout_); @@ -701,8 +710,9 @@ export default class DashPlaylistLoader extends EventTarget { this.masterPlaylistLoader_.srcUrl = location; } - // create a minimumUpdatePeriodTimeout_ if needed - this.updateMinimumUpdatePeriodTimeout_(); + if (!oldMaster || (newMaster && newMaster.minimumUpdatePeriod !== oldMaster.minimumUpdatePeriod)) { + this.updateMinimumUpdatePeriodTimeout_(); + } return Boolean(newMaster); } @@ -710,37 +720,53 @@ export default class DashPlaylistLoader extends EventTarget { updateMinimumUpdatePeriodTimeout_() { const mpl = this.masterPlaylistLoader_; - if (mpl.minimumUpdatePeriodTimeout_) { - return; + // cancel any pending creation of mup on media + // a new one will be added if needed. + if (mpl.createMupOnMedia_) { + mpl.off('loadedmetadata', mpl.createMupOnMedia_); + mpl.createMupOnMedia_ = null; } - const createMUPTimeout = () => { - let minimumUpdatePeriod = mpl.master && mpl.master.minimumUpdatePeriod; - - // If the minimumUpdatePeriod has a value of 0, that indicates that the current - // MPD has no future validity, so a new one will need to be acquired when new - // media segments are to be made available. Thus, we use the target duration - // in this case - if (minimumUpdatePeriod === 0) { - // If we haven't yet selected a playlist, wait until then so we know the - // target duration - if (!mpl.media()) { - mpl.one('loadedmetadata', createMUPTimeout); - } else { - minimumUpdatePeriod = mpl.media().targetDuration * 1000; - } - } - if (typeof minimumUpdatePeriod !== 'number') { - return; + // clear any pending timeouts, master was just updated + // so if minimumUpdatePeriodTimeout_ is still valid + window.clearTimeout(mpl.minimumUpdatePeriodTimeout_); + mpl.minimumUpdatePeriodTimeout_ = null; + mpl.createMUPTimeout_(); + } + + createMUPTimeout_() { + const mpl = this.masterPlaylistLoader_; + let mup = mpl.master && mpl.master.minimumUpdatePeriod; + + // If the minimumUpdatePeriod has a value of 0, that indicates that the current + // MPD has no future validity, so a new one will need to be acquired when new + // media segments are to be made available. Thus, we use the target duration + // in this case + if (mup === 0) { + if (mpl.media()) { + mup = mpl.media().targetDuration * 1000; + } else { + mpl.createMupOnMedia_ = mpl.updateMinimumUpdatePeriodTimeout_; + mpl.one('loadedmetadata', mpl.createMupOnMedia_); } + } - mpl.minimumUpdatePeriodTimeout_ = window.setTimeout(() => { - mpl.trigger('minimumUpdatePeriod'); - createMUPTimeout(); - }, minimumUpdatePeriod); - }; + // if minimumUpdatePeriod is invalid or <= zero skip creating a timeout + if (typeof mup !== 'number' || mup <= 0) { + return; + } + + mpl.minimumUpdatePeriodTimeout_ = window.setTimeout(() => { + mpl.minimumUpdatePeriodTimeout_ = null; + mpl.createMUPTimeout_(); - createMUPTimeout(); + // if a minimumUpdatePeriodTimeout_ was created again + // then the minimumUpdatePeriod was valid + // trigger minimumUpdatePeriod + if (mpl.minimumUpdatePeriodTimeout_) { + mpl.trigger('minimumUpdatePeriod'); + } + }, mup); } /** From 2a5929cc738841a629a7abbc83efb241caf65800 Mon Sep 17 00:00:00 2001 From: brandonocasey Date: Wed, 3 Mar 2021 16:38:50 -0500 Subject: [PATCH 7/9] cr --- src/dash-playlist-loader.js | 33 ++++++++++++++++----------------- 1 file changed, 16 insertions(+), 17 deletions(-) diff --git a/src/dash-playlist-loader.js b/src/dash-playlist-loader.js index 53f52c9ef..72f210660 100644 --- a/src/dash-playlist-loader.js +++ b/src/dash-playlist-loader.js @@ -727,15 +727,12 @@ export default class DashPlaylistLoader extends EventTarget { mpl.createMupOnMedia_ = null; } - // clear any pending timeouts, master was just updated - // so if minimumUpdatePeriodTimeout_ is still valid - window.clearTimeout(mpl.minimumUpdatePeriodTimeout_); - mpl.minimumUpdatePeriodTimeout_ = null; - mpl.createMUPTimeout_(); - } + // clear any pending timeouts + if (mpl.minimumUpdatePeriodTimeout_) { + window.clearTimeout(mpl.minimumUpdatePeriodTimeout_); + mpl.minimumUpdatePeriodTimeout_ = null; + } - createMUPTimeout_() { - const mpl = this.masterPlaylistLoader_; let mup = mpl.master && mpl.master.minimumUpdatePeriod; // If the minimumUpdatePeriod has a value of 0, that indicates that the current @@ -751,21 +748,23 @@ export default class DashPlaylistLoader extends EventTarget { } } - // if minimumUpdatePeriod is invalid or <= zero skip creating a timeout + // if minimumUpdatePeriod is invalid or <= zero, which + // can happen when a live video becomes VOD. skip timeout + // creation. if (typeof mup !== 'number' || mup <= 0) { return; } + this.createMUPTimeout_(mup); + } + + createMUPTimeout_(mup) { + const mpl = this.masterPlaylistLoader_; + mpl.minimumUpdatePeriodTimeout_ = window.setTimeout(() => { mpl.minimumUpdatePeriodTimeout_ = null; - mpl.createMUPTimeout_(); - - // if a minimumUpdatePeriodTimeout_ was created again - // then the minimumUpdatePeriod was valid - // trigger minimumUpdatePeriod - if (mpl.minimumUpdatePeriodTimeout_) { - mpl.trigger('minimumUpdatePeriod'); - } + mpl.trigger('minimumUpdatePeriod'); + mpl.createMUPTimeout_(mup); }, mup); } From 4494e16712f6fe54bdd668e5a060e43b02877cb5 Mon Sep 17 00:00:00 2001 From: brandonocasey Date: Fri, 5 Mar 2021 11:49:07 -0500 Subject: [PATCH 8/9] log warning on < 0 minimumUpdatePeriod --- src/dash-playlist-loader.js | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/src/dash-playlist-loader.js b/src/dash-playlist-loader.js index 72f210660..810223972 100644 --- a/src/dash-playlist-loader.js +++ b/src/dash-playlist-loader.js @@ -19,6 +19,7 @@ import { } from './manifest'; import containerRequest from './util/container-request.js'; import {toUint8} from '@videojs/vhs-utils/es/byte-helpers'; +import logger from './util/logger'; const { EventTarget, mergeOptions } = videojs; @@ -291,6 +292,7 @@ export default class DashPlaylistLoader extends EventTarget { this.state = 'HAVE_NOTHING'; this.loadedPlaylists_ = {}; + this.logger_ = logger('DashPlaylistLoader'); // initialize the loader state // The masterPlaylistLoader will be created with a string @@ -752,6 +754,9 @@ export default class DashPlaylistLoader extends EventTarget { // can happen when a live video becomes VOD. skip timeout // creation. if (typeof mup !== 'number' || mup <= 0) { + if (mup < 0) { + this.logger_('minimumUpdatePeriod is less then 0 and invalid'); + } return; } From 9a3acef2b87de9990b0a8db2a307573cde57e21c Mon Sep 17 00:00:00 2001 From: brandonocasey Date: Fri, 5 Mar 2021 12:27:55 -0500 Subject: [PATCH 9/9] logger change --- src/dash-playlist-loader.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/dash-playlist-loader.js b/src/dash-playlist-loader.js index 810223972..1c96825dd 100644 --- a/src/dash-playlist-loader.js +++ b/src/dash-playlist-loader.js @@ -755,7 +755,7 @@ export default class DashPlaylistLoader extends EventTarget { // creation. if (typeof mup !== 'number' || mup <= 0) { if (mup < 0) { - this.logger_('minimumUpdatePeriod is less then 0 and invalid'); + this.logger_(`found invalid minimumUpdatePeriod of ${mup}, not setting a timeout`); } return; }