diff --git a/lib/player.js b/lib/player.js index a89149d0c7..8ca2ba270c 100644 --- a/lib/player.js +++ b/lib/player.js @@ -154,9 +154,6 @@ shaka.Player = function(mediaElement, dependencyInjector) { /** @private {!Set.} */ this.loadingTextStreams_ = new Set(); - /** @private {boolean} */ - this.isBuffering_ = false; - /** @private {boolean} */ this.switchingPeriods_ = true; @@ -1130,8 +1127,8 @@ shaka.Player.prototype.onUnload_ = async function(has, wants) { this.stats_ = null; this.switchingPeriods_ = true; - // Force an exit from the buffering state. - this.onBuffering_(false); + // Make sure that the app knows of the new buffering state. + this.updateBufferState_(); }; @@ -1448,7 +1445,8 @@ shaka.Player.prototype.onLoad_ = async function(has, wants) { // Save the uri so that it can be used outside of the load-graph. this.assetUri_ = assetUri; - // Stats are for a single playback/load session. + // Stats are for a single playback/load session. Stats must be initialized + // before we allow calls to |updateStateHistory|. this.stats_ = new shaka.util.Stats(wants.startTimeOfLoad); const updateStateHistory = () => this.updateStateHistory_(); @@ -1491,7 +1489,6 @@ shaka.Player.prototype.onLoad_ = async function(has, wants) { this.playhead_ = this.createPlayhead(has.startTime); this.playheadObservers_ = this.createPlayheadObservers_(); - // Initializing the buffering controller needs to be near the end because it // will set the initial buffering state and that depends on other components // being initialized. @@ -1793,7 +1790,7 @@ shaka.Player.prototype.startBufferManagement_ = function(rebufferingGoal) { // Force us back to a buffering state. This ensure everything is starting in // the same state. this.bufferObserver_.setState(shaka.media.BufferingObserver.State.STARVING); - this.onBuffering_(true); + this.updateBufferState_(); // TODO: We should take some time to look into the effects of our // quarter-second refresh practice. We often use a quarter-second @@ -1828,9 +1825,7 @@ shaka.Player.prototype.pollBufferState_ = function() { // If the state changed, we need to surface the event. if (stateChanged) { - const state = this.bufferObserver_.getState(); - const buffering = state == shaka.media.BufferingObserver.State.STARVING; - this.onBuffering_(buffering); + this.updateBufferState_(); } }; @@ -2204,7 +2199,11 @@ shaka.Player.prototype.getExpiration = function() { * @export */ shaka.Player.prototype.isBuffering = function() { - return this.isBuffering_; + const State = shaka.media.BufferingObserver.State; + + return this.bufferObserver_ ? + this.bufferObserver_.getState() == State.STARVING : + false; }; @@ -3305,20 +3304,26 @@ shaka.Player.prototype.adjustStartTime_ = function(time) { /** - * Callback from PlayheadObserver. + * Update the buffering state to be either "we are buffering" or "we are not + * buffering", firing events to the app as needed. * - * @param {boolean} buffering * @private */ -shaka.Player.prototype.onBuffering_ = function(buffering) { - this.isBuffering_ = buffering; - this.updateStateHistory_(); +shaka.Player.prototype.updateBufferState_ = function() { + const isBuffering = this.isBuffering(); - if (this.playhead_) { - this.playhead_.setBuffering(buffering); + // Make sure we have all the components we need before we consider ourselves + // as being loaded. + const loaded = this.stats_ && this.bufferObserver_ && this.playhead_; + + // TODO: Make the check for "loaded" simpler. + if (loaded) { + this.updateStateHistory_(); + this.playhead_.setBuffering(isBuffering); } - let event = new shaka.util.FakeEvent('buffering', {'buffering': buffering}); + // Surface the buffering event so that the app knows if/when we are buffering. + let event = new shaka.util.FakeEvent('buffering', {'buffering': isBuffering}); this.dispatchEvent(event); }; @@ -3341,10 +3346,13 @@ shaka.Player.prototype.onChangePeriod_ = function() { shaka.Player.prototype.updateStateHistory_ = function() { // If we have not finish initializing, this will be a no-op. if (!this.stats_) { return; } + if (!this.bufferObserver_) { return; } + + const State = shaka.media.BufferingObserver.State; const history = this.stats_.getStateHistory(); - if (this.isBuffering_) { + if (this.bufferObserver_.getState() == State.STARVING) { history.update('buffering'); } else if (this.video_.paused) { history.update('paused'); @@ -4252,7 +4260,9 @@ shaka.Player.prototype.getPresentationText_ = function() { /** - * Check if we are buffered to the end of the presentation. + * Check if the player has buffered content to the end of the presentation. + * Ideally this should only be called when content is loaded, but will assume + * that if nothing can be buffered, it means it is not buffered to the end. * * @return {boolean} * @private diff --git a/test/player_unit.js b/test/player_unit.js index 03fb24629a..df694948ca 100644 --- a/test/player_unit.js +++ b/test/player_unit.js @@ -1859,8 +1859,7 @@ describe('Player', function() { expect(stats.bufferingTime).toBeCloseTo(0); // Stop buffering and start "playing". - buffering(false); - + forceBufferingTo(false); jasmine.clock().tick(5000); stats = player.getStats(); @@ -1873,7 +1872,7 @@ describe('Player', function() { expect(stats.playTime).toBeCloseTo(0); expect(stats.bufferingTime).toBeCloseTo(0); - buffering(true); + forceBufferingTo(true); jasmine.clock().tick(5000); stats = player.getStats(); @@ -1882,27 +1881,19 @@ describe('Player', function() { }); it('tracks correct time when switching states', function() { - buffering(false); + forceBufferingTo(false); jasmine.clock().tick(3000); - buffering(true); + forceBufferingTo(true); jasmine.clock().tick(5000); - buffering(true); + forceBufferingTo(true); jasmine.clock().tick(9000); - buffering(false); + forceBufferingTo(false); jasmine.clock().tick(1000); let stats = player.getStats(); expect(stats.playTime).toBeCloseTo(4); expect(stats.bufferingTime).toBeCloseTo(14); }); - - /** - * @param {boolean} buffering - * @suppress {accessControls} - */ - function buffering(buffering) { - player.onBuffering_(buffering); - } }); describe('.switchHistory', function() { @@ -1973,28 +1964,46 @@ describe('Player', function() { }); describe('.stateHistory', function() { + function history() { + return player.getStats().stateHistory; + } + + // We expect that the player will start us in the buffering state after + // loading. We should see that the only entry is a buffering entry. it('begins with buffering state', function() { - setBuffering(true); - expect(player.getStats().stateHistory).toEqual([ + expect(history()).toEqual([ { - // We are using a mock date, so this is not a race. - timestamp: Date.now() / 1000, + timestamp: jasmine.any(Number), duration: 0, state: 'buffering', }, ]); }); + // We expect that the player will start us in the buffering state, but + // when the media element is paused, we should see that we change to the + // paused state. it('transitions to paused if the video is paused', function() { - setBuffering(true); + // Start playback, we must be playing in order to be paused. The + // buffering state takes precedent over other states, so if we are + // buffering and then paused, it will only report buffering. + forceBufferingTo(false); + + // Trigger a pause event. video.paused = true; - setBuffering(false); - expect(player.getStats().stateHistory).toEqual([ + video.on['pause'](); + + expect(history()).toEqual([ { timestamp: jasmine.any(Number), duration: jasmine.any(Number), state: 'buffering', }, + { + timestamp: jasmine.any(Number), + duration: jasmine.any(Number), + state: 'playing', + }, { timestamp: jasmine.any(Number), duration: jasmine.any(Number), @@ -2003,11 +2012,13 @@ describe('Player', function() { ]); }); + // We expect that the player will start us in the buffering state, but + // once we leave that state, we should be playing. it('transitions to playing if the video is playing', function() { - setBuffering(true); - video.paused = false; - setBuffering(false); - expect(player.getStats().stateHistory).toEqual([ + // Leave buffering (and enter playing). + forceBufferingTo(false); + + expect(history()).toEqual([ { timestamp: jasmine.any(Number), duration: jasmine.any(Number), @@ -2021,14 +2032,19 @@ describe('Player', function() { ]); }); + // We expect that the player will start us in the buffering state. We + // expect that once we reach the end, we will enter the ended state. Since + // we must be first playing before reaching the end, this test will + // reflect that. it('transitions to ended when the video ends', function() { - setBuffering(false); + // Stop buffering (and start playing); + forceBufferingTo(false); + // Signal the playhead reaching the end of the content. video.ended = true; - // Fire an 'ended' event on the mock video. video.on['ended'](); - expect(player.getStats().stateHistory).toEqual([ + expect(history()).toEqual([ { timestamp: jasmine.any(Number), duration: jasmine.any(Number), @@ -2046,15 +2062,25 @@ describe('Player', function() { }, ]); }); - - /** - * @param {boolean} buffering - * @suppress {accessControls} - */ - function setBuffering(buffering) { - player.onBuffering_(buffering); - } }); + + /** + * @param {boolean} buffering + * @suppress {accessControls} + */ + function forceBufferingTo(buffering) { + const State = shaka.media.BufferingObserver.State; + + // Replace the |getState| method on the buffer controllers so that any + // others calls relying on the state will get the state that we want them + // to have. + player.bufferObserver_.getState = () => { + return buffering ? State.STARVING : State.SATISFIED; + }; + + // Force the update. + player.updateBufferState_(); + } }); describe('unplayable periods', function() {