From e0d3e5138d736b6e19c4226fe0a1c416a0c01fcc Mon Sep 17 00:00:00 2001 From: brandonocasey Date: Thu, 4 Mar 2021 11:10:25 -0500 Subject: [PATCH 1/3] chore: usage on rendition change --- src/master-playlist-controller.js | 17 +++++++++++------ 1 file changed, 11 insertions(+), 6 deletions(-) diff --git a/src/master-playlist-controller.js b/src/master-playlist-controller.js index d82ebcda6..1064b79d9 100644 --- a/src/master-playlist-controller.js +++ b/src/master-playlist-controller.js @@ -301,10 +301,15 @@ export class MasterPlaylistController extends videojs.EventTarget { const nextPlaylist = this.selectPlaylist(); if (this.shouldSwitchToMedia_(nextPlaylist)) { - this.masterPlaylistLoader_.media(nextPlaylist); + this.switchMedia_(nextPlaylist, null, 'abr'); } } + switchMedia_(playlist, delay, cause) { + this.tech_.trigger({type: 'usage', name: `vhs-rendition-change-${cause}`}); + this.masterPlaylistLoader_.media(playlist); + } + /** * Start a timer that periodically calls checkABR_ * @@ -416,7 +421,7 @@ export class MasterPlaylistController extends videojs.EventTarget { this.initialMedia_ = selectedMedia; - this.masterPlaylistLoader_.media(this.initialMedia_); + this.switchMedia_(this.initialMedia_, null, 'initial'); // Under the standard case where a source URL is provided, loadedplaylist will // fire again since the playlist will be requested. In the case of vhs-json @@ -608,7 +613,7 @@ export class MasterPlaylistController extends videojs.EventTarget { const nextPlaylist = this.selectPlaylist(); if (this.shouldSwitchToMedia_(nextPlaylist)) { - this.masterPlaylistLoader_.media(nextPlaylist); + this.switchMedia_(this.initialMedia_, null, 'bandwidthupdate'); } this.tech_.trigger('bandwidthupdate'); @@ -734,7 +739,7 @@ export class MasterPlaylistController extends videojs.EventTarget { return; } - this.masterPlaylistLoader_.media(media); + this.switchMedia_(media, null, 'smooth-quality'); this.mainSegmentLoader_.resetLoader(); // don't need to reset audio as it is reset when media changes @@ -754,7 +759,7 @@ export class MasterPlaylistController extends videojs.EventTarget { return; } - this.masterPlaylistLoader_.media(media); + this.switchMedia_(media, null, 'fast-quality'); // Delete all buffered data to allow an immediate quality switch, then seek to give // the browser a kick to remove any cached frames from the previous rendtion (.04 seconds @@ -1106,7 +1111,7 @@ export class MasterPlaylistController extends videojs.EventTarget { (Date.now() - nextPlaylist.lastRequest) <= delayDuration; // delay if it's a final rendition or if the last refresh is sooner than half targetDuration - return this.masterPlaylistLoader_.media(nextPlaylist, isFinalRendition || shouldDelay); + return this.switchMedia_(nextPlaylist, isFinalRendition || shouldDelay, 'exclude'); } /** From f44cb93f041e72dca2a9d75a05e9b3d8e70e2b38 Mon Sep 17 00:00:00 2001 From: brandonocasey Date: Thu, 4 Mar 2021 11:13:31 -0500 Subject: [PATCH 2/3] log rendition switch and reason --- src/master-playlist-controller.js | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/master-playlist-controller.js b/src/master-playlist-controller.js index 1064b79d9..de74fa199 100644 --- a/src/master-playlist-controller.js +++ b/src/master-playlist-controller.js @@ -306,6 +306,9 @@ export class MasterPlaylistController extends videojs.EventTarget { } switchMedia_(playlist, delay, cause) { + const oldMedia = this.masterPlaylistLoader_.media(); + + this.logger_(`switch media ${oldMedia && (oldMedia.id || oldMedia.uri)} -> ${playlist.id || playlist.uri} from ${cause}`); this.tech_.trigger({type: 'usage', name: `vhs-rendition-change-${cause}`}); this.masterPlaylistLoader_.media(playlist); } From ee1abb86e8ba1a54358cfb7eea9540ab9f8d1d0d Mon Sep 17 00:00:00 2001 From: brandonocasey Date: Thu, 4 Mar 2021 13:38:06 -0500 Subject: [PATCH 3/3] test fixes --- src/master-playlist-controller.js | 26 ++++++++++++++----------- test/master-playlist-controller.test.js | 2 +- test/playback-watcher.test.js | 8 ++++++-- test/videojs-http-streaming.test.js | 8 ++++++-- 4 files changed, 28 insertions(+), 16 deletions(-) diff --git a/src/master-playlist-controller.js b/src/master-playlist-controller.js index de74fa199..549226a9c 100644 --- a/src/master-playlist-controller.js +++ b/src/master-playlist-controller.js @@ -301,16 +301,20 @@ export class MasterPlaylistController extends videojs.EventTarget { const nextPlaylist = this.selectPlaylist(); if (this.shouldSwitchToMedia_(nextPlaylist)) { - this.switchMedia_(nextPlaylist, null, 'abr'); + this.switchMedia_(nextPlaylist, 'abr'); } } - switchMedia_(playlist, delay, cause) { - const oldMedia = this.masterPlaylistLoader_.media(); + switchMedia_(playlist, cause, delay) { + const oldMedia = this.media(); + const oldId = oldMedia && (oldMedia.id || oldMedia.uri); + const newId = playlist.id || playlist.uri; - this.logger_(`switch media ${oldMedia && (oldMedia.id || oldMedia.uri)} -> ${playlist.id || playlist.uri} from ${cause}`); - this.tech_.trigger({type: 'usage', name: `vhs-rendition-change-${cause}`}); - this.masterPlaylistLoader_.media(playlist); + if (oldId && oldId !== newId) { + this.logger_(`switch media ${oldId} -> ${newId} from ${cause}`); + this.tech_.trigger({type: 'usage', name: `vhs-rendition-change-${cause}`}); + } + this.masterPlaylistLoader_.media(playlist, delay); } /** @@ -424,7 +428,7 @@ export class MasterPlaylistController extends videojs.EventTarget { this.initialMedia_ = selectedMedia; - this.switchMedia_(this.initialMedia_, null, 'initial'); + this.switchMedia_(this.initialMedia_, 'initial'); // Under the standard case where a source URL is provided, loadedplaylist will // fire again since the playlist will be requested. In the case of vhs-json @@ -616,7 +620,7 @@ export class MasterPlaylistController extends videojs.EventTarget { const nextPlaylist = this.selectPlaylist(); if (this.shouldSwitchToMedia_(nextPlaylist)) { - this.switchMedia_(this.initialMedia_, null, 'bandwidthupdate'); + this.switchMedia_(nextPlaylist, 'bandwidthupdate'); } this.tech_.trigger('bandwidthupdate'); @@ -742,7 +746,7 @@ export class MasterPlaylistController extends videojs.EventTarget { return; } - this.switchMedia_(media, null, 'smooth-quality'); + this.switchMedia_(media, 'smooth-quality'); this.mainSegmentLoader_.resetLoader(); // don't need to reset audio as it is reset when media changes @@ -762,7 +766,7 @@ export class MasterPlaylistController extends videojs.EventTarget { return; } - this.switchMedia_(media, null, 'fast-quality'); + this.switchMedia_(media, 'fast-quality'); // Delete all buffered data to allow an immediate quality switch, then seek to give // the browser a kick to remove any cached frames from the previous rendtion (.04 seconds @@ -1114,7 +1118,7 @@ export class MasterPlaylistController extends videojs.EventTarget { (Date.now() - nextPlaylist.lastRequest) <= delayDuration; // delay if it's a final rendition or if the last refresh is sooner than half targetDuration - return this.switchMedia_(nextPlaylist, isFinalRendition || shouldDelay, 'exclude'); + return this.switchMedia_(nextPlaylist, 'exclude', isFinalRendition || shouldDelay); } /** diff --git a/test/master-playlist-controller.test.js b/test/master-playlist-controller.test.js index f033eae79..f08b4ea30 100644 --- a/test/master-playlist-controller.test.js +++ b/test/master-playlist-controller.test.js @@ -1948,7 +1948,7 @@ QUnit.test('does not get stuck in a loop due to inconsistent network/caching', f const origMedia = playlistLoader.media.bind(playlistLoader); const mediaChanges = []; - mpc.masterPlaylistLoader_.media = (media) => { + mpc.switchMedia_ = (media) => { if (media) { mediaChanges.push(media); } diff --git a/test/playback-watcher.test.js b/test/playback-watcher.test.js index a60436416..32fb60929 100644 --- a/test/playback-watcher.test.js +++ b/test/playback-watcher.test.js @@ -1112,6 +1112,7 @@ loaderTypes.forEach(function(type) { expectedUsage['vhs-rendition-blacklisted'] = 1; expectedUsage['hls-rendition-blacklisted'] = 1; + // expectedUsage['vhs-rendition-change-exclude'] = 1; assert.deepEqual(this.usageEvents, expectedUsage, 'usage as expected'); @@ -1131,7 +1132,7 @@ loaderTypes.forEach(function(type) { const loader = this.mpc[`${type}SegmentLoader_`]; const playlists = this.mpc.master().playlists; - const excludeAndVerify = () => { + const excludeAndVerify = (last) => { let oldPlaylist; // this test only needs 9 appends, since we do an intial append @@ -1150,6 +1151,9 @@ loaderTypes.forEach(function(type) { expectedUsage[`vhs-${type}-download-exclusion`] = 1; expectedUsage['vhs-rendition-blacklisted'] = 1; expectedUsage['hls-rendition-blacklisted'] = 1; + if (!last) { + expectedUsage['vhs-rendition-change-exclude'] = 1; + } assert.deepEqual(this.usageEvents, expectedUsage, 'usage as expected'); this.usageEvents = {}; @@ -1190,7 +1194,7 @@ loaderTypes.forEach(function(type) { // exclude all playlists and verify while (i--) { - excludeAndVerify(); + excludeAndVerify((i === 0)); } }); diff --git a/test/videojs-http-streaming.test.js b/test/videojs-http-streaming.test.js index 53eff2e69..d98aa457d 100644 --- a/test/videojs-http-streaming.test.js +++ b/test/videojs-http-streaming.test.js @@ -1937,7 +1937,9 @@ QUnit.test('blacklists fmp4 playlists by browser support', function(assert) { assert.strictEqual(typeof playlists[2].excludeUntil, 'undefined', 'did not blacklist second playlist'); assert.deepEqual(debugLogs, [ `Internal problem encountered with playlist ${playlists[0].id}. browser does not support codec(s): "hvc1". Switching to playlist ${playlists[1].id}.`, - `Internal problem encountered with playlist ${playlists[1].id}. browser does not support codec(s): "ac-3". Switching to playlist ${playlists[2].id}.` + `switch media ${playlists[0].id} -> ${playlists[1].id} from exclude`, + `Internal problem encountered with playlist ${playlists[1].id}. browser does not support codec(s): "ac-3". Switching to playlist ${playlists[2].id}.`, + `switch media ${playlists[1].id} -> ${playlists[2].id} from exclude` ], 'debug log as expected'); window.MediaSource.isTypeSupported = oldIsTypeSupported; @@ -2008,7 +2010,9 @@ QUnit.test('blacklists ts playlists by muxer support', function(assert) { assert.strictEqual(typeof playlists[2].excludeUntil, 'undefined', 'did not blacklist third playlist'); assert.deepEqual(debugLogs, [ `Internal problem encountered with playlist ${playlists[0].id}. muxer does not support codec(s): "hvc1". Switching to playlist ${playlists[1].id}.`, - `Internal problem encountered with playlist ${playlists[1].id}. muxer does not support codec(s): "ac-3". Switching to playlist ${playlists[2].id}.` + `switch media ${playlists[0].id} -> ${playlists[1].id} from exclude`, + `Internal problem encountered with playlist ${playlists[1].id}. muxer does not support codec(s): "ac-3". Switching to playlist ${playlists[2].id}.`, + `switch media ${playlists[1].id} -> ${playlists[2].id} from exclude` ], 'debug log as expected'); window.MediaSource.isTypeSupported = oldIsTypeSupported;