Skip to content

Commit

Permalink
fix: Content reload starttime with HLS on iOS (#4575)
Browse files Browse the repository at this point in the history 
A new method `shaka.media.Playhead.ready` has been added for start time
operations.

Fixes #4244
  • Loading branch information
@alexandercerutti @joeyparrish
alexandercerutti authored and joeyparrish committed Nov 8, 2022
1 parent 5e0e942 commit 7fea5c2
Show file tree
Hide file tree
Showing 4 changed files with 70 additions and 1 deletion.
24 changes: 23 additions & 1 deletion lib/media/playhead.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,14 @@ goog.requireType('shaka.media.PresentationTimeline');
* @interface
*/
shaka.media.Playhead = class {
/**
* Called when the Player is ready to begin playback. Anything that depends
* on setStartTime() should be done here, not in the constructor.
*
* @see https://github.com/shaka-project/shaka-player/issues/4244
*/
ready() {}

/**
* Set the start time. If the content has already started playback, this will
* be ignored.
Expand Down Expand Up @@ -76,6 +84,14 @@ shaka.media.SrcEqualsPlayhead = class {

/** @private {shaka.util.EventManager} */
this.eventManager_ = new shaka.util.EventManager();
}

/** @override */
ready() {
goog.asserts.assert(
this.mediaElement_ != null,
'Playhead should not be released before calling ready()',
);

// We listen for the loaded-data-event so that we know when we can
// interact with |currentTime|.
Expand All @@ -88,6 +104,7 @@ shaka.media.SrcEqualsPlayhead = class {
this.eventManager_.listenOnce(this.mediaElement_, 'seeking', () => {
this.started_ = true;
});

const currentTime = this.mediaElement_.currentTime;
// Using the currentTime allows using a negative number in Live HLS
const newTime = Math.max(0, currentTime + this.startTime_);
Expand Down Expand Up @@ -211,7 +228,12 @@ shaka.media.MediaSourcePlayhead = class {
/** @type {shaka.util.Timer} */
this.checkWindowTimer_ = new shaka.util.Timer(() => {
this.onPollWindow_();
}).tickEvery(/* seconds= */ 0.25);
});
}

/** @override */
ready() {
this.checkWindowTimer_.tickEvery(/* seconds= */ 0.25);
}

/** @override */
Expand Down
3 changes: 3 additions & 0 deletions lib/player.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2040,6 +2040,8 @@ shaka.Player = class extends shaka.util.FakeEventTarget {
shaka.util.StreamUtils.variantToTrack(initialVariant));
}

this.playhead_.ready();

// Decide if text should be shown automatically.
// similar to video/audio track, we would skip switch initial text track
// if user already pick text track (via selectTextTrack api)
Expand Down Expand Up @@ -2374,6 +2376,7 @@ shaka.Player = class extends shaka.util.FakeEventTarget {
HTMLMediaElement.HAVE_METADATA,
this.loadEventManager_,
() => {
this.playhead_.ready();
fullyLoaded.resolve();
});

Expand Down
41 changes: 41 additions & 0 deletions test/media/playhead_unit.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -171,6 +171,8 @@ describe('Playhead', () => {
Util.spyFunc(onSeek),
Util.spyFunc(onEvent));

playhead.ready();

expect(video.currentTime).toBe(5);
expect(playhead.getTime()).toBe(5);

Expand All @@ -192,6 +194,8 @@ describe('Playhead', () => {
Util.spyFunc(onSeek),
Util.spyFunc(onEvent));

playhead.ready();

expect(video.addEventListener).toHaveBeenCalledWith(
'loadedmetadata', jasmine.any(Function), jasmine.anything());
expect(video.addEventListener).not.toHaveBeenCalledWith(
Expand Down Expand Up @@ -233,6 +237,8 @@ describe('Playhead', () => {
Util.spyFunc(onSeek),
Util.spyFunc(onEvent));

playhead.ready();

video.on['seeking']();
expect(playhead.getTime()).toBe(5);
expect(video.currentTime).toBe(5);
Expand Down Expand Up @@ -266,6 +272,8 @@ describe('Playhead', () => {
video, manifest, config, /* startTime= */ 60, Util.spyFunc(onSeek),
Util.spyFunc(onEvent));

playhead.ready();

expect(playhead.getTime()).toBe(59); // duration - durationBackoff
expect(video.currentTime).toBe(59); // duration - durationBackoff
});
Expand Down Expand Up @@ -296,6 +304,8 @@ describe('Playhead', () => {
video, manifest, config, /* startTime= */ -40, Util.spyFunc(onSeek),
Util.spyFunc(onEvent));

playhead.ready();

expect(playhead.getTime()).toBe(30);
});

Expand All @@ -308,6 +318,8 @@ describe('Playhead', () => {
Util.spyFunc(onSeek),
Util.spyFunc(onEvent));

playhead.ready();

expect(video.addEventListener).toHaveBeenCalledWith(
'loadedmetadata', jasmine.any(Function), jasmine.anything());

Expand Down Expand Up @@ -341,6 +353,8 @@ describe('Playhead', () => {
Util.spyFunc(onSeek),
Util.spyFunc(onEvent));

playhead.ready();

expect(video.addEventListener).toHaveBeenCalledWith(
'loadedmetadata', jasmine.any(Function), jasmine.anything());

Expand Down Expand Up @@ -374,6 +388,8 @@ describe('Playhead', () => {
Util.spyFunc(onSeek),
Util.spyFunc(onEvent));

playhead.ready();

// This has to periodically increment the mock date to allow the onSeeking_
// handler to seek, if appropriate.

Expand Down Expand Up @@ -646,6 +662,8 @@ describe('Playhead', () => {
expect(currentTime).toBe(1000);
seekCount = 0;

playhead.ready();

// The availability window slips ahead.
timeline.getSeekRangeStart.and.returnValue(1030);
timeline.getSeekRangeEnd.and.returnValue(1030);
Expand Down Expand Up @@ -690,6 +708,8 @@ describe('Playhead', () => {
Util.spyFunc(onSeek),
Util.spyFunc(onEvent));

playhead.ready();

video.on['seeking']();
expect(video.currentTime).toBe(5);
expect(playhead.getTime()).toBe(5);
Expand Down Expand Up @@ -722,6 +742,8 @@ describe('Playhead', () => {
Util.spyFunc(onSeek),
Util.spyFunc(onEvent));

playhead.ready();

video.on['seeking']();
expect(video.currentTime).toBe(5);
expect(playhead.getTime()).toBe(5);
Expand Down Expand Up @@ -759,6 +781,9 @@ describe('Playhead', () => {

video.currentTime = 0;
video.seeking = true;

playhead.ready();

// "video.seeking" stays true until the buffered range intersects with
// "video.currentTime". Playhead should correct anyway.
video.on['seeking']();
Expand Down Expand Up @@ -803,6 +828,8 @@ describe('Playhead', () => {
Util.spyFunc(onSeek),
Util.spyFunc(onEvent));

playhead.ready();

/**
* Prevent retries on the initial start time seek. This will ensure that
* only one call is made for the initial seek, and that any additional calls
Expand Down Expand Up @@ -955,6 +982,8 @@ describe('Playhead', () => {
Util.spyFunc(onSeek),
Util.spyFunc(onEvent));

playhead.ready();

jasmine.clock().tick(500);
for (let time = data.start; time < data.waitingAt; time++) {
// We don't want to run tick() for 1 second because it will trigger
Expand Down Expand Up @@ -1208,6 +1237,8 @@ describe('Playhead', () => {
Util.spyFunc(onSeek),
Util.spyFunc(onEvent));

playhead.ready();

jasmine.clock().tick(500);
expect(onEvent).not.toHaveBeenCalled();

Expand Down Expand Up @@ -1257,6 +1288,8 @@ describe('Playhead', () => {
Util.spyFunc(onSeek),
Util.spyFunc(onEvent));

playhead.ready();

playhead.notifyOfBufferingChange();
jasmine.clock().tick(500);

Expand All @@ -1280,6 +1313,8 @@ describe('Playhead', () => {
Util.spyFunc(onSeek),
Util.spyFunc(onEvent));

playhead.ready();

playhead.notifyOfBufferingChange();
jasmine.clock().tick(500);

Expand All @@ -1305,6 +1340,8 @@ describe('Playhead', () => {
Util.spyFunc(onSeek),
Util.spyFunc(onEvent));

playhead.ready();

playhead.notifyOfBufferingChange();
jasmine.clock().tick(500);

Expand All @@ -1330,6 +1367,8 @@ describe('Playhead', () => {
Util.spyFunc(onSeek),
Util.spyFunc(onEvent));

playhead.ready();

playhead.notifyOfBufferingChange();
jasmine.clock().tick(500);

Expand Down Expand Up @@ -1362,6 +1401,8 @@ describe('Playhead', () => {
Util.spyFunc(onSeek),
Util.spyFunc(onEvent));

playhead.ready();

jasmine.clock().tick(500);
expect(onEvent).not.toHaveBeenCalled();

Expand Down
3 changes: 3 additions & 0 deletions test/test/util/simple_fakes.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -333,6 +333,9 @@ shaka.test.FakePlayhead = class {
/** @private {number} */
this.startTime_ = 0;

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

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

0 comments on commit 7fea5c2

Please sign in to comment.