From 4a7541c4b5acab4d044850da502e170ad35debea Mon Sep 17 00:00:00 2001 From: brandonocasey Date: Tue, 9 Feb 2021 14:18:13 -0500 Subject: [PATCH 1/3] fix: live dash segment changes should be considered a playlist update --- src/dash-playlist-loader.js | 4 +- src/playlist-loader.js | 60 ++++++++++-- test/dash-playlist-loader.test.js | 158 ++++++++++++++++++++++++++++++ test/manifests/dashByterange.mpd | 58 +++++++++++ 4 files changed, 272 insertions(+), 8 deletions(-) create mode 100644 test/manifests/dashByterange.mpd diff --git a/src/dash-playlist-loader.js b/src/dash-playlist-loader.js index 2fb50011e..ae155f342 100644 --- a/src/dash-playlist-loader.js +++ b/src/dash-playlist-loader.js @@ -92,7 +92,7 @@ export const updateMaster = (oldMaster, newMaster, sidxMapping) => { addSidxSegmentsToPlaylist(playlist, sidxMapping[sidxKey].sidx, playlist.sidx.resolvedUri); } } - const playlistUpdate = updatePlaylist(update, playlist); + const playlistUpdate = updatePlaylist(update, playlist, true); if (playlistUpdate) { update = playlistUpdate; @@ -104,7 +104,7 @@ export const updateMaster = (oldMaster, newMaster, sidxMapping) => { forEachMediaGroup(newMaster, (properties, type, group, label) => { if (properties.playlists && properties.playlists.length) { const id = properties.playlists[0].id; - const playlistUpdate = updatePlaylist(update, properties.playlists[0]); + const playlistUpdate = updatePlaylist(update, properties.playlists[0], true); if (playlistUpdate) { update = playlistUpdate; diff --git a/src/playlist-loader.js b/src/playlist-loader.js index 3ad659588..c753f3240 100644 --- a/src/playlist-loader.js +++ b/src/playlist-loader.js @@ -69,7 +69,7 @@ export const resolveSegmentUris = (segment, baseUri) => { * master playlist with the updated media playlist merged in, or * null if the merge produced no change. */ -export const updateMaster = (master, media) => { +export const updateMaster = (master, media, dash = false) => { const result = mergeOptions(master, {}); const playlist = result.playlists[media.id]; @@ -79,11 +79,59 @@ export const updateMaster = (master, media) => { // consider the playlist unchanged if the number of segments is equal, the media // sequence number is unchanged, and this playlist hasn't become the end of the playlist - if (playlist.segments && - media.segments && - playlist.segments.length === media.segments.length && - playlist.endList === media.endList && - playlist.mediaSequence === media.mediaSequence) { + let unchanged = playlist.segments && + media.segments && + playlist.segments.length === media.segments.length && + playlist.endList === media.endList && + playlist.mediaSequence === media.mediaSequence; + + // for dash mediaSequence and segment lengths will often match as + // mediaSequence is almost always 1 and the number of segments generated for a + // given time is often the same. So we need to make sure that the underlying segments are + // different. + if (dash && unchanged) { + const oldSidx = playlist.sidx; + const newSidx = media.sidx; + + // if sidx changed then the playlists are different. + if (oldSidx && newSidx && (oldSidx.offset !== newSidx.offset || oldSidx.length !== newSidx.length)) { + unchanged = false; + } else if ((!oldSidx && newSidx) || (oldSidx && !newSidx)) { + unchanged = false; + } else { + for (let i = 0; i < playlist.segments.length; i++) { + const oldSegment = playlist.segments[i]; + const newSegment = media.segments[i]; + + // if uris are different between segments there was a change + if (oldSegment.uri !== newSegment.uri) { + unchanged = false; + continue; + } + + // neither segment has a byternage, there will be no byterange change. + if (!oldSegment.byterange && !newSegment.byterange) { + continue; + } + const oldByterange = oldSegment.byterange; + const newByterange = newSegment.byterange; + + // if byterange only exists on one of the segments, there was a change. + if ((oldByterange && !newByterange) || (!oldByterange && newByterange)) { + unchanged = false; + continue; + } + + // if both segments have byterange with different offsets, there was a change. + if (oldByterange.offset !== newByterange.offset || oldByterange.length !== newByterange.length) { + unchanged = false; + continue; + } + } + } + } + + if (unchanged) { return null; } diff --git a/test/dash-playlist-loader.test.js b/test/dash-playlist-loader.test.js index 0ec06922b..7bd47b33f 100644 --- a/test/dash-playlist-loader.test.js +++ b/test/dash-playlist-loader.test.js @@ -1684,6 +1684,164 @@ QUnit.test('refreshXml_: updates media playlist reference if master changed', fu ); }); +QUnit.test('refreshXml_: updates playlists if segment uri changed, but media sequence did not', function(assert) { + const loader = new DashPlaylistLoader('dash.mpd', this.fakeVhs); + + loader.load(); + this.standardXHRResponse(this.requests.shift()); + + const oldMaster = loader.master; + const oldMedia = loader.media(); + + // change segment uris + const newMasterXml = loader.masterXml_ + .replace(/\$RepresentationID\$/g, '$RepresentationID$-foo') + .replace('media="segment-$Number$.mp4"', 'media="segment-foo$Number$.mp4"'); + + loader.refreshXml_(); + + assert.strictEqual(this.requests.length, 1, 'manifest is being requested'); + + this.requests.shift().respond(200, null, newMasterXml); + + const newMaster = loader.master; + const newMedia = loader.media(); + + assert.notEqual(newMaster, oldMaster, 'master changed'); + assert.notEqual(newMedia, oldMedia, 'media changed'); + assert.equal( + newMedia, + newMaster.playlists[newMedia.id], + 'media from updated master' + ); +}); + +QUnit.test('refreshXml_: updates playlists if sidx changed', function(assert) { + const loader = new DashPlaylistLoader('dash-sidx.mpd', this.fakeVhs); + + loader.load(); + this.standardXHRResponse(this.requests.shift()); + this.standardXHRResponse(this.requests.shift(), mp4VideoInitSegment().subarray(0, 10)); + this.standardXHRResponse(this.requests.shift(), sidxResponse()); + + const oldMaster = loader.master; + const oldMedia = loader.media(); + + const newMasterXml = loader.masterXml_ + .replace(/indexRange="200-399"/g, 'indexRange="500-699"'); + + loader.refreshXml_(); + + assert.strictEqual(this.requests.length, 1, 'manifest is being requested'); + + this.standardXHRResponse(this.requests.shift(), newMasterXml); + + const newMaster = loader.master; + const newMedia = loader.media(); + + assert.notEqual(newMaster, oldMaster, 'master changed'); + assert.notEqual(newMedia, oldMedia, 'media changed'); + assert.equal( + newMedia, + newMaster.playlists[newMedia.id], + 'media from updated master' + ); +}); + +QUnit.test('refreshXml_: updates playlists if sidx removed', function(assert) { + const loader = new DashPlaylistLoader('dash-sidx.mpd', this.fakeVhs); + + loader.load(); + this.standardXHRResponse(this.requests.shift()); + this.standardXHRResponse(this.requests.shift(), mp4VideoInitSegment().subarray(0, 10)); + this.standardXHRResponse(this.requests.shift(), sidxResponse()); + + const oldMaster = loader.master; + const oldMedia = loader.media(); + + const newMasterXml = loader.masterXml_ + .replace(/indexRange="200-399"/g, ''); + + loader.refreshXml_(); + + assert.strictEqual(this.requests.length, 1, 'manifest is being requested'); + + this.standardXHRResponse(this.requests.shift(), newMasterXml); + + const newMaster = loader.master; + const newMedia = loader.media(); + + assert.notEqual(newMaster, oldMaster, 'master changed'); + assert.notEqual(newMedia, oldMedia, 'media changed'); + assert.equal( + newMedia, + newMaster.playlists[newMedia.id], + 'media from updated master' + ); +}); + +QUnit.test('refreshXml_: updates playlists if only segment byteranges change', function(assert) { + const loader = new DashPlaylistLoader('dashByterange.mpd', this.fakeVhs); + + loader.load(); + this.standardXHRResponse(this.requests.shift()); + + const oldMaster = loader.master; + const oldMedia = loader.media(); + + const newMasterXml = loader.masterXml_ + .replace('mediaRange="12883295-13124492"', 'mediaRange="12883296-13124492"'); + + loader.refreshXml_(); + + assert.strictEqual(this.requests.length, 1, 'manifest is being requested'); + + this.standardXHRResponse(this.requests.shift(), newMasterXml); + + const newMaster = loader.master; + const newMedia = loader.media(); + + assert.notEqual(newMaster, oldMaster, 'master changed'); + assert.notEqual(newMedia, oldMedia, 'media changed'); + assert.equal( + newMedia, + newMaster.playlists[newMedia.id], + 'media from updated master' + ); +}); + +QUnit.test('refreshXml_: updates playlists if sidx removed', function(assert) { + const loader = new DashPlaylistLoader('dash-sidx.mpd', this.fakeVhs); + + loader.load(); + this.standardXHRResponse(this.requests.shift()); + this.standardXHRResponse(this.requests.shift(), mp4VideoInitSegment().subarray(0, 10)); + this.standardXHRResponse(this.requests.shift(), sidxResponse()); + + const oldMaster = loader.master; + const oldMedia = loader.media(); + + const newMasterXml = loader.masterXml_ + .replace(/indexRange="200-399"/g, ''); + + loader.refreshXml_(); + + assert.strictEqual(this.requests.length, 1, 'manifest is being requested'); + + this.standardXHRResponse(this.requests.shift(), newMasterXml); + + const newMaster = loader.master; + const newMedia = loader.media(); + + assert.notEqual(newMaster, oldMaster, 'master changed'); + assert.notEqual(newMedia, oldMedia, 'media changed'); + assert.equal( + newMedia, + newMaster.playlists[newMedia.id], + 'media from updated master' + ); +}); + QUnit.test('addSidxSegments_: updates master with sidx information', function(assert) { const loader = new DashPlaylistLoader('dash.mpd', this.fakeVhs); const sidxData = sidxResponse(); diff --git a/test/manifests/dashByterange.mpd b/test/manifests/dashByterange.mpd new file mode 100644 index 000000000..8499b39cb --- /dev/null +++ b/test/manifests/dashByterange.mpd @@ -0,0 +1,58 @@ + + +http://www-itec.uni-klu.ac.at/ftp/datasets/mmsys12/BigBuckBunny/bunny_15s/ + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + From a8e6f8e267bf681eb4663f5fdb0a16f4fa0b0082 Mon Sep 17 00:00:00 2001 From: brandonocasey Date: Wed, 10 Feb 2021 11:50:18 -0500 Subject: [PATCH 2/3] cr --- src/dash-playlist-loader.js | 68 +++++++++++++++++++++++++++++++++++-- src/playlist-loader.js | 66 ++++++----------------------------- 2 files changed, 75 insertions(+), 59 deletions(-) diff --git a/src/dash-playlist-loader.js b/src/dash-playlist-loader.js index ae155f342..1feda8e1c 100644 --- a/src/dash-playlist-loader.js +++ b/src/dash-playlist-loader.js @@ -6,7 +6,8 @@ import { } from 'mpd-parser'; import { refreshDelay, - updateMaster as updatePlaylist + updateMaster as updatePlaylist, + isPlaylistUnchanged } from './playlist-loader'; import { resolveUrl, resolveManifestRedirect } from './resolve-url'; import parseSidx from 'mux.js/lib/tools/parse-sidx'; @@ -21,6 +22,67 @@ import {toUint8} from '@videojs/vhs-utils/es/byte-helpers'; const { EventTarget, mergeOptions } = videojs; +const dashPlaylistUnchanged = function(a, b) { + if (!isPlaylistUnchanged(a, b)) { + return false; + } + + // for dash the above check will often return true in scenarios where + // the playlist actually has changed because mediaSequence isn't a + // dash thing, and we often set it to 1. So if the playlists have the same amount + // of segments we return true. + // So for dash we need to make sure that the underlying segments are different. + + // if sidx changed then the playlists are different. + if (a.sidx && b.sidx && (a.sidx.offset !== b.sidx.offset || a.sidx.length !== b.sidx.length)) { + return false; + } else if ((!a.sidx && b.sidx) || (a.sidx && !b.sidx)) { + return false; + } + + // one or the other does not have segments + // there was a change. + if (a.segments && !b.segments || !a.segments && b.segments) { + return false; + } + + // neither has segments nothing changed + if (!a.segments && !b.segments) { + return true; + } + + // check segments themselves + for (let i = 0; i < a.segments.length; i++) { + const aSegment = a.segments[i]; + const bSegment = b.segments[i]; + + // if uris are different between segments there was a change + if (aSegment.uri !== bSegment.uri) { + return false; + } + + // neither segment has a byterange, there will be no byterange change. + if (!aSegment.byterange && !bSegment.byterange) { + continue; + } + const aByterange = aSegment.byterange; + const bByterange = bSegment.byterange; + + // if byterange only exists on one of the segments, there was a change. + if ((aByterange && !bByterange) || (!aByterange && bByterange)) { + return false; + } + + // if both segments have byterange with different offsets, there was a change. + if (aByterange.offset !== bByterange.offset || aByterange.length !== bByterange.length) { + return false; + } + } + + // if everything was the same with segments, this is the same playlist. + return true; +}; + /** * Parses the master XML string and updates playlist URI references. * @@ -92,7 +154,7 @@ export const updateMaster = (oldMaster, newMaster, sidxMapping) => { addSidxSegmentsToPlaylist(playlist, sidxMapping[sidxKey].sidx, playlist.sidx.resolvedUri); } } - const playlistUpdate = updatePlaylist(update, playlist, true); + const playlistUpdate = updatePlaylist(update, playlist, dashPlaylistUnchanged); if (playlistUpdate) { update = playlistUpdate; @@ -104,7 +166,7 @@ export const updateMaster = (oldMaster, newMaster, sidxMapping) => { forEachMediaGroup(newMaster, (properties, type, group, label) => { if (properties.playlists && properties.playlists.length) { const id = properties.playlists[0].id; - const playlistUpdate = updatePlaylist(update, properties.playlists[0], true); + const playlistUpdate = updatePlaylist(update, properties.playlists[0], dashPlaylistUnchanged); if (playlistUpdate) { update = playlistUpdate; diff --git a/src/playlist-loader.js b/src/playlist-loader.js index c753f3240..aca1b8dba 100644 --- a/src/playlist-loader.js +++ b/src/playlist-loader.js @@ -57,6 +57,14 @@ export const resolveSegmentUris = (segment, baseUri) => { } }; +// consider the playlist unchanged if the playlist object is the same or +// the number of segments is equal, the media sequence number is unchanged, +// and this playlist hasn't become the end of the playlist +export const isPlaylistUnchanged = (a, b) => a === b || + (a.segments && b.segments && a.segments.length === b.segments.length && + a.endList === b.endList && + a.mediaSequence === b.mediaSequence); + /** * Returns a new master playlist that is the result of merging an * updated media playlist into the original version. If the @@ -69,7 +77,7 @@ export const resolveSegmentUris = (segment, baseUri) => { * master playlist with the updated media playlist merged in, or * null if the merge produced no change. */ -export const updateMaster = (master, media, dash = false) => { +export const updateMaster = (master, media, unchangedCheck = isPlaylistUnchanged) => { const result = mergeOptions(master, {}); const playlist = result.playlists[media.id]; @@ -77,61 +85,7 @@ export const updateMaster = (master, media, dash = false) => { return null; } - // consider the playlist unchanged if the number of segments is equal, the media - // sequence number is unchanged, and this playlist hasn't become the end of the playlist - let unchanged = playlist.segments && - media.segments && - playlist.segments.length === media.segments.length && - playlist.endList === media.endList && - playlist.mediaSequence === media.mediaSequence; - - // for dash mediaSequence and segment lengths will often match as - // mediaSequence is almost always 1 and the number of segments generated for a - // given time is often the same. So we need to make sure that the underlying segments are - // different. - if (dash && unchanged) { - const oldSidx = playlist.sidx; - const newSidx = media.sidx; - - // if sidx changed then the playlists are different. - if (oldSidx && newSidx && (oldSidx.offset !== newSidx.offset || oldSidx.length !== newSidx.length)) { - unchanged = false; - } else if ((!oldSidx && newSidx) || (oldSidx && !newSidx)) { - unchanged = false; - } else { - for (let i = 0; i < playlist.segments.length; i++) { - const oldSegment = playlist.segments[i]; - const newSegment = media.segments[i]; - - // if uris are different between segments there was a change - if (oldSegment.uri !== newSegment.uri) { - unchanged = false; - continue; - } - - // neither segment has a byternage, there will be no byterange change. - if (!oldSegment.byterange && !newSegment.byterange) { - continue; - } - const oldByterange = oldSegment.byterange; - const newByterange = newSegment.byterange; - - // if byterange only exists on one of the segments, there was a change. - if ((oldByterange && !newByterange) || (!oldByterange && newByterange)) { - unchanged = false; - continue; - } - - // if both segments have byterange with different offsets, there was a change. - if (oldByterange.offset !== newByterange.offset || oldByterange.length !== newByterange.length) { - unchanged = false; - continue; - } - } - } - } - - if (unchanged) { + if (unchangedCheck(playlist, media)) { return null; } From f4611b93fa9804ffa4c47ec1572552fdb1d09c76 Mon Sep 17 00:00:00 2001 From: Brandon Casey <2381475+brandonocasey@users.noreply.github.com> Date: Thu, 18 Feb 2021 09:32:13 -0800 Subject: [PATCH 3/3] cr spacing change Co-authored-by: Gary Katsevman --- src/playlist-loader.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/playlist-loader.js b/src/playlist-loader.js index aca1b8dba..2f77b3400 100644 --- a/src/playlist-loader.js +++ b/src/playlist-loader.js @@ -62,8 +62,8 @@ export const resolveSegmentUris = (segment, baseUri) => { // and this playlist hasn't become the end of the playlist export const isPlaylistUnchanged = (a, b) => a === b || (a.segments && b.segments && a.segments.length === b.segments.length && - a.endList === b.endList && - a.mediaSequence === b.mediaSequence); + a.endList === b.endList && + a.mediaSequence === b.mediaSequence); /** * Returns a new master playlist that is the result of merging an