Skip to content

Commit

Permalink
Make Buffering Observer the Authority on "Is Buffering"
Browse files Browse the repository at this point in the history 
Previously there was a boolean that held the "is buffering" state,
however with the buffering observer directly assessable, we can just ask
it if we have enough content.

Change-Id: Ic5adb2b54fc7a7f6d732520ac257bf4ca1407287
  • Loading branch information
@vaage
vaage committed Mar 29, 2019
1 parent 1284dd4 commit 2d9c6d6
Show file tree
Hide file tree
Showing 2 changed files with 95 additions and 59 deletions.
54 changes: 32 additions & 22 deletions lib/player.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -154,9 +154,6 @@ shaka.Player = function(mediaElement, dependencyInjector) {
/** @private {!Set.<shaka.extern.Stream>} */
this.loadingTextStreams_ = new Set();

/** @private {boolean} */
this.isBuffering_ = false;

/** @private {boolean} */
this.switchingPeriods_ = true;

Expand Down Expand Up @@ -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_();
};


Expand Down Expand Up @@ -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_();
Expand Down Expand Up @@ -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.
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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_();
}
};

Expand Down Expand Up @@ -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;
};


Expand Down Expand Up @@ -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);
};

Expand All @@ -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');
Expand Down Expand Up @@ -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
Expand Down
100 changes: 63 additions & 37 deletions test/player_unit.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand All @@ -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();
Expand All @@ -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() {
Expand Down Expand Up @@ -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),
Expand All @@ -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),
Expand All @@ -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),
Expand All @@ -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() {
Expand Down

0 comments on commit 2d9c6d6

Please sign in to comment.