From c438e857f2f122eb45899148e067d68ffec3477c Mon Sep 17 00:00:00 2001 From: theodab Date: Thu, 24 Feb 2022 11:43:30 -0800 Subject: [PATCH] fix(hls): Fixed buffering issue with live HLS (#4002) HLS now appends segments in sequence mode. In order to handle seeks, we set the timestampOffset property on SourceBuffer to the startTime of the segment. This is done after every seek, or on startup. For non-sequence-mode content (DASH), we normally set timestampOffset based on the Period structure. This should be suppressed in sequence mode, though, where only the reference startTime matters. The buffering issue was caused by two things in combination: 1. The HLS parser set meaningless timestampOffset values that would change when a playlist was updated 2. We would use those timestampOffset values in setStreamProperties, even though this should be skipped in sequence mode These two things in combination would lead MediaSourceEngine to start inserting segments near the start of the presentation, rather than at the live edge. This changes MediaSourceEngine so that in sequence mode, timestampOffset is ignored in setStreamProperties. This also cleans up the HLS parser to set each reference's timestampOffset to 0, since they should be ignored anyway. --- lib/hls/hls_parser.js | 8 ++---- 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 +++++--- 7 files changed, 50 insertions(+), 18 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); 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(); }