Skip to content

Commit

Permalink
fix: Fix errors with TS segments on Chromecast (#4543)
Browse files Browse the repository at this point in the history
On Chromecast devices, TS-based streams tend to be more susceptible to
failures related to the Cast platform's MSE implementation.

One of these failures was observed when Shaka Player updates
`SourceBuffer#timestampOffset` within
`MediaSourceEngine#appendBuffer()`:
1. To extract a timestamp from `MediaSource` to align text segments:
https://github.com/shaka-project/shaka-player/blob/3d0f752c7d0677f750dbbd9bcd2895358358628f/lib/media/media_source_engine.js#L657
2. To re-align segments as a result of an unbuffered seek or automatic
adaptation:
https://github.com/shaka-project/shaka-player/blob/3d0f752c7d0677f750dbbd9bcd2895358358628f/lib/media/media_source_engine.js#L684-L686

The Cast platform throws the following error:
```
...
"stack":"Error: Failed to set the 'timestampOffset' property on 'SourceBuffer': The timestamp offset
may not be set while the SourceBuffer's append state is 'PARSING_MEDIA_SEGMENT'.
...
```

One way of resolving this is to reset the SourceBuffer's parser state,
which in turn resets the append state from `PARSING_MEDIA_SEGMENT` =>
`WAITING_FOR_SEGMENT`:
https://www.w3.org/TR/media-source-2/#sourcebuffer-reset-parser-state.

On Cast devices, the issue is reproduced consistently when Shaka
switches to a new variant. There are no internally queued MSE operations
in `MediaSourceEngine` when the error is thrown. Based on these
observations, the input buffer passed to `MediaSourceEngine` seems to be
an "incomplete" media segment, meaning the `SourceBuffer#appendBuffer()`
call exits when the append state is still `PARSING_MEDIA_SEGMENT` ([Step
`6.4` in W3C
specs](https://www.w3.org/TR/media-source-2/#sourcebuffer-segment-parser-loop))
hence the error.

This is also suggested as a root cause from a Chromium engineer, with
one suggested solution to call abort() beforehand
([source](https://bugs.chromium.org/p/chromium/issues/detail?id=766002#c3)).
Since Shaka Player already does preemptive abort() calls when [setting
stream
properties](https://github.com/shaka-project/shaka-player/blob/3d0f752c7d0677f750dbbd9bcd2895358358628f/lib/media/media_source_engine.js#L836-L842),
this will do the same thing.

Co-authored-by: Joey Parrish <joeyparrish@users.noreply.github.com>
  • Loading branch information
JulianDomingo and joeyparrish committed Oct 6, 2022
1 parent eb8401f commit 15a1c60
Show file tree
Hide file tree
Showing 3 changed files with 105 additions and 1 deletion.
16 changes: 15 additions & 1 deletion lib/media/media_source_engine.js
Original file line number Diff line number Diff line change
Expand Up @@ -527,7 +527,8 @@ shaka.media.MediaSourceEngine = class {
* @return {!Promise}
*/
async appendBuffer(
contentType, data, reference, hasClosedCaptions, seeked, adaptation) {
contentType, data, reference, hasClosedCaptions, seeked = false,
adaptation = false) {
const ContentType = shaka.util.ManifestParserUtils.ContentType;

if (contentType == ContentType.TEXT) {
Expand Down Expand Up @@ -628,6 +629,14 @@ shaka.media.MediaSourceEngine = class {

await this.enqueueOperation_(
contentType, () => this.append_(contentType, data));
// If the input buffer passed to SourceBuffer#appendBuffer() does not
// contain a complete media segment, the call will exit while the
// SourceBuffer's append state is
// still PARSING_MEDIA_SEGMENT. Reset the parser state by calling
// abort() to safely reset timestampOffset to 'originalOffset'.
// https://www.w3.org/TR/media-source-2/#sourcebuffer-segment-parser-loop
await this.enqueueOperation_(
contentType, () => this.abort_(contentType));

// Reset the offset and append window.
sourceBuffer.timestampOffset = originalOffset;
Expand Down Expand Up @@ -657,6 +666,11 @@ shaka.media.MediaSourceEngine = class {
// adaptation, we need to set a new timestampOffset on the sourceBuffer.
if (seeked || adaptation) {
const timestampOffset = reference.startTime;
// The logic to call abort() before setting the timestampOffset is
// extended during unbuffered seeks or automatic adaptations; it is
// possible for the append state to be PARSING_MEDIA_SEGMENT from the
// previous SourceBuffer#appendBuffer() call.
this.enqueueOperation_(contentType, () => this.abort_(contentType));
this.enqueueOperation_(
contentType,
() => this.setTimestampOffset_(contentType, timestampOffset));
Expand Down
36 changes: 36 additions & 0 deletions test/media/media_source_engine_integration.js
Original file line number Diff line number Diff line change
Expand Up @@ -380,6 +380,42 @@ describe('MediaSourceEngine', () => {
expect(textDisplayer.appendSpy).toHaveBeenCalledTimes(3);
});

it('buffers partial TS video segments in sequence mode', async () => {
metadata = shaka.test.TestScheme.DATA['cea-708_ts'];
generators = shaka.test.TestScheme.GENERATORS['cea-708_ts'];

const videoType = ContentType.VIDEO;
const initObject = new Map();
initObject.set(videoType, getFakeStream(metadata.video));

await mediaSourceEngine.init(
initObject, /* forceTransmuxTS= */ false, /* sequenceMode= */ true);
await mediaSourceEngine.setDuration(presentationDuration);
await mediaSourceEngine.setStreamProperties(
videoType,
/* timestampOffset= */ 0,
/* appendWindowStart= */ 0,
/* appendWindowEnd= */ Infinity,
/* sequenceMode= */ true);

const segment = generators[videoType].getSegment(0, Date.now() / 1000);
const partialSegmentLength = Math.floor(segment.byteLength / 3);

let partialSegment = shaka.util.BufferUtils.toUint8(
segment, /* offset= */ 0, /* length= */ partialSegmentLength);
let reference = dummyReference(videoType, 0);
await mediaSourceEngine.appendBuffer(
videoType, partialSegment, reference, /* hasClosedCaptions= */ false);

partialSegment = shaka.util.BufferUtils.toUint8(
segment,
/* offset= */ partialSegmentLength);
reference = dummyReference(videoType, 1);
await mediaSourceEngine.appendBuffer(
videoType, partialSegment, reference, /* hasClosedCaptions= */ false,
/* seeked= */ true);
});

it('extracts CEA-708 captions from dash', async () => {
// Load MP4 file with CEA-708 closed captions.
metadata = shaka.test.TestScheme.DATA['cea-708_mp4'];
Expand Down
54 changes: 54 additions & 0 deletions test/media/media_source_engine_unit.js
Original file line number Diff line number Diff line change
Expand Up @@ -647,6 +647,60 @@ describe('MediaSourceEngine', () => {

expect(videoSourceBuffer.timestampOffset).toBe(0.50);
});

it('calls abort before setting timestampOffset', async () => {
const simulateUpdate = async () => {
await Util.shortDelay();
videoSourceBuffer.updateend();
};
const initObject = new Map();
initObject.set(ContentType.VIDEO, fakeVideoStream);

await mediaSourceEngine.init(
initObject, /* forceTransmuxTS= */ false, /* sequenceMode= */ true);

// First, mock the scenario where timestampOffset is set to help align
// text segments. In this case, SourceBuffer mode is still 'segments'.
let reference = dummyReference(0, 1000);
let appendVideo = mediaSourceEngine.appendBuffer(
ContentType.VIDEO, buffer, reference, /* hasClosedCaptions= */ false);
// Wait for the first appendBuffer(), in segments mode.
await simulateUpdate();
// Next, wait for abort(), used to reset the parser state for a safe
// setting of timestampOffset. Shaka fakes an updateend event on abort(),
// so simulateUpdate() isn't needed.
await Util.shortDelay();
// Next, wait for remove(), used to clear the SourceBuffer from the
// initial append.
await simulateUpdate();
// Next, wait for the second appendBuffer(), falling through to normal
// operations.
await simulateUpdate();
// Lastly, wait for the function-scoped MediaSourceEngine#appendBuffer()
// promise to resolve.
await appendVideo;
expect(videoSourceBuffer.abort).toHaveBeenCalledTimes(1);

// Second, mock the scenario where timestampOffset is set during an
// unbuffered seek or adaptation. SourceBuffer mode is 'sequence' now.
reference = dummyReference(0, 1000);
appendVideo = mediaSourceEngine.appendBuffer(
ContentType.VIDEO, buffer, reference, /* hasClosedCaptions= */ false,
/* seeked= */ true);
// First, wait for abort(), used to reset the parser state for a safe
// setting of timestampOffset.
await Util.shortDelay();
// The subsequent setTimestampOffset() fakes an updateend event for us, so
// simulateUpdate() isn't needed.
await Util.shortDelay();
// Next, wait for the second appendBuffer(), falling through to normal
// operations.
await simulateUpdate();
// Lastly, wait for the function-scoped MediaSourceEngine#appendBuffer()
// promise to resolve.
await appendVideo;
expect(videoSourceBuffer.abort).toHaveBeenCalledTimes(2);
});
});

describe('remove', () => {
Expand Down

0 comments on commit 15a1c60

Please sign in to comment.