Skip to content

Commit

Permalink
fix: Fix exception with streaming.startAtSegmentBoundary (#4216)
Browse files Browse the repository at this point in the history
v4.0.0 introduced a regression.  When SegmentIterator's seek() method
was removed, one reference to it was missed in Player.  This was only
used when streaming.startAtSegmentBoundary is true, and there were no
tests to exercise that code path.

This adds new tests and fixes the regression.

Closes #4188
  • Loading branch information
joeyparrish committed May 17, 2022
1 parent ae84aac commit 24eac2c
Show file tree
Hide file tree
Showing 3 changed files with 63 additions and 4 deletions.
3 changes: 2 additions & 1 deletion lib/player.js
Expand Up @@ -5010,7 +5010,8 @@ shaka.Player = class extends shaka.util.FakeEventTarget {
}

await stream.createSegmentIndex();
const ref = stream.segmentIndex[Symbol.iterator]().seek(time);
const iter = stream.segmentIndex.getIteratorForTime(time);
const ref = iter ? iter.next().value : null;
if (!ref) {
return null;
}
Expand Down
53 changes: 52 additions & 1 deletion test/player_unit.js
Expand Up @@ -124,7 +124,13 @@ describe('Player', () => {

player.createDrmEngine = () => drmEngine;
player.createNetworkingEngine = () => networkingEngine;
player.createPlayhead = () => playhead;
player.createPlayhead = (startTime) => {
const callableSetStartTime =
shaka.test.Util.spyFunc(playhead.setStartTime);
callableSetStartTime(startTime);
playhead.setStartTime.calls.reset();
return playhead;
};
player.createMediaSourceEngine = () => mediaSourceEngine;
player.createStreamingEngine = () => streamingEngine;
}
Expand Down Expand Up @@ -3710,6 +3716,51 @@ describe('Player', () => {
});
});

describe('config streaming.startAtSegmentBoundary', () => {
// Test for https://github.com/shaka-project/shaka-player/issues/4188
// In that issue, using streaming.startAtSegmentBoundary in v4.0.0 caused
// an exception due to the use of a removed method.
it('adjusts start time', async () => {
player.configure('streaming.startAtSegmentBoundary', true);

// In order to adjust the start time to a segment boundary, we need
// segment descriptions in the segment index. So create a non-default
// fake manifest.
const timeline = new shaka.media.PresentationTimeline(300, 0);
timeline.setStatic(true);
// This duration is used by useSegmentTemplate below to decide how many
// references to generate.
timeline.setDuration(300);
manifest = shaka.test.ManifestGenerator.generate((manifest) => {
manifest.presentationTimeline = timeline;
manifest.addVariant(0, (variant) => {
variant.addVideo(1, (stream) => {
stream.useSegmentTemplate(
'$Number$.mp4', /* segmentDuration= */ 10);
});
});
});

await player.load(fakeManifestUri, 12, fakeMimeType);

expect(playhead.setStartTime).toHaveBeenCalledWith(10);
});

it('does not fail with no segments', async () => {
player.configure('streaming.startAtSegmentBoundary', true);

// Without useSegmentTemplate in the fake manifest, the call to
// getIteratorForTime() in Player produces a null iterator. Using the
// default fake manifest should cover that case. But just to make sure,
// verify that we have no segments before calling load().
const variant = manifest.variants[0];
expect(variant.video.segmentIndex.getIteratorForTime(0)).toBeNull();
expect(variant.audio.segmentIndex.getIteratorForTime(0)).toBeNull();

await player.load(fakeManifestUri, 0, fakeMimeType);
});
});

/**
* Gets the currently active variant track.
* @return {shaka.extern.Track}
Expand Down
11 changes: 9 additions & 2 deletions test/test/util/simple_fakes.js
Expand Up @@ -308,11 +308,18 @@ shaka.test.FakePlayhead = class {
/** @type {!jasmine.Spy} */
this.setRebufferingGoal = jasmine.createSpy('setRebufferingGoal');

/** @private {number} */
this.startTime_ = 0;

/** @type {!jasmine.Spy} */
this.setStartTime = jasmine.createSpy('setStartTime');
this.setStartTime = jasmine.createSpy('setStartTime')
.and.callFake((value) => {
this.startTime_ = value;
});

/** @type {!jasmine.Spy} */
this.getTime = jasmine.createSpy('getTime').and.returnValue(0);
this.getTime = jasmine.createSpy('getTime')
.and.callFake(() => this.startTime_);

/** @type {!jasmine.Spy} */
this.setBuffering = jasmine.createSpy('setBuffering');
Expand Down

0 comments on commit 24eac2c

Please sign in to comment.