Skip to content

Commit

Permalink
fix(hls): Fixed buffering issue with live HLS (shaka-project#4002)
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
theodab authored Feb 24, 2022
1 parent 5894b8c commit c438e85
Show file tree
Hide file tree
Showing 7 changed files with 50 additions and 18 deletions.
8 changes: 3 additions & 5 deletions lib/hls/hls_parser.js
Original file line number Diff line number Diff line change
Expand Up @@ -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.<string, string>} variables
* @param {string} absoluteMediaPlaylistUri
* @return {!shaka.media.SegmentReference}
Expand All @@ -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);
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -1960,7 +1959,6 @@ shaka.hls.HlsParser = class {
previousReference,
item,
startTime,
firstStartTime,
variables,
playlist.absoluteUri,
type);
Expand Down
14 changes: 11 additions & 3 deletions lib/media/media_source_engine.js
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
Expand All @@ -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_(
Expand Down
2 changes: 1 addition & 1 deletion lib/media/streaming_engine.js
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
28 changes: 25 additions & 3 deletions test/media/media_source_engine_integration.js
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand All @@ -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);
Expand All @@ -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);
Expand Down
3 changes: 2 additions & 1 deletion test/media/media_source_engine_unit.js
Original file line number Diff line number Diff line change
Expand Up @@ -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);
});
Expand Down
2 changes: 1 addition & 1 deletion test/media/streaming_engine_unit.js
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
11 changes: 7 additions & 4 deletions test/test/util/fake_media_source_engine.js
Original file line number Diff line number Diff line change
Expand Up @@ -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')
Expand Down Expand Up @@ -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();
}
Expand Down

0 comments on commit c438e85

Please sign in to comment.