Skip to content

Commit

Permalink
Fix exception on adaptation decisions
Browse files Browse the repository at this point in the history
Because aborting requires knowledge of the new stream, this process
must be done asynchronously.  This makes the abort logic async, and
checks carefully for any stream or operation changes during the
process.

Issue #1339 (flatten periods)
Issue #892 (refactor StreamingEngine)

Change-Id: Ic187676eeca907603efeb0ffa11855b9af2fc5ca
  • Loading branch information
joeyparrish committed Nov 7, 2019
1 parent a706768 commit c395a75
Show file tree
Hide file tree
Showing 2 changed files with 74 additions and 9 deletions.
57 changes: 48 additions & 9 deletions lib/media/streaming_engine.js
Original file line number Diff line number Diff line change
Expand Up @@ -568,11 +568,6 @@ shaka.media.StreamingEngine = class {
const streamTag = shaka.media.StreamingEngine.logPrefix_(mediaState);
shaka.log.debug('switch: switching to Stream ' + streamTag);

if (this.shouldAbortCurrentRequest_(mediaState, periodIndex)) {
shaka.log.info('Aborting current segment request to switch.');
mediaState.operation.abort();
}

if (clearBuffer) {
if (mediaState.clearingBuffer) {
// We are already going to clear the buffer, but make sure it is also
Expand All @@ -598,23 +593,67 @@ shaka.media.StreamingEngine = class {
});
}
}

this.makeAbortDecision_(mediaState, periodIndex).catch((error) => {
if (this.playerInterface_) {
this.playerInterface_.onError(error);
}
});
}


/**
* Returns whether we should abort the current request.
* Decide if it makes sense to abort the current operation, and abort it if
* so.
*
* @param {!shaka.media.StreamingEngine.MediaState_} mediaState
* @param {number} periodIndex
* @return {boolean}
* @private
*/
shouldAbortCurrentRequest_(mediaState, periodIndex) {
async makeAbortDecision_(mediaState, periodIndex) {
// If the operation is completed, it will be set to null, and there's no
// need to abort the request.
if (!mediaState.operation) {
return false;
return;
}

const originalStream = mediaState.stream;
const originalOperation = mediaState.operation;

if (!originalStream.segmentIndex) {
await originalStream.createSegmentIndex();
}

if (mediaState.operation != originalOperation) {
// The original operation completed while we were getting a segment index,
// so there's nothing to do now.
return;
}

if (mediaState.stream != originalStream) {
// The stream changed again while we were getting a segment index. We
// can't carry out this check, since another one might be in progress by
// now.
return;
}

if (this.shouldAbortCurrentRequest_(mediaState, periodIndex)) {
shaka.log.info('Aborting current segment request.');
mediaState.operation.abort();
}
}

/**
* Returns whether we should abort the current request.
*
* @param {!shaka.media.StreamingEngine.MediaState_} mediaState
* @param {number} periodIndex
* @return {boolean}
* @private
*/
shouldAbortCurrentRequest_(mediaState, periodIndex) {
goog.asserts.assert(mediaState.operation,
'Abort logic requires an ongoing operation!');

const presentationTime = this.playerInterface_.getPresentationTime();
const bufferEnd =
Expand Down
26 changes: 26 additions & 0 deletions test/media/streaming_engine_unit.js
Original file line number Diff line number Diff line change
Expand Up @@ -3027,6 +3027,32 @@ describe('StreamingEngine', () => {
await bufferAndCheck(/* didAbort= */ true, /* hasInit= */ true);
});

it('aborts pending requests after fetching new segment index', async () => {
await prepareForAbort();

/** @type {shaka.net.NetworkingEngine.PendingRequest} */
const secondRequest = lastPendingRequest;

// Simulate a segment index which is not yet available. This is
// necessary because the fake content used in this test is in the style
// of a segment template, where the index is already known.
const segmentIndex = newVariant.video.segmentIndex;
expect(segmentIndex).not.toBe(null);
newVariant.video.segmentIndex = null;
newVariant.video.createSegmentIndex = () => {
newVariant.video.segmentIndex = segmentIndex;
return Promise.resolve();
};

// This should abort the pending request for the second segment.
streamingEngine.switchVariant(
newVariant, /* clear_buffer= */ false, /* safe_margin= */ 0);

await bufferAndCheck(/* didAbort= */ true);

expect(secondRequest.abort).toHaveBeenCalled();
});

function flushDelayedRequests() {
for (const delay of delayedRequests) {
delay.resolve();
Expand Down

0 comments on commit c395a75

Please sign in to comment.