From 09dcabc39d467145ce42fb19e49f72fac11a2d08 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=81lvaro=20Velad=20Galv=C3=A1n?= Date: Fri, 15 Nov 2024 13:23:40 +0100 Subject: [PATCH] fix(DASH): Fix playback after DASH period eviction (#7603) Fixes https://github.com/shaka-project/shaka-player/issues/7516 in a simpler way Reverts https://github.com/shaka-project/shaka-player/commit/5eff0384d04e22a971336c90aae0a83db2a451d5 Reverts https://github.com/shaka-project/shaka-player/commit/037193c2d601e02c8611a4527dcb04d32adbbd27 --- externs/shaka/manifest_parser.js | 3 - lib/dash/dash_parser.js | 16 ++- lib/media/streaming_engine.js | 26 ---- lib/offline/storage.js | 1 - lib/player.js | 7 - lib/util/periods.js | 25 +--- .../dash_parser_content_protection_unit.js | 1 - test/dash/dash_parser_live_unit.js | 1 - test/dash/dash_parser_manifest_unit.js | 1 - test/dash/dash_parser_patch_unit.js | 1 - test/dash/dash_parser_segment_base_unit.js | 1 - test/dash/dash_parser_segment_list_unit.js | 1 - .../dash/dash_parser_segment_template_unit.js | 1 - test/hls/hls_live_unit.js | 1 - test/hls/hls_parser_unit.js | 1 - test/media/streaming_engine_integration.js | 2 - test/media/streaming_engine_unit.js | 1 - test/mss/mss_parser_unit.js | 1 - test/test/util/dash_parser_util.js | 2 - test/test/util/mss_parser_util.js | 2 - test/util/content_steering_manager_unit.js | 1 - test/util/periods_unit.js | 125 ------------------ 22 files changed, 12 insertions(+), 209 deletions(-) diff --git a/externs/shaka/manifest_parser.js b/externs/shaka/manifest_parser.js index 22ec3d6ffe..6eaa98d1e7 100644 --- a/externs/shaka/manifest_parser.js +++ b/externs/shaka/manifest_parser.js @@ -140,7 +140,6 @@ shaka.extern.ManifestParser = class { * getBandwidthEstimate: function():number, * onMetadata: function(string, number, ?number, * !Array.), - * closeSegmentIndex: function(shaka.extern.Stream, function()), * disableStream: function(!shaka.extern.Stream), * addFont: function(string, string) * }} @@ -187,8 +186,6 @@ shaka.extern.ManifestParser = class { * @property {function(!shaka.extern.Stream)} disableStream * Called to temporarily disable a stream i.e. disabling all variant * containing said stream. - * @property {function(!shaka.extern.Stream, function())} closeSegmentIndex - * Called to close a segment index. * @property {function(string, string)} addFont * Called when a new font needs to be added. * @exportDoc diff --git a/lib/dash/dash_parser.js b/lib/dash/dash_parser.js index bb0ec502d2..a816901781 100644 --- a/lib/dash/dash_parser.js +++ b/lib/dash/dash_parser.js @@ -239,9 +239,6 @@ shaka.dash.DashParser = class { this.lowLatencyMode_ = playerInterface.isLowLatencyMode(); this.manifestUris_ = [uri]; this.playerInterface_ = playerInterface; - if (this.periodCombiner_) { - this.periodCombiner_.setPlayerInterface(this.playerInterface_); - } const updateDelay = await this.requestManifest_(); @@ -684,7 +681,6 @@ shaka.dash.DashParser = class { } this.manifestPatchContext_.type = mpdType; - this.cleanStreamMap_(); /** @type {!shaka.media.PresentationTimeline} */ let presentationTimeline; if (this.manifest_) { @@ -861,6 +857,8 @@ shaka.dash.DashParser = class { // after period combining, while we still have a direct reference, so that // any new streams will appear in the period combiner. this.playerInterface_.makeTextStreamsForClosedCaptions(this.manifest_); + + this.cleanStreamMap_(); } /** @@ -1504,13 +1502,21 @@ shaka.dash.DashParser = class { return !this.lastManifestUpdatePeriodIds_.includes(pId); }); for (const pId of diffPeriodsIDs) { + let shouldDeleteIndex = true; for (const contextId of this.indexStreamMap_[pId]) { + const stream = this.streamMap_[contextId]; + if (stream.segmentIndex && !stream.segmentIndex.isEmpty()) { + shouldDeleteIndex = false; + continue; + } if (this.periodCombiner_) { this.periodCombiner_.deleteStream(this.streamMap_[contextId], pId); } delete this.streamMap_[contextId]; } - delete this.indexStreamMap_[pId]; + if (shouldDeleteIndex) { + delete this.indexStreamMap_[pId]; + } } } diff --git a/lib/media/streaming_engine.js b/lib/media/streaming_engine.js index 196c0af21b..eca072ed05 100644 --- a/lib/media/streaming_engine.js +++ b/lib/media/streaming_engine.js @@ -513,32 +513,6 @@ shaka.media.StreamingEngine = class { } } - /** - * closeSegmentIndex if the stream is not active - * @param {shaka.extern.Stream} stream - * @param {function()} closeSegmentIndex - */ - closeSegmentIndex(stream, closeSegmentIndex) { - const type = /** @type {!shaka.util.ManifestParserUtils.ContentType} */ - (stream.type); - const mediaState = this.mediaStates_.get(type); - if (mediaState) { - /** - * Closes the segmentIndex only if the stream is not active. - * If the stream is active the eviction will be peformed - * by the next onUpdate_(). - */ - const matchedStreams = mediaState.stream.matchedStreams; - const found = matchedStreams.find((match) => match.id === stream.id); - if (!found) { - closeSegmentIndex(); - } - } else { - // type of stream not active - closeSegmentIndex(); - } - } - /** * Handles deferred releases of old SegmentIndexes for the mediaState's diff --git a/lib/offline/storage.js b/lib/offline/storage.js index 9285f9c977..e73a30822e 100644 --- a/lib/offline/storage.js +++ b/lib/offline/storage.js @@ -1217,7 +1217,6 @@ shaka.offline.Storage = class { onManifestUpdated: () => {}, getBandwidthEstimate: () => config.abr.defaultBandwidthEstimate, onMetadata: () => {}, - closeSegmentIndex: (stream, closeSegmentIndex) => {}, disableStream: (stream) => {}, addFont: (name, url) => {}, }; diff --git a/lib/player.js b/lib/player.js index 49aded3d3e..be74743d74 100644 --- a/lib/player.js +++ b/lib/player.js @@ -2169,13 +2169,6 @@ shaka.Player = class extends shaka.util.FakeEventTarget { }); } }, - closeSegmentIndex: (stream, closeSegmentIndex) => { - if (this.streamingEngine_) { - this.streamingEngine_.closeSegmentIndex(stream, closeSegmentIndex); - } else { - closeSegmentIndex(); - } - }, disableStream: (stream) => this.disableStream( stream, this.config_.streaming.maxDisabledTime), addFont: (name, url) => this.addFont(name, url), diff --git a/lib/util/periods.js b/lib/util/periods.js index bc6acb76fd..2055e944c8 100644 --- a/lib/util/periods.js +++ b/lib/util/periods.js @@ -48,9 +48,6 @@ shaka.util.PeriodCombiner = class { /** @private {boolean} */ this.useStreamOnce_ = false; - /** @private {?shaka.extern.ManifestParser.PlayerInterface} */ - this.playerInterface_ = null; - /** * The IDs of the periods we have already used to generate streams. * This helps us identify the periods which have been added when a live @@ -83,15 +80,6 @@ shaka.util.PeriodCombiner = class { this.usedPeriodIds_.clear(); } - /** - * @param {?shaka.extern.ManifestParser.PlayerInterface} playerInterface - * - * @export - */ - setPlayerInterface(playerInterface) { - this.playerInterface_ = playerInterface; - } - /** * @return {!Array.} * @@ -167,19 +155,8 @@ shaka.util.PeriodCombiner = class { } } if (stream.segmentIndex) { - if (this.playerInterface_) { - const closeSegmentIndex = () => { - stream.closeSegmentIndex(); - }; - - this.playerInterface_.closeSegmentIndex( - stream, - closeSegmentIndex); - } else { - stream.closeSegmentIndex(); - } + stream.closeSegmentIndex(); } - this.usedPeriodIds_.delete(periodId); } diff --git a/test/dash/dash_parser_content_protection_unit.js b/test/dash/dash_parser_content_protection_unit.js index 4ec124ec7e..1a390b8027 100644 --- a/test/dash/dash_parser_content_protection_unit.js +++ b/test/dash/dash_parser_content_protection_unit.js @@ -47,7 +47,6 @@ describe('DashParser ContentProtection', () => { onManifestUpdated: () => {}, getBandwidthEstimate: () => 1e6, onMetadata: () => {}, - closeSegmentIndex: (stream, closeSegmentIndex) => {}, disableStream: (stream) => {}, addFont: (name, url) => {}, }; diff --git a/test/dash/dash_parser_live_unit.js b/test/dash/dash_parser_live_unit.js index c849349f6b..5a3f691a84 100644 --- a/test/dash/dash_parser_live_unit.js +++ b/test/dash/dash_parser_live_unit.js @@ -39,7 +39,6 @@ describe('DashParser Live', () => { onManifestUpdated: () => {}, getBandwidthEstimate: () => 1e6, onMetadata: () => {}, - closeSegmentIndex: (stream, closeSegmentIndex) => {}, disableStream: (stream) => {}, addFont: (name, url) => {}, }; diff --git a/test/dash/dash_parser_manifest_unit.js b/test/dash/dash_parser_manifest_unit.js index 43fdabb450..74bfb04732 100644 --- a/test/dash/dash_parser_manifest_unit.js +++ b/test/dash/dash_parser_manifest_unit.js @@ -60,7 +60,6 @@ describe('DashParser Manifest', () => { onManifestUpdated: () => {}, getBandwidthEstimate: () => 1e6, onMetadata: () => {}, - closeSegmentIndex: (stream, closeSegmentIndex) => {}, disableStream: (stream) => {}, addFont: shaka.test.Util.spyFunc(addFontSpy), }; diff --git a/test/dash/dash_parser_patch_unit.js b/test/dash/dash_parser_patch_unit.js index 6ff8d1c231..70e7b54c23 100644 --- a/test/dash/dash_parser_patch_unit.js +++ b/test/dash/dash_parser_patch_unit.js @@ -50,7 +50,6 @@ describe('DashParser Patch', () => { onManifestUpdated: () => {}, getBandwidthEstimate: () => 1e6, onMetadata: () => {}, - closeSegmentIndex: (stream, closeSegmentIndex) => {}, disableStream: (stream) => {}, addFont: (name, url) => {}, }; diff --git a/test/dash/dash_parser_segment_base_unit.js b/test/dash/dash_parser_segment_base_unit.js index cdbff25593..2295722289 100644 --- a/test/dash/dash_parser_segment_base_unit.js +++ b/test/dash/dash_parser_segment_base_unit.js @@ -43,7 +43,6 @@ describe('DashParser SegmentBase', () => { onManifestUpdated: () => {}, getBandwidthEstimate: () => 1e6, onMetadata: () => {}, - closeSegmentIndex: (stream, closeSegmentIndex) => {}, disableStream: (stream) => {}, addFont: (name, url) => {}, }; diff --git a/test/dash/dash_parser_segment_list_unit.js b/test/dash/dash_parser_segment_list_unit.js index f9bea13d63..a212426623 100644 --- a/test/dash/dash_parser_segment_list_unit.js +++ b/test/dash/dash_parser_segment_list_unit.js @@ -353,7 +353,6 @@ describe('DashParser SegmentList', () => { onManifestUpdated: () => {}, getBandwidthEstimate: () => 1e6, onMetadata: () => {}, - closeSegmentIndex: (stream, closeSegmentIndex) => {}, disableStream: (stream) => {}, addFont: (name, url) => {}, }; diff --git a/test/dash/dash_parser_segment_template_unit.js b/test/dash/dash_parser_segment_template_unit.js index f7d1c7023b..169de3246a 100644 --- a/test/dash/dash_parser_segment_template_unit.js +++ b/test/dash/dash_parser_segment_template_unit.js @@ -52,7 +52,6 @@ describe('DashParser SegmentTemplate', () => { onManifestUpdated: () => {}, getBandwidthEstimate: () => 1e6, onMetadata: () => {}, - closeSegmentIndex: (stream, closeSegmentIndex) => {}, disableStream: (stream) => {}, addFont: (name, url) => {}, }; diff --git a/test/hls/hls_live_unit.js b/test/hls/hls_live_unit.js index fab21bc732..5c117bfab3 100644 --- a/test/hls/hls_live_unit.js +++ b/test/hls/hls_live_unit.js @@ -82,7 +82,6 @@ describe('HlsParser live', () => { onManifestUpdated: () => {}, getBandwidthEstimate: () => 1e6, onMetadata: () => {}, - closeSegmentIndex: (stream, closeSegmentIndex) => {}, disableStream: (stream) => {}, addFont: (name, url) => {}, }; diff --git a/test/hls/hls_parser_unit.js b/test/hls/hls_parser_unit.js index bcd90296da..5f1f8fb62d 100644 --- a/test/hls/hls_parser_unit.js +++ b/test/hls/hls_parser_unit.js @@ -96,7 +96,6 @@ describe('HlsParser', () => { onManifestUpdated: () => {}, getBandwidthEstimate: () => 1e6, onMetadata: shaka.test.Util.spyFunc(onMetadataSpy), - closeSegmentIndex: (stream, closeSegmentIndex) => {}, disableStream: (stream) => {}, addFont: (name, url) => {}, }; diff --git a/test/media/streaming_engine_integration.js b/test/media/streaming_engine_integration.js index 5f76ce5178..9f47541b27 100644 --- a/test/media/streaming_engine_integration.js +++ b/test/media/streaming_engine_integration.js @@ -271,8 +271,6 @@ describe('StreamingEngine', () => { onSegmentAppended: () => playhead.notifyOfBufferingChange(), onInitSegmentAppended: () => {}, beforeAppendSegment: () => Promise.resolve(), - onMetadata: () => {}, - closeSegmentIndex: (stream, closeSegmentIndex) => {}, disableStream: (stream, time) => false, }; streamingEngine = new shaka.media.StreamingEngine( diff --git a/test/media/streaming_engine_unit.js b/test/media/streaming_engine_unit.js index f8b86a8f53..b129ad92e0 100644 --- a/test/media/streaming_engine_unit.js +++ b/test/media/streaming_engine_unit.js @@ -467,7 +467,6 @@ describe('StreamingEngine', () => { onSegmentAppended: Util.spyFunc(onSegmentAppended), onInitSegmentAppended: () => {}, beforeAppendSegment: Util.spyFunc(beforeAppendSegment), - closeSegmentIndex: (stream, closeSegmentIndex) => {}, disableStream: Util.spyFunc(disableStream), }; streamingEngine = new shaka.media.StreamingEngine( diff --git a/test/mss/mss_parser_unit.js b/test/mss/mss_parser_unit.js index b6c19573ec..071bba0d1a 100644 --- a/test/mss/mss_parser_unit.js +++ b/test/mss/mss_parser_unit.js @@ -82,7 +82,6 @@ describe('MssParser Manifest', () => { onManifestUpdated: () => {}, getBandwidthEstimate: () => 1e6, onMetadata: () => {}, - closeSegmentIndex: (stream, closeSegmentIndex) => {}, disableStream: (stream) => {}, addFont: (name, url) => {}, }; diff --git a/test/test/util/dash_parser_util.js b/test/test/util/dash_parser_util.js index 0202c2fb81..c047e92617 100644 --- a/test/test/util/dash_parser_util.js +++ b/test/test/util/dash_parser_util.js @@ -48,7 +48,6 @@ shaka.test.Dash = class { onManifestUpdated: () => {}, getBandwidthEstimate: () => 1e6, onMetadata: () => {}, - closeSegmentIndex: (stream, closeSegmentIndex) => {}, disableStream: (stream) => {}, addFont: (name, url) => {}, }; @@ -99,7 +98,6 @@ shaka.test.Dash = class { onManifestUpdated: () => {}, getBandwidthEstimate: () => 1e6, onMetadata: () => {}, - closeSegmentIndex: (stream, closeSegmentIndex) => {}, disableStream: (stream) => {}, addFont: (name, url) => {}, }; diff --git a/test/test/util/mss_parser_util.js b/test/test/util/mss_parser_util.js index bf228a53da..13e88b780f 100644 --- a/test/test/util/mss_parser_util.js +++ b/test/test/util/mss_parser_util.js @@ -48,7 +48,6 @@ shaka.test.Mss = class { onManifestUpdated: () => {}, getBandwidthEstimate: () => 1e6, onMetadata: () => {}, - closeSegmentIndex: (stream, closeSegmentIndex) => {}, disableStream: (stream) => {}, addFont: (name, url) => {}, }; @@ -90,7 +89,6 @@ shaka.test.Mss = class { onManifestUpdated: () => {}, getBandwidthEstimate: () => 1e6, onMetadata: () => {}, - closeSegmentIndex: (stream, closeSegmentIndex) => {}, disableStream: (stream) => {}, addFont: (name, url) => {}, }; diff --git a/test/util/content_steering_manager_unit.js b/test/util/content_steering_manager_unit.js index 6bf7e52498..a3a0760517 100644 --- a/test/util/content_steering_manager_unit.js +++ b/test/util/content_steering_manager_unit.js @@ -30,7 +30,6 @@ describe('ContentSteeringManager', () => { onManifestUpdated: fail, getBandwidthEstimate: () => 1e6, onMetadata: () => {}, - closeSegmentIndex: (stream, closeSegmentIndex) => {}, disableStream: (stream) => {}, addFont: (name, url) => {}, }; diff --git a/test/util/periods_unit.js b/test/util/periods_unit.js index 7e3c9d0193..12e814fb49 100644 --- a/test/util/periods_unit.js +++ b/test/util/periods_unit.js @@ -1259,125 +1259,6 @@ describe('PeriodCombiner', () => { expect(video2.originalId).toBe('3,4'); expect(video2.matchedStreams.length).toBe(2); - expect(stream1.closeSegmentIndex).not.toHaveBeenCalled(); - expect(stream2.closeSegmentIndex).not.toHaveBeenCalled(); - expect(stream3.closeSegmentIndex).not.toHaveBeenCalled(); - expect(stream4.closeSegmentIndex).not.toHaveBeenCalled(); - - combiner.deleteStream(stream1, '0'); - combiner.deleteStream(stream3, '1'); - - variants = combiner.getVariants(); - expect(variants.length).toBe(2); - - video1 = variants[0].video; - expect(video1.originalId).toBe('1,2'); - expect(video1.matchedStreams.length).toBe(1); - - video2 = variants[1].video; - expect(video2.originalId).toBe('3,4'); - expect(video2.matchedStreams.length).toBe(1); - - expect(stream1.closeSegmentIndex).toHaveBeenCalledTimes(1); - expect(stream2.closeSegmentIndex).not.toHaveBeenCalled(); - expect(stream3.closeSegmentIndex).toHaveBeenCalledTimes(1); - expect(stream4.closeSegmentIndex).not.toHaveBeenCalled(); - }); - - it('Delete streams with playerInterface', async () => { - const stream1 = makeVideoStream(1080); - stream1.originalId = '1'; - stream1.bandwidth = 120000; - stream1.codecs = 'hvc1.1.4.L126.B0'; - - const stream2 = makeVideoStream(1080); - stream2.originalId = '2'; - stream2.bandwidth = 120000; - stream2.codecs = 'hev1.2.4.L123.B0'; - - const stream3 = makeVideoStream(1080); - stream3.originalId = '3'; - stream3.bandwidth = 120000; - stream3.codecs = 'dvhe.05.01'; - - const stream4 = makeVideoStream(1080); - stream4.originalId = '4'; - stream4.bandwidth = 120000; - stream4.codecs = 'dvh1.05.01'; - - /** @type {!Array.} */ - const periods = [ - { - id: '0', - videoStreams: [ - stream1, stream3, - ], - audioStreams: [], - textStreams: [], - imageStreams: [], - }, - { - id: '1', - videoStreams: [ - stream2, stream4, - ], - audioStreams: [], - textStreams: [], - imageStreams: [], - }, - ]; - - const fakeNetEngine = new shaka.test.FakeNetworkingEngine(); - const closeSegmentIndex = (stream, closeSegmentIndex) => { - closeSegmentIndex(); - }; - - const closeSegmentIndexSpy = jasmine.createSpy('closeSegmentIndex') - .and.callFake(closeSegmentIndex); - - /** @type {shaka.extern.ManifestParser.PlayerInterface} */ - const playerInterface = { - networkingEngine: fakeNetEngine, - modifyManifestRequest: (request, manifestInfo) => {}, - modifySegmentRequest: (request, segmentInfo) => {}, - filter: (manifest) => Promise.resolve(), - makeTextStreamsForClosedCaptions: (manifest) => {}, - onTimelineRegionAdded: (region) => {}, - onEvent: () => {}, - onError: (e) => {}, - isLowLatencyMode: () => false, - isAutoLowLatencyMode: () => false, - enableLowLatencyMode: () => {}, - updateDuration: () => {}, - newDrmInfo: (stream) => {}, - onManifestUpdated: () => {}, - getBandwidthEstimate: () => 1e6, - onMetadata: () => {}, - closeSegmentIndex: shaka.test.Util.spyFunc(closeSegmentIndexSpy), - disableStream: (stream) => {}, - addFont: (name, url) => {}, - }; - - combiner.setPlayerInterface(playerInterface); - - await combiner.combinePeriods(periods, /* isDynamic= */ true); - let variants = combiner.getVariants(); - expect(variants.length).toBe(2); - - let video1 = variants[0].video; - expect(video1.originalId).toBe('1,2'); - expect(video1.matchedStreams.length).toBe(2); - - let video2 = variants[1].video; - expect(video2.originalId).toBe('3,4'); - expect(video2.matchedStreams.length).toBe(2); - - expect(stream1.closeSegmentIndex).not.toHaveBeenCalled(); - expect(stream2.closeSegmentIndex).not.toHaveBeenCalled(); - expect(stream3.closeSegmentIndex).not.toHaveBeenCalled(); - expect(stream4.closeSegmentIndex).not.toHaveBeenCalled(); - expect(playerInterface.closeSegmentIndex).not.toHaveBeenCalled(); - combiner.deleteStream(stream1, '0'); combiner.deleteStream(stream3, '1'); @@ -1391,12 +1272,6 @@ describe('PeriodCombiner', () => { video2 = variants[1].video; expect(video2.originalId).toBe('3,4'); expect(video2.matchedStreams.length).toBe(1); - - expect(stream1.closeSegmentIndex).toHaveBeenCalledTimes(1); - expect(stream2.closeSegmentIndex).not.toHaveBeenCalled(); - expect(stream3.closeSegmentIndex).toHaveBeenCalledTimes(1); - expect(stream4.closeSegmentIndex).not.toHaveBeenCalled(); - expect(playerInterface.closeSegmentIndex).toHaveBeenCalledTimes(2); }); it('Variant has highest bandwidth from matched streams', async () => {