Skip to content

Commit

Permalink
Fix end of stream detection on IE & Edge
Browse files Browse the repository at this point in the history
PlayheadObserver has to know when the stream has been fully buffered.
It did this based on duration and the buffered ranges, but that
required the use of a fudge factor and caused false negatives on IE &
Edge.

Instead, use MediaSource's  readyState attribute to decide when we've
buffered to the end.  This way, IE & Edge do not go into a buffering
state when the discrepancy between audio & video sizes is larger than
an arbitrary fudge factor.

Closes #913

Change-Id: I307dce12a883201a88a18bf5b9a84f954a1c5033
  • Loading branch information
joeyparrish committed Aug 10, 2017
1 parent 404d42c commit efd184f
Show file tree
Hide file tree
Showing 3 changed files with 32 additions and 27 deletions.
26 changes: 11 additions & 15 deletions lib/media/playhead_observer.js
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@ goog.require('shaka.util.StreamUtils');
* </ul>
*
* @param {HTMLMediaElement} video
* @param {MediaSource} mediaSource
* @param {shakaExtern.Manifest} manifest
* @param {shakaExtern.StreamingConfiguration} config
* @param {function(boolean)} onBuffering Called and passed true when stopped
Expand All @@ -52,10 +53,14 @@ goog.require('shaka.util.StreamUtils');
* @implements {shaka.util.IDestroyable}
*/
shaka.media.PlayheadObserver = function(
video, manifest, config, onBuffering, onEvent, onChangePeriod) {
video, mediaSource, manifest, config, onBuffering, onEvent,
onChangePeriod) {
/** @private {HTMLMediaElement} */
this.video_ = video;

/** @private {MediaSource} */
this.mediaSource_ = mediaSource;

/** @private {?shakaExtern.Manifest} */
this.manifest_ = manifest;

Expand Down Expand Up @@ -101,16 +106,6 @@ shaka.media.PlayheadObserver = function(
shaka.media.PlayheadObserver.UNDERFLOW_THRESHOLD_ = 0.5;


/**
* A fudge factor used when comparing buffered ranges to the duration to
* determine if we have buffered all available content.
*
* @private {number}
* @const
*/
shaka.media.PlayheadObserver.FUDGE_FACTOR_ = 0.1;


/**
* @enum {number}
* @private
Expand Down Expand Up @@ -146,6 +141,7 @@ shaka.media.PlayheadObserver.prototype.destroy = function() {
this.cancelWatchdogTimer_();

this.video_ = null;
this.mediaSource_ = null;
this.manifest_ = null;
this.config_ = null;
this.onBuffering_ = null;
Expand Down Expand Up @@ -310,14 +306,14 @@ shaka.media.PlayheadObserver.prototype.onWatchdogTimer_ = function() {
var bufferedAhead = shaka.media.TimeRangesUtils.bufferedAheadOf(
this.video_.buffered, this.video_.currentTime);
var bufferEnd = shaka.media.TimeRangesUtils.bufferEnd(this.video_.buffered);

var fudgeFactor = shaka.media.PlayheadObserver.FUDGE_FACTOR_;
var threshold = shaka.media.PlayheadObserver.UNDERFLOW_THRESHOLD_;

var timeline = this.manifest_.presentationTimeline;
var duration = timeline.getSegmentAvailabilityEnd() - fudgeFactor;
var liveEdge = timeline.getSegmentAvailabilityEnd();
var bufferedToLiveEdge = timeline.isLive() && bufferEnd >= liveEdge;
var noMoreSegments = this.mediaSource_.readyState == 'ended';

var atEnd = (bufferEnd >= duration) || (this.video_.ended);
var atEnd = bufferedToLiveEdge || this.video_.ended || noMoreSegments;
if (!this.buffering_) {
// If there are no buffered ranges but the playhead is at the end of
// the video then we shouldn't enter a buffering state.
Expand Down
2 changes: 1 addition & 1 deletion lib/player.js
Original file line number Diff line number Diff line change
Expand Up @@ -817,7 +817,7 @@ shaka.Player.prototype.createPlayhead = function(opt_startTime) {
shaka.Player.prototype.createPlayheadObserver = function() {
goog.asserts.assert(this.manifest_, 'Must have manifest');
return new shaka.media.PlayheadObserver(
this.video_, this.manifest_, this.config_.streaming,
this.video_, this.mediaSource_, this.manifest_, this.config_.streaming,
this.onBuffering_.bind(this), this.onEvent_.bind(this),
this.onChangePeriod_.bind(this));
};
Expand Down
31 changes: 20 additions & 11 deletions test/media/playhead_observer_unit.js
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,10 @@ describe('PlayheadObserver', function() {
var observer;
/** @type {!shaka.test.FakeVideo} */
var video;
/** @type {{readyState: string}} */
var mockMediaSource;
/** @type {?} */
var mediaSource;
/** @type {!shaka.test.FakePresentationTimeline} */
var timeline;
/** @type {shakaExtern.Manifest} */
Expand Down Expand Up @@ -51,6 +55,9 @@ describe('PlayheadObserver', function() {
video.duration = 60;
video.buffered = createFakeBuffered([]);

mockMediaSource = { readyState: 'open' };
mediaSource = /** @type {?} */(mockMediaSource);

timeline = new shaka.test.FakePresentationTimeline();

manifest = {
Expand All @@ -77,8 +84,9 @@ describe('PlayheadObserver', function() {
onChangePeriod = jasmine.createSpy('onChangePeriod');
onEvent = jasmine.createSpy('onEvent');

// The observer should only call getSegmentAvailabilityEnd.
// The observer may only call methods mocked after this.
shaka.test.Util.makeMockObjectStrict(timeline);
timeline.isLive.and.returnValue(false);
timeline.getSegmentAvailabilityEnd.and.returnValue(60);
});

Expand All @@ -91,7 +99,7 @@ describe('PlayheadObserver', function() {
video.buffered = createFakeBuffered([{start: 0, end: 20}]);
video.currentTime = 0;
observer = new shaka.media.PlayheadObserver(
video, manifest, config, Util.spyFunc(onBuffering),
video, mediaSource, manifest, config, Util.spyFunc(onBuffering),
Util.spyFunc(onEvent), Util.spyFunc(onChangePeriod));

observer.seeked();
Expand All @@ -103,7 +111,7 @@ describe('PlayheadObserver', function() {
video.buffered = createFakeBuffered([{start: 0, end: 20}]);
video.currentTime = 0;
observer = new shaka.media.PlayheadObserver(
video, manifest, config, Util.spyFunc(onBuffering),
video, mediaSource, manifest, config, Util.spyFunc(onBuffering),
Util.spyFunc(onEvent), Util.spyFunc(onChangePeriod));

video.currentTime = 20;
Expand All @@ -116,7 +124,7 @@ describe('PlayheadObserver', function() {
video.buffered = createFakeBuffered([{start: 0, end: 20}]);
video.currentTime = 0;
observer = new shaka.media.PlayheadObserver(
video, manifest, config, Util.spyFunc(onBuffering),
video, mediaSource, manifest, config, Util.spyFunc(onBuffering),
Util.spyFunc(onEvent), Util.spyFunc(onChangePeriod));

video.currentTime = 40;
Expand All @@ -130,7 +138,7 @@ describe('PlayheadObserver', function() {
video.buffered = createFakeBuffered([{start: 0, end: 20}]);
video.currentTime = 0;
observer = new shaka.media.PlayheadObserver(
video, manifest, config, Util.spyFunc(onBuffering),
video, mediaSource, manifest, config, Util.spyFunc(onBuffering),
Util.spyFunc(onEvent), Util.spyFunc(onChangePeriod));

video.currentTime = 22;
Expand All @@ -149,7 +157,7 @@ describe('PlayheadObserver', function() {
video.buffered = createFakeBuffered([]);
video.currentTime = 0;
observer = new shaka.media.PlayheadObserver(
video, manifest, config, Util.spyFunc(onBuffering),
video, mediaSource, manifest, config, Util.spyFunc(onBuffering),
Util.spyFunc(onEvent), Util.spyFunc(onChangePeriod));

jasmine.clock().tick(1000);
Expand All @@ -167,7 +175,7 @@ describe('PlayheadObserver', function() {
video.buffered = createFakeBuffered([]);
video.currentTime = 0;
observer = new shaka.media.PlayheadObserver(
video, manifest, config, Util.spyFunc(onBuffering),
video, mediaSource, manifest, config, Util.spyFunc(onBuffering),
Util.spyFunc(onEvent), Util.spyFunc(onChangePeriod));

jasmine.clock().tick(1000);
Expand All @@ -189,13 +197,14 @@ describe('PlayheadObserver', function() {
timeline.getSegmentAvailabilityEnd.and.returnValue(60);

observer = new shaka.media.PlayheadObserver(
video, manifest, config, Util.spyFunc(onBuffering),
video, mediaSource, manifest, config, Util.spyFunc(onBuffering),
Util.spyFunc(onEvent), Util.spyFunc(onChangePeriod));

jasmine.clock().tick(1000);
expect(onBuffering).not.toHaveBeenCalled();

video.currentTime = 60;
mockMediaSource.readyState = 'ended';
jasmine.clock().tick(1000);
expect(onBuffering).not.toHaveBeenCalled();
});
Expand All @@ -208,7 +217,7 @@ describe('PlayheadObserver', function() {
timeline.getSegmentAvailabilityEnd.and.returnValue(60);

observer = new shaka.media.PlayheadObserver(
video, manifest, config, Util.spyFunc(onBuffering),
video, mediaSource, manifest, config, Util.spyFunc(onBuffering),
Util.spyFunc(onEvent), Util.spyFunc(onChangePeriod));

jasmine.clock().tick(1000);
Expand Down Expand Up @@ -253,7 +262,7 @@ describe('PlayheadObserver', function() {
video.buffered = createFakeBuffered([{start: 0, end: 60}]);
video.currentTime = 0;
observer = new shaka.media.PlayheadObserver(
video, manifest, config, Util.spyFunc(onBuffering),
video, mediaSource, manifest, config, Util.spyFunc(onBuffering),
Util.spyFunc(onEvent), Util.spyFunc(onChangePeriod));
});

Expand Down Expand Up @@ -469,7 +478,7 @@ describe('PlayheadObserver', function() {
];

observer = new shaka.media.PlayheadObserver(
video, manifest, config, Util.spyFunc(onBuffering),
video, mediaSource, manifest, config, Util.spyFunc(onBuffering),
Util.spyFunc(onEvent), Util.spyFunc(onChangePeriod));

// Ignore the call for the initial Period.
Expand Down

0 comments on commit efd184f

Please sign in to comment.