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

Backported to v2.1.x

Change-Id: I307dce12a883201a88a18bf5b9a84f954a1c5033
  • Loading branch information
@joeyparrish
joeyparrish committed Aug 11, 2017
1 parent 87b40b1 commit f0e89a7
Show file tree
Hide file tree
Showing 3 changed files with 40 additions and 27 deletions.
26 changes: 11 additions & 15 deletions lib/media/playhead_observer.js
View file
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
View file
Original file line number Diff line number Diff line change
Expand Up @@ -764,7 +764,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
39 changes: 28 additions & 11 deletions test/media/playhead_observer_unit.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,8 @@
describe('PlayheadObserver', function() {
var observer;
var video;
var mockMediaSource;
var mediaSource;
var timeline;
var manifest;
var config;
Expand All @@ -40,6 +42,9 @@ describe('PlayheadObserver', function() {
video.duration = 60;
video.buffered = createFakeBuffered([]);

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

timeline = new shaka.test.FakePresentationTimeline();

// shakaExtern.Manifest
Expand Down Expand Up @@ -68,8 +73,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 @@ -82,7 +88,8 @@ describe('PlayheadObserver', function() {
video.buffered = createFakeBuffered([{start: 0, end: 20}]);
video.currentTime = 0;
observer = new shaka.media.PlayheadObserver(
video, manifest, config, onBuffering, onEvent, onChangePeriod);
video, mediaSource, manifest, config, onBuffering, onEvent,
onChangePeriod);

observer.seeked();
jasmine.clock().tick(1000);
Expand All @@ -93,7 +100,8 @@ describe('PlayheadObserver', function() {
video.buffered = createFakeBuffered([{start: 0, end: 20}]);
video.currentTime = 0;
observer = new shaka.media.PlayheadObserver(
video, manifest, config, onBuffering, onEvent, onChangePeriod);
video, mediaSource, manifest, config, onBuffering, onEvent,
onChangePeriod);

video.currentTime = 20;
jasmine.clock().tick(1000);
Expand All @@ -105,7 +113,8 @@ describe('PlayheadObserver', function() {
video.buffered = createFakeBuffered([{start: 0, end: 20}]);
video.currentTime = 0;
observer = new shaka.media.PlayheadObserver(
video, manifest, config, onBuffering, onEvent, onChangePeriod);
video, mediaSource, manifest, config, onBuffering, onEvent,
onChangePeriod);

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

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

jasmine.clock().tick(1000);
expect(onBuffering).toHaveBeenCalledTimes(1);
Expand All @@ -153,7 +164,8 @@ describe('PlayheadObserver', function() {
video.buffered = createFakeBuffered([]);
video.currentTime = 0;
observer = new shaka.media.PlayheadObserver(
video, manifest, config, onBuffering, onEvent, onChangePeriod);
video, mediaSource, manifest, config, onBuffering, onEvent,
onChangePeriod);

jasmine.clock().tick(1000);
expect(onBuffering).toHaveBeenCalledTimes(1);
Expand All @@ -174,12 +186,14 @@ describe('PlayheadObserver', function() {
timeline.getSegmentAvailabilityEnd.and.returnValue(60);

observer = new shaka.media.PlayheadObserver(
video, manifest, config, onBuffering, onEvent, onChangePeriod);
video, mediaSource, manifest, config, onBuffering, onEvent,
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 @@ -192,7 +206,8 @@ describe('PlayheadObserver', function() {
timeline.getSegmentAvailabilityEnd.and.returnValue(60);

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

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

describe('adding regions', function() {
Expand Down Expand Up @@ -451,7 +467,8 @@ describe('PlayheadObserver', function() {
];

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

// Ignore the call for the initial Period.
jasmine.clock().tick(1000);
Expand Down

0 comments on commit f0e89a7

Please sign in to comment.