Skip to content

Commit

Permalink
fix: Clear buffer on seek if mediaState is updating (#3795)
Browse files Browse the repository at this point in the history
Previously, we only cleared the buffer if the media state had
something buffered. This ensured that we did not pointlessly clear the
buffer when nothing was there.
However, this lead to problems if a seek was performed early during the
loading process, before the source buffer is initialized. During that
time, nothing is buffered, so we would not clear the buffer on a seek.
This lead to us accidentally fetching content from the beginning of the
presentation, rather than starting from the new clock time.
This changes the streaming engine to also clear the buffer if
mediaState.performingUpdate is true, to avoid that situation.

Closes #3299
  • Loading branch information
pcruiksh authored Jan 14, 2022
1 parent 087a9b4 commit 9705639
Show file tree
Hide file tree
Showing 2 changed files with 65 additions and 1 deletion.
2 changes: 1 addition & 1 deletion lib/media/streaming_engine.js
Original file line number Diff line number Diff line change
Expand Up @@ -628,7 +628,7 @@ shaka.media.StreamingEngine = class {

// Don't clear the buffer unless something is buffered. This extra
// check prevents extra, useless calls to clear the buffer.
if (somethingBuffered) {
if (somethingBuffered || mediaState.performingUpdate) {
this.forceClearBuffer_(mediaState);
streamCleared = true;
}
Expand Down
64 changes: 64 additions & 0 deletions test/media/streaming_engine_unit.js
Original file line number Diff line number Diff line change
Expand Up @@ -97,6 +97,9 @@ describe('StreamingEngine', () => {
/** @type {!shaka.media.StreamingEngine} */
let streamingEngine;

/** @type {function(function(), number)} */
let realSetTimeout;

/**
* Runs the fake event loop.
* @param {function()=} callback An optional callback that is executed
Expand All @@ -116,6 +119,7 @@ describe('StreamingEngine', () => {
}

beforeAll(() => {
realSetTimeout = window.setTimeout;
jasmine.clock().install();
jasmine.clock().mockDate();
});
Expand Down Expand Up @@ -1438,6 +1442,66 @@ describe('StreamingEngine', () => {
});
});

it('into unbuffered regions when nothing is buffered ' +
'and mediaState is performing an update', async () => {
// Here we go!
streamingEngine.switchVariant(variant);
streamingEngine.switchTextStream(textStream);
// ensure init source buffer promise does not resolve before seeked()
// so mediaState remains in "performingUpdate" state
const initSourceBufferPromise = new shaka.util.PublicPromise();
mediaSourceEngine.setStreamProperties.and
.returnValue(initSourceBufferPromise);
await streamingEngine.start();
playing = true;

// tick to trigger mediaState updates
jasmine.clock().tick(1);
// give a chance for fetchAndAppend_ to be invoked
// by async onUpdate_ callback
await Util.shortDelay(realSetTimeout);

// Nothing is buffered yet.
expect(mediaSourceEngine.segments).toEqual({
audio: [false, false, false, false],
video: [false, false, false, false],
text: [false, false, false, false],
});

// Seek forward to an unbuffered region in the first Period.
presentationTimeInSeconds = 15;
streamingEngine.seeked();

// resolve initSourceBufferPromise after seeked(), waitingToClearBuffer
// should have been set, so this will now trigger the actual flush of
// the buffer
initSourceBufferPromise.resolve();
mediaSourceEngine.setStreamProperties.and
.returnValue(Promise.resolve());

// allow mediaState update to resolve
await Util.shortDelay(realSetTimeout);

onTick.and.callFake(() => {
expect(mediaSourceEngine.clear).toHaveBeenCalled();
onTick.and.stub();
});

await runTest(Util.spyFunc(onTick));

// Verify buffers.
expect(mediaSourceEngine.initSegments).toEqual({
audio: [false, true],
video: [false, true],
text: [],
});
expect(mediaSourceEngine.segments).toEqual({
audio: [false, true, true, true],
video: [false, true, true, true],
text: [false, true, true, true],
});
});

it('into unbuffered regions near segment start', async () => {
// Here we go!
streamingEngine.switchVariant(variant);
Expand Down

0 comments on commit 9705639

Please sign in to comment.