Skip to content

Commit

Permalink
fix(hls): Fixed buffering issue with live HLS
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
theodab committed Feb 24, 2022
1 parent 5894b8c commit f04d314
Show file tree
Hide file tree
Showing 6 changed files with 47 additions and 13 deletions.
14 changes: 11 additions & 3 deletions lib/media/media_source_engine.js
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
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
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
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
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
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 f04d314

Please sign in to comment.