From f04d3141372f07cffa2d270ad749b3012cc9f68f Mon Sep 17 00:00:00 2001 From: Theodore Abshire Date: Thu, 24 Feb 2022 08:38:08 -0800 Subject: [PATCH 1/2] fix(hls): Fixed buffering issue with live HLS HLS now appends segments in sequence mode. In order to handle seeks, we use the timestampOffset property to inform MediaSource where the new segment should go. However, we were still setting timestampOffset when the manifest was updated, which caused issues in live HLS; suddenly, MediaSource would start inserting segments near the start of the presentation, rather than at the live edge. This changes MediaSourceEngine to not apply timestampOffset during initialization when in sequence mode, to fix that problem. --- lib/media/media_source_engine.js | 14 ++++++++-- lib/media/streaming_engine.js | 2 +- test/media/media_source_engine_integration.js | 28 +++++++++++++++++-- test/media/media_source_engine_unit.js | 3 +- test/media/streaming_engine_unit.js | 2 +- test/test/util/fake_media_source_engine.js | 11 +++++--- 6 files changed, 47 insertions(+), 13 deletions(-) diff --git a/lib/media/media_source_engine.js b/lib/media/media_source_engine.js index 03a1600e8c..a1588f657b 100644 --- a/lib/media/media_source_engine.js +++ b/lib/media/media_source_engine.js @@ -691,13 +691,18 @@ shaka.media.MediaSourceEngine = class { * @param {number} appendWindowEnd The timestamp to set the append window end * to. For future appends, frames/samples with timestamps greater than this * value will be dropped. + * @param {boolean} sequenceMode If true, the timestampOffset will not be + * applied in this step. * @return {!Promise} */ async setStreamProperties( - contentType, timestampOffset, appendWindowStart, appendWindowEnd) { + contentType, timestampOffset, appendWindowStart, appendWindowEnd, + sequenceMode) { const ContentType = shaka.util.ManifestParserUtils.ContentType; if (contentType == ContentType.TEXT) { - this.textEngine_.setTimestampOffset(timestampOffset); + if (!sequenceMode) { + this.textEngine_.setTimestampOffset(timestampOffset); + } this.textEngine_.setAppendWindow(appendWindowStart, appendWindowEnd); return; } @@ -714,7 +719,10 @@ shaka.media.MediaSourceEngine = class { this.enqueueOperation_( contentType, () => this.abort_(contentType)), - this.enqueueOperation_( + // Don't set the timestampOffset here when in sequenceMode, since we + // use timestampOffset for a different purpose in that mode (e.g. to + // indicate where the current segment is). + sequenceMode ? Promise.resolve() : this.enqueueOperation_( contentType, () => this.setTimestampOffset_(contentType, timestampOffset)), this.enqueueOperation_( diff --git a/lib/media/streaming_engine.js b/lib/media/streaming_engine.js index 272a2bf73c..a3fa69ce50 100644 --- a/lib/media/streaming_engine.js +++ b/lib/media/streaming_engine.js @@ -1520,7 +1520,7 @@ shaka.media.StreamingEngine = class { await this.playerInterface_.mediaSourceEngine.setStreamProperties( mediaState.type, timestampOffset, appendWindowStart, - appendWindowEnd); + appendWindowEnd, this.manifest_.sequenceMode); } catch (error) { mediaState.lastAppendWindowStart = null; mediaState.lastAppendWindowEnd = null; diff --git a/test/media/media_source_engine_integration.js b/test/media/media_source_engine_integration.js index 7debe00e6f..4d0727a1b5 100644 --- a/test/media/media_source_engine_integration.js +++ b/test/media/media_source_engine_integration.js @@ -295,7 +295,27 @@ describe('MediaSourceEngine', () => { await mediaSourceEngine.setStreamProperties(ContentType.VIDEO, /* timestampOffset= */ 0, /* appendWindowStart= */ 5, - /* appendWindowEnd= */ 18); + /* appendWindowEnd= */ 18, + /* sequenceMode= */ false); + expect(buffered(ContentType.VIDEO, 0)).toBe(0); + await append(ContentType.VIDEO, 0); + expect(bufferStart(ContentType.VIDEO)).toBeCloseTo(5, 1); + expect(buffered(ContentType.VIDEO, 5)).toBeCloseTo(5, 1); + await append(ContentType.VIDEO, 1); + expect(buffered(ContentType.VIDEO, 5)).toBeCloseTo(13, 1); + }); + + it('does not initialize timestamp offset in sequence mode', async () => { + const initObject = new Map(); + initObject.set(ContentType.VIDEO, getFakeStream(metadata.video)); + await mediaSourceEngine.init(initObject, false); + await mediaSourceEngine.setDuration(presentationDuration); + await appendInit(ContentType.VIDEO); + await mediaSourceEngine.setStreamProperties(ContentType.VIDEO, + /* timestampOffset= */ 100, + /* appendWindowStart= */ 5, + /* appendWindowEnd= */ 18, + /* sequenceMode= */ true); expect(buffered(ContentType.VIDEO, 0)).toBe(0); await append(ContentType.VIDEO, 0); expect(bufferStart(ContentType.VIDEO)).toBeCloseTo(5, 1); @@ -314,7 +334,8 @@ describe('MediaSourceEngine', () => { await mediaSourceEngine.setStreamProperties(ContentType.VIDEO, /* timestampOffset= */ 0, /* appendWindowStart= */ 0, - /* appendWindowEnd= */ 20); + /* appendWindowEnd= */ 20, + /* sequenceMode= */ false); await append(ContentType.VIDEO, 0); await append(ContentType.VIDEO, 1); expect(bufferStart(ContentType.VIDEO)).toBeCloseTo(0, 1); @@ -326,7 +347,8 @@ describe('MediaSourceEngine', () => { await mediaSourceEngine.setStreamProperties(ContentType.VIDEO, /* timestampOffset= */ 15, /* appendWindowStart= */ 20, - /* appendWindowEnd= */ 35); + /* appendWindowEnd= */ 35, + /* sequenceMode= */ false); await append(ContentType.VIDEO, 0); await append(ContentType.VIDEO, 1); expect(bufferStart(ContentType.VIDEO)).toBeCloseTo(0, 1); diff --git a/test/media/media_source_engine_unit.js b/test/media/media_source_engine_unit.js index 7b8e4ac522..2a286b8091 100644 --- a/test/media/media_source_engine_unit.js +++ b/test/media/media_source_engine_unit.js @@ -825,7 +825,8 @@ describe('MediaSourceEngine', () => { await mediaSourceEngine.setStreamProperties(ContentType.TEXT, /* timestampOffset= */ 10, /* appendWindowStart= */ 0, - /* appendWindowEnd= */ 20); + /* appendWindowEnd= */ 20, + /* sequenceMode= */ false); expect(mockTextEngine.setTimestampOffset).toHaveBeenCalledWith(10); expect(mockTextEngine.setAppendWindow).toHaveBeenCalledWith(0, 20); }); diff --git a/test/media/streaming_engine_unit.js b/test/media/streaming_engine_unit.js index 4523791fe8..d2357d6fb1 100644 --- a/test/media/streaming_engine_unit.js +++ b/test/media/streaming_engine_unit.js @@ -826,7 +826,7 @@ describe('StreamingEngine', () => { asymmetricMatch: (val) => val > 40 && val <= 40.1, }; expect(mediaSourceEngine.setStreamProperties) - .toHaveBeenCalledWith('video', 0, lt20, gt40); + .toHaveBeenCalledWith('video', 0, lt20, gt40, false); }); // Regression test for https://github.com/google/shaka-player/issues/3717 diff --git a/test/test/util/fake_media_source_engine.js b/test/test/util/fake_media_source_engine.js index 587e5e7fc7..5e142e214e 100644 --- a/test/test/util/fake_media_source_engine.js +++ b/test/test/util/fake_media_source_engine.js @@ -114,8 +114,8 @@ shaka.test.FakeMediaSourceEngine = class { /** @type {!jasmine.Spy} */ this.setStreamProperties = jasmine.createSpy('setStreamProperties') - .and.callFake((type, offset, end) => - this.setStreamPropertiesImpl_(type, offset, end)); + .and.callFake((type, offset, end, sequenceMode) => + this.setStreamPropertiesImpl_(type, offset, end, sequenceMode)); /** @type {!jasmine.Spy} */ this.remove = jasmine.createSpy('remove') @@ -362,14 +362,17 @@ shaka.test.FakeMediaSourceEngine = class { * @param {string} type * @param {number} offset * @param {number} appendWindowEnd + * @param {boolean} sequenceMode * @return {!Promise} * @private */ - setStreamPropertiesImpl_(type, offset, appendWindowEnd) { + setStreamPropertiesImpl_(type, offset, appendWindowEnd, sequenceMode) { if (!this.segments[type]) { throw new Error('unexpected type'); } - this.timestampOffsets_[type] = offset; + if (!sequenceMode) { + this.timestampOffsets_[type] = offset; + } // Don't use |appendWindowEnd|. return Promise.resolve(); } From a8e5f29f03cbafca0c5c4e9bfeaee0bcd5fa9c47 Mon Sep 17 00:00:00 2001 From: Theodore Abshire Date: Thu, 24 Feb 2022 10:34:11 -0800 Subject: [PATCH 2/2] refactor: Removes timestampOffset setting in HLS When in sequence mode, we no longer use the timestamp offset of segment references. So, in the HLS parser, there's no reason to set it anymore. --- lib/hls/hls_parser.js | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/lib/hls/hls_parser.js b/lib/hls/hls_parser.js index 3b78cebbe6..27c80ea308 100644 --- a/lib/hls/hls_parser.js +++ b/lib/hls/hls_parser.js @@ -1704,7 +1704,6 @@ shaka.hls.HlsParser = class { * @param {shaka.media.SegmentReference} previousReference * @param {!shaka.hls.Segment} hlsSegment * @param {number} startTime - * @param {number} timestampOffset * @param {!Map.} variables * @param {string} absoluteMediaPlaylistUri * @return {!shaka.media.SegmentReference} @@ -1713,7 +1712,7 @@ shaka.hls.HlsParser = class { */ createSegmentReference_( initSegmentReference, previousReference, hlsSegment, startTime, - timestampOffset, variables, absoluteMediaPlaylistUri, type) { + variables, absoluteMediaPlaylistUri, type) { const tags = hlsSegment.tags; const absoluteSegmentUri = this.variableSubstitution_( hlsSegment.absoluteUri, variables); @@ -1766,7 +1765,7 @@ shaka.hls.HlsParser = class { pStartByte, pEndByte, initSegmentReference, - timestampOffset, + /* timestampOffset= */ 0, // This value is ignored in sequence mode. /* appendWindowStart= */ 0, /* appendWindowEnd= */ Infinity); partialSegmentRefs.push(partial); @@ -1834,7 +1833,7 @@ shaka.hls.HlsParser = class { startByte, endByte, initSegmentReference, - timestampOffset, + /* timestampOffset= */ 0, // This value is ignored in sequence mode. /* appendWindowStart= */ 0, /* appendWindowEnd= */ Infinity, partialSegmentRefs, @@ -1960,7 +1959,6 @@ shaka.hls.HlsParser = class { previousReference, item, startTime, - firstStartTime, variables, playlist.absoluteUri, type);