Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: Fix inefficient buffering behavior with negative trick play rate #6489

Merged
merged 1 commit into from Apr 29, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
72 changes: 54 additions & 18 deletions lib/media/segment_index.js
Expand Up @@ -436,10 +436,11 @@ shaka.media.SegmentIndex = class {
*
* @param {number} time
* @param {boolean=} allowNonIndepedent
* @param {boolean=} reverse
* @return {?shaka.media.SegmentIterator}
* @export
*/
getIteratorForTime(time, allowNonIndepedent = false) {
getIteratorForTime(time, allowNonIndepedent = false, reverse = false) {
let index = this.find(time);
if (index == null) {
return null;
Expand Down Expand Up @@ -476,7 +477,8 @@ shaka.media.SegmentIndex = class {
}
}
}
return new shaka.media.SegmentIterator(this, index, partialSegmentIndex);
return new shaka.media.SegmentIterator(
this, index, partialSegmentIndex, reverse);
}

/**
Expand Down Expand Up @@ -552,8 +554,9 @@ shaka.media.SegmentIterator = class {
* @param {shaka.media.SegmentIndex} segmentIndex
* @param {number} index
* @param {number} partialSegmentIndex
* @param {boolean} reverse
*/
constructor(segmentIndex, index, partialSegmentIndex) {
constructor(segmentIndex, index, partialSegmentIndex, reverse) {
/** @private {shaka.media.SegmentIndex} */
this.segmentIndex_ = segmentIndex;

Expand All @@ -562,6 +565,17 @@ shaka.media.SegmentIterator = class {

/** @private {number} */
this.currentPartialPosition_ = partialSegmentIndex;

/** @private {boolean} */
this.reverse = reverse;
}

/**
* @param {boolean} reverse
* @export
*/
setReverse(reverse) {
this.reverse = reverse;
}

/**
Expand Down Expand Up @@ -606,25 +620,47 @@ shaka.media.SegmentIterator = class {
next() {
const ref = this.segmentIndex_.get(this.currentPosition_);

if (ref && ref.hasPartialSegments()) {
// If the regular segment contains partial segments, move to the next
// partial SegmentReference.
this.currentPartialPosition_++;
// If the current regular segment has been published completely, and
// we've reached the end of its partial segments list, move to the next
// regular segment.
// If the Partial Segments list is still on the fly, do not move to
// the next regular segment.
if (ref.hasAllPartialSegments() &&
this.currentPartialPosition_ == ref.partialReferences.length) {
if (!this.reverse) {
if (ref && ref.hasPartialSegments()) {
// If the regular segment contains partial segments, move to the next
// partial SegmentReference.
this.currentPartialPosition_++;
// If the current regular segment has been published completely, and
// we've reached the end of its partial segments list, move to the next
// regular segment.
// If the Partial Segments list is still on the fly, do not move to
// the next regular segment.
if (ref.hasAllPartialSegments() &&
this.currentPartialPosition_ == ref.partialReferences.length) {
this.currentPosition_++;
this.currentPartialPosition_ = 0;
}
} else {
// If the regular segment doesn't contain partial segments, move to the
// next regular segment.
this.currentPosition_++;
this.currentPartialPosition_ = 0;
}
} else {
// If the regular segment doesn't contain partial segments, move to the
// next regular segment.
this.currentPosition_++;
this.currentPartialPosition_ = 0;
if (ref && ref.hasPartialSegments()) {
// If the regular segment contains partial segments, move to the
// previous partial SegmentReference.
this.currentPartialPosition_--;
if (this.currentPartialPosition_ < 0) {
this.currentPosition_--;
const prevRef = this.segmentIndex_.get(this.currentPosition_);
if (prevRef && prevRef.hasPartialSegments()) {
this.currentPartialPosition_ = prevRef.partialReferences.length - 1;
} else {
this.currentPartialPosition_ = 0;
}
}
} else {
// If the regular segment doesn't contain partial segments, move to the
// previous regular segment.
this.currentPosition_--;
this.currentPartialPosition_ = 0;
}
}

const res = this.current();
Expand Down
39 changes: 36 additions & 3 deletions lib/media/streaming_engine.js
Expand Up @@ -360,6 +360,8 @@ shaka.media.StreamingEngine = class {
setTrickPlay(on) {
const ContentType = shaka.util.ManifestParserUtils.ContentType;

this.updateSegmentIteratorReverse_();

const mediaState = this.mediaStates_.get(ContentType.VIDEO);
if (!mediaState) {
return;
Expand Down Expand Up @@ -1422,8 +1424,10 @@ shaka.media.StreamingEngine = class {
shaka.log.v1(
logPrefix, 'looking up segment from new stream endTime:', time);

const reverse = this.playerInterface_.getPlaybackRate() < 0;
mediaState.segmentIterator =
mediaState.stream.segmentIndex.getIteratorForTime(time);
mediaState.stream.segmentIndex.getIteratorForTime(
time, /* allowNonIndepedent= */ false, reverse);
const ref = mediaState.segmentIterator &&
mediaState.segmentIterator.next().value;
if (ref == null) {
Expand All @@ -1443,18 +1447,21 @@ shaka.media.StreamingEngine = class {
'lookupTime:', lookupTime,
'presentationTime:', presentationTime);

const reverse = this.playerInterface_.getPlaybackRate() < 0;
let ref = null;
if (inaccurateTolerance) {
mediaState.segmentIterator =
mediaState.stream.segmentIndex.getIteratorForTime(lookupTime);
mediaState.stream.segmentIndex.getIteratorForTime(
lookupTime, /* allowNonIndepedent= */ false, reverse);
ref = mediaState.segmentIterator &&
mediaState.segmentIterator.next().value;
}
if (!ref) {
// If we can't find a valid segment with the drifted time, look for a
// segment with the presentation time.
mediaState.segmentIterator =
mediaState.stream.segmentIndex.getIteratorForTime(presentationTime);
mediaState.stream.segmentIndex.getIteratorForTime(
presentationTime, /* allowNonIndepedent= */ false, reverse);
ref = mediaState.segmentIterator &&
mediaState.segmentIterator.next().value;
}
Expand Down Expand Up @@ -2752,6 +2759,29 @@ shaka.media.StreamingEngine = class {
}
}

/**
* Update the segment iterator direction.
*
* @private
*/
updateSegmentIteratorReverse_() {
const ContentType = shaka.util.ManifestParserUtils.ContentType;

const reverse = this.playerInterface_.getPlaybackRate() < 0;
const videoState = this.mediaStates_.get(ContentType.VIDEO);
if (videoState && videoState.segmentIterator) {
videoState.segmentIterator.setReverse(reverse);
}
const audioState = this.mediaStates_.get(ContentType.AUDIO);
if (audioState && audioState.segmentIterator) {
audioState.segmentIterator.setReverse(reverse);
}
const textState = this.mediaStates_.get(ContentType.TEXT);
if (textState && textState.segmentIterator) {
textState.segmentIterator.setReverse(reverse);
}
}

/**
* @param {shaka.media.StreamingEngine.MediaState_} mediaState
* @return {string} A log prefix of the form ($CONTENT_TYPE:$STREAM_ID), e.g.,
Expand All @@ -2768,6 +2798,7 @@ shaka.media.StreamingEngine = class {
* @typedef {{
* getPresentationTime: function():number,
* getBandwidthEstimate: function():number,
* getPlaybackRate: function():number,
* mediaSourceEngine: !shaka.media.MediaSourceEngine,
* netEngine: shaka.net.NetworkingEngine,
* onError: function(!shaka.util.Error),
Expand All @@ -2787,6 +2818,8 @@ shaka.media.StreamingEngine = class {
* viewer is seeing on screen right now.
* @property {function():number} getBandwidthEstimate
* Get the estimated bandwidth in bits per second.
* @property {function():number} getPlaybackRate
* Get the playback rate
* @property {!shaka.media.MediaSourceEngine} mediaSourceEngine
* The MediaSourceEngine. The caller retains ownership.
* @property {shaka.net.NetworkingEngine} netEngine
Expand Down
1 change: 1 addition & 0 deletions lib/player.js
Expand Up @@ -3246,6 +3246,7 @@ shaka.Player = class extends shaka.util.FakeEventTarget {
const playerInterface = {
getPresentationTime: () => this.playhead_ ? this.playhead_.getTime() : 0,
getBandwidthEstimate: () => this.abrManager_.getBandwidthEstimate(),
getPlaybackRate: () => this.getPlaybackRate(),
mediaSourceEngine: this.mediaSourceEngine_,
netEngine: this.networkingEngine_,
onError: (error) => this.onError_(error),
Expand Down
5 changes: 3 additions & 2 deletions lib/util/cmcd_manager.js
Expand Up @@ -187,12 +187,12 @@ shaka.util.CmcdManager = class {

const stream = context.stream;
if (stream) {
const playbackRate = this.playerInterface_.getPlaybackRate();
if (isMedia) {
data.bl = this.getBufferLength_(stream.type);
if (data.ot !== ObjectType.TIMED_TEXT) {
const remainingBufferLength =
this.getRemainingBufferLength_(stream.type);
const playbackRate = this.playerInterface_.getPlaybackRate();
if (playbackRate) {
data.dl = remainingBufferLength / Math.abs(playbackRate);
} else {
Expand All @@ -206,8 +206,9 @@ shaka.util.CmcdManager = class {
}

if (stream.segmentIndex && segment) {
const reverse = playbackRate < 0;
const iterator = stream.segmentIndex.getIteratorForTime(
segment.endTime, /* allowNonIndepedent= */ true);
segment.endTime, /* allowNonIndepedent= */ true, reverse);
if (iterator) {
const nextSegment = iterator.next().value;
if (nextSegment && nextSegment != segment) {
Expand Down
34 changes: 34 additions & 0 deletions test/media/segment_index_unit.js
Expand Up @@ -742,6 +742,40 @@ describe('SegmentIndex', /** @suppress {accessControls} */ () => {
expect(iterator.current()).toBe(null);
});

it('reverse iteration', () => {
const refs = inputRefs.slice();
const index = new shaka.media.SegmentIndex(refs);

// This simulates the pattern of calls in StreamingEngine when we buffer
// to the edge of a live stream.
const iterator = index[Symbol.iterator]();
iterator.next();
expect(iterator.current()).toBe(inputRefs[0]);

iterator.next();
expect(iterator.current()).toBe(inputRefs[1]);

iterator.next();
expect(iterator.current()).toBe(inputRefs[2]);

iterator.next();
expect(iterator.current()).toBe(null);

iterator.setReverse(true);

iterator.next();
expect(iterator.current()).toBe(inputRefs[2]);

iterator.next();
expect(iterator.current()).toBe(inputRefs[1]);

iterator.next();
expect(iterator.current()).toBe(inputRefs[0]);

iterator.next();
expect(iterator.current()).toBe(null);
});

describe('getIteratorForTime', () => {
it('begins with an independent partial segment', () => {
// This test contains its own segment refs, which are manipulated to
Expand Down
1 change: 1 addition & 0 deletions test/media/streaming_engine_integration.js
Expand Up @@ -257,6 +257,7 @@ describe('StreamingEngine', () => {
const playerInterface = {
getPresentationTime: () => playhead.getTime(),
getBandwidthEstimate: () => 1e6,
getPlaybackRate: () => video.playbackRate,
mediaSourceEngine: mediaSourceEngine,
netEngine: /** @type {!shaka.net.NetworkingEngine} */(netEngine),
onError: Util.spyFunc(onError),
Expand Down
5 changes: 5 additions & 0 deletions test/media/streaming_engine_unit.js
Expand Up @@ -70,6 +70,8 @@ describe('StreamingEngine', () => {
let onSegmentAppended;
/** @type {!jasmine.Spy} */
let getBandwidthEstimate;
/** @type {!jasmine.Spy} */
let getPlaybackRate;
/** @type {!shaka.media.StreamingEngine} */
let streamingEngine;
/** @type {!jasmine.Spy} */
Expand Down Expand Up @@ -441,6 +443,8 @@ describe('StreamingEngine', () => {
onMetadata = jasmine.createSpy('onMetadata');
getBandwidthEstimate = jasmine.createSpy('getBandwidthEstimate');
getBandwidthEstimate.and.returnValue(1e3);
getPlaybackRate = jasmine.createSpy('getPlaybackRate');
getPlaybackRate.and.returnValue(1);
disableStream = jasmine.createSpy('disableStream');
disableStream.and.callFake(() => false);

Expand All @@ -462,6 +466,7 @@ describe('StreamingEngine', () => {
const playerInterface = {
getPresentationTime: () => presentationTimeInSeconds,
getBandwidthEstimate: Util.spyFunc(getBandwidthEstimate),
getPlaybackRate: Util.spyFunc(getPlaybackRate),
mediaSourceEngine: mediaSourceEngine,
netEngine: /** @type {!shaka.net.NetworkingEngine} */(netEngine),
onError: Util.spyFunc(onError),
Expand Down
2 changes: 2 additions & 0 deletions test/test/util/simple_fakes.js
Expand Up @@ -501,6 +501,8 @@ shaka.test.FakeSegmentIndex = class {
nextPosition = this.find(time);
return this.get(nextPosition++);
},

setReverse: () => {},
};
});
}
Expand Down