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

Return undefined from getCurrentTime() when it is unknown #131

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
80 changes: 74 additions & 6 deletions script-test/bigscreenplayertest.js
Original file line number Diff line number Diff line change
Expand Up @@ -252,7 +252,7 @@ require(
expect(callback).toHaveBeenCalledWith({state: MediaState.WAITING, isSeeking: false, endOfStream: false});
});

it('should set the isPaused flag to true when waiting after a setCurrentTime', function () {
it('should set the isSeeking flag to true when waiting after a setCurrentTime', function () {
mockEventHook({data: {state: MediaState.PLAYING}});

expect(callback).toHaveBeenCalledWith({state: MediaState.PLAYING, endOfStream: false});
Expand All @@ -265,16 +265,24 @@ require(
expect(callback).toHaveBeenCalledWith({state: MediaState.WAITING, isSeeking: true, endOfStream: false});
});

it('should set clear the isPaused flag after a waiting event is fired', function () {
it('should set the isSeeking flag to false when it is a normal waiting state', function () {
mockEventHook({data: {state: MediaState.PLAYING}});

mockEventHook({data: {state: MediaState.WAITING}});

expect(callback).toHaveBeenCalledWith({state: MediaState.WAITING, isSeeking: false, endOfStream: false});
});

it('should set the isSeeking flag to false when it is a normal waiting state after a previous seek', function () {
mockEventHook({data: {state: MediaState.PLAYING}});

bigscreenPlayer.setCurrentTime(60);
mockEventHook({data: {state: MediaState.WAITING}});

expect(callback).toHaveBeenCalledWith({state: MediaState.WAITING, isSeeking: true, endOfStream: false});

mockEventHook({data: {state: MediaState.PLAYING}});
callback.calls.reset();

mockEventHook({data: {state: MediaState.WAITING}});

expect(callback).toHaveBeenCalledWith({state: MediaState.WAITING, isSeeking: false, endOfStream: false});
Expand Down Expand Up @@ -473,16 +481,75 @@ require(
});

describe('getCurrentTime', function () {
it('should return the current time from the strategy', function () {
it('should return the initialPlaybackTime before player initialised', function () {
initialiseBigscreenPlayer({initialPlaybackTime: 123});

mockPlayerComponentInstance.getCurrentTime.and.returnValue(10);

expect(bigscreenPlayer.getCurrentTime()).toBe(123);

mockEventHook({data: {state: MediaState.WAITING}});

expect(bigscreenPlayer.getCurrentTime()).toBe(123);
});

it('should return the latest requested time before player initialised', function () {
initialiseBigscreenPlayer({initialPlaybackTime: 123});

mockPlayerComponentInstance.getCurrentTime.and.returnValue(10);

expect(bigscreenPlayer.getCurrentTime()).toBe(123);

bigscreenPlayer.setCurrentTime(11);

expect(bigscreenPlayer.getCurrentTime()).toBe(11);
});

it('should return undefined before player initialised if there is no initialPlaybackTime', function () {
initialiseBigscreenPlayer();

mockPlayerComponentInstance.getCurrentTime.and.returnValue(10);

expect(bigscreenPlayer.getCurrentTime()).toBe(undefined);
});

it('should return the current time from the strategy once player initialised', function () {
initialiseBigscreenPlayer({initialPlaybackTime: 123});

mockPlayerComponentInstance.getCurrentTime.and.returnValue(10);

mockEventHook({data: {state: MediaState.PAUSED}});

expect(bigscreenPlayer.getCurrentTime()).toBe(10);
});

it('should return 0 if bigscreenPlayer is not initialised', function () {
expect(bigscreenPlayer.getCurrentTime()).toBe(0);
it('should return undefined if bigscreenPlayer is not initialised', function () {
expect(bigscreenPlayer.getCurrentTime()).toBe(undefined);
});

it('should return the value provided to setCurrentTime whilst seek in flight', function () {
initialiseBigscreenPlayer();

mockPlayerComponentInstance.getCurrentTime.and.returnValue(10);
bigscreenPlayer.setCurrentTime(11);

expect(bigscreenPlayer.getCurrentTime()).toBe(11);

mockEventHook({data: {state: MediaState.PAUSED}});

expect(bigscreenPlayer.getCurrentTime()).toBe(11);

bigscreenPlayer.setCurrentTime(12);

expect(bigscreenPlayer.getCurrentTime()).toBe(12);

mockEventHook({data: {state: MediaState.WAITING}});

expect(bigscreenPlayer.getCurrentTime()).toBe(12);

mockEventHook({data: {state: MediaState.PLAYING}});

expect(bigscreenPlayer.getCurrentTime()).toBe(10);
});
});

Expand Down Expand Up @@ -530,6 +597,7 @@ require(

it('should return true when playing live and current time is within tolerance of seekable range end', function () {
initialiseBigscreenPlayer({windowType: WindowTypes.SLIDING});
mockEventHook({data: {state: MediaState.PLAYING}});

mockPlayerComponentInstance.getCurrentTime.and.returnValue(100);
mockPlayerComponentInstance.getSeekableRange.and.returnValue({start: 0, end: 105});
Expand Down
9 changes: 5 additions & 4 deletions script-test/playbackstrategies/msestrategytest.js
Original file line number Diff line number Diff line change
Expand Up @@ -26,8 +26,8 @@ require(
var mockPlugins;
var mockPluginsInterface;
var mockDynamicWindowUtils;
var mockAudioElement = document.createElement('audio');
var mockVideoElement = document.createElement('video');
var mockAudioElement = document.createElement('div');
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Setting mockVideoElement.readyState does not work if it's a real element

var mockVideoElement = document.createElement('div');
var mockRefresher;
var testManifestObject;
var timeUtilsMock;
Expand Down Expand Up @@ -512,16 +512,17 @@ require(
it('returns the correct time from the DASH Mediaplayer', function () {
setUpMSE();
mockVideoElement.currentTime = 10;
mockVideoElement.readyState = 1;

mseStrategy.load(null, 0);

expect(mseStrategy.getCurrentTime()).toBe(10);
});

it('returns 0 when MediaPlayer is undefined', function () {
it('returns undefined when MediaPlayer is undefined', function () {
setUpMSE();

expect(mseStrategy.getCurrentTime()).toBe(0);
expect(mseStrategy.getCurrentTime()).toBe(undefined);
});
});

Expand Down
27 changes: 21 additions & 6 deletions script/bigscreenplayer.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
define('bigscreenplayer/bigscreenplayer',
[
'bigscreenplayer/models/mediastate',
'bigscreenplayer/models/seekstate',
'bigscreenplayer/playercomponent',
'bigscreenplayer/models/pausetriggers',
'bigscreenplayer/dynamicwindowutils',
Expand All @@ -13,7 +14,7 @@ define('bigscreenplayer/bigscreenplayer',
'bigscreenplayer/mediasources',
'bigscreenplayer/version'
],
function (MediaState, PlayerComponent, PauseTriggers, DynamicWindowUtils, WindowTypes, MockBigscreenPlayer, Plugins, Chronicle, DebugTool, SlidingWindowUtils, MediaSources, Version) {
function (MediaState, SeekState, PlayerComponent, PauseTriggers, DynamicWindowUtils, WindowTypes, MockBigscreenPlayer, Plugins, Chronicle, DebugTool, SlidingWindowUtils, MediaSources, Version) {
'use strict';
function BigscreenPlayer () {
var stateChangeCallbacks = [];
Expand All @@ -24,7 +25,8 @@ define('bigscreenplayer/bigscreenplayer',
var serverDate;
var playerComponent;
var pauseTrigger;
var isSeeking = false;
var seekState = SeekState.NONE;
var seekTime;
var endOfStream;
var windowType;
var device;
Expand Down Expand Up @@ -57,8 +59,11 @@ define('bigscreenplayer/bigscreenplayer',
}

if (evt.data.state === MediaState.WAITING) {
stateObject.isSeeking = isSeeking;
isSeeking = false;
stateObject.isSeeking = seekState !== SeekState.NONE;
seekState = SeekState.IN_FLIGHT;
} else if (seekState === SeekState.IN_FLIGHT) {
seekState = SeekState.NONE;
seekTime = undefined;
}

stateObject.endOfStream = endOfStream;
Expand Down Expand Up @@ -145,6 +150,11 @@ define('bigscreenplayer/bigscreenplayer',
callbacks = {};
}

// always pretend there is a seek in progress so that we return a stable
// time from `getCurrentTime` before the strategy is ready
seekState = SeekState.IN_FLIGHT;
seekTime = bigscreenPlayerData.initialPlaybackTime;

var mediaSourceCallbacks = {
onSuccess: function () {
bigscreenPlayerDataLoaded(playbackElement, bigscreenPlayerData, enableSubtitles, device, callbacks.onSuccess);
Expand Down Expand Up @@ -201,13 +211,18 @@ define('bigscreenplayer/bigscreenplayer',
setCurrentTime: function (time) {
DebugTool.apicall('setCurrentTime');
if (playerComponent) {
isSeeking = true; // this flag must be set before calling into playerComponent.setCurrentTime - as this synchronously fires a WAITING event (when native strategy).
seekState = SeekState.QUEUEUD; // this must be set before calling into playerComponent.setCurrentTime - as this synchronously fires a WAITING event (when native strategy).
seekTime = time;
playerComponent.setCurrentTime(time);
endOfStream = windowType !== WindowTypes.STATIC && Math.abs(this.getSeekableRange().end - time) < END_OF_STREAM_TOLERANCE;
}
},
getCurrentTime: function () {
return playerComponent && playerComponent.getCurrentTime() || 0;
if (seekState !== SeekState.NONE) {
// always return the position we are seeking to whilst seek in flight
return seekTime;
}
return playerComponent && playerComponent.getCurrentTime() || undefined;
},
getMediaKind: function () {
return mediaKind;
Expand Down
23 changes: 23 additions & 0 deletions script/models/seekstate.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
define(
'bigscreenplayer/models/seekstate',
/**
* Provides an enumeration of possible media states.
*/
function () {
'use strict';

/**
* Enumeration of possible seek states.
*/
var SeekState = {
/** No seek in progress. */
NONE: 0,
/** A seek is about to be processed. */
QUEUED: 1,
/** A seek is in progress. */
IN_FLIGHT: 2
};

return SeekState;
}
);
5 changes: 4 additions & 1 deletion script/playbackstrategy/msestrategy.js
Original file line number Diff line number Diff line change
Expand Up @@ -406,7 +406,10 @@ define('bigscreenplayer/playbackstrategy/msestrategy',
}

function getCurrentTime () {
return (mediaElement) ? mediaElement.currentTime - timeCorrection : 0;
if (!mediaPlayer || !mediaPlayer.isReady() || !mediaElement || mediaElement.readyState < 1 /* HAVE_METADATA */) {
return undefined;
}
return mediaElement.currentTime - timeCorrection;
}

function refreshManifestBeforeSeek (seekToTime) {
Expand Down