Skip to content
This repository has been archived by the owner on Jan 12, 2019. It is now read-only.

Commit

Permalink
Fix 'ended' event not firing after replay (#842)
Browse files Browse the repository at this point in the history 
* add eos check in checkbuffer

* don't rely on 'this' in checkBuffer

* make use of detectEndOfStream in fill buffer
  • Loading branch information
@mjneil @forbesjo
mjneil authored and forbesjo committed Oct 18, 2016
1 parent d7fb9c1 commit 49a03b0
Show file tree
Hide file tree
Showing 2 changed files with 105 additions and 71 deletions.
145 changes: 81 additions & 64 deletions src/segment-loader.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -54,10 +54,9 @@ const updateSegmentMetadata = function(playlist, segmentIndex, segmentEnd) {
* @param {Object} playlist a media playlist object
* @param {Object} mediaSource the MediaSource object
* @param {Number} segmentIndex the index of segment we last appended
* @param {Object} currentBuffered buffered region that currentTime resides in
* @returns {Boolean} do we need to call endOfStream on the MediaSource
*/
const detectEndOfStream = function(playlist, mediaSource, segmentIndex, currentBuffered) {
const detectEndOfStream = function(playlist, mediaSource, segmentIndex) {
if (!playlist) {
return false;
}
Expand All @@ -66,16 +65,14 @@ const detectEndOfStream = function(playlist, mediaSource, segmentIndex, currentB

// determine a few boolean values to help make the branch below easier
// to read
let appendedLastSegment = (segmentIndex === segments.length - 1);
let bufferedToEnd = (currentBuffered.length &&
segments[segments.length - 1].end <= currentBuffered.end(0));
let appendedLastSegment = segmentIndex === segments.length;

// if we've buffered to the end of the video, we need to call endOfStream
// so that MediaSources can trigger the `ended` event when it runs out of
// buffered data instead of waiting for me
return playlist.endList &&
mediaSource.readyState === 'open' &&
(appendedLastSegment || bufferedToEnd);
appendedLastSegment;
};

/**
Expand Down Expand Up @@ -363,10 +360,14 @@ export default class SegmentLoader extends videojs.EventTarget {
* @param {TimeRanges} buffered - the state of the buffer
* @param {Object} playlist - the playlist object to fetch segments from
* @param {Number} currentTime - the playback position in seconds
* @param {Boolean} hasPlayed - if the player has played before
* @param {Number} expired - the seconds expired off the playlist
* @param {Number} timeCorrection - correction value to add to current time when
* when determining media index to use
* @returns {Object} a segment info object that describes the
* request that should be made or null if no request is necessary
*/
checkBuffer_(buffered, playlist, currentTime) {
checkBuffer_(buffered, playlist, currentTime, hasPlayed, expired, timeCorrection) {
let currentBuffered = Ranges.findRange(buffered, currentTime);

// There are times when MSE reports the first segment as starting a
Expand All @@ -379,18 +380,17 @@ export default class SegmentLoader extends videojs.EventTarget {

let bufferedTime;
let currentBufferedEnd;
let segment;
let mediaIndex;

if (!playlist.segments.length) {
return;
return null;
}

if (currentBuffered.length === 0) {
// find the segment containing currentTime
mediaIndex = getMediaIndexForTime(playlist,
currentTime + this.timeCorrection_,
this.expired_);
currentTime + timeCorrection,
expired);
} else {
// find the segment adjacent to the end of the current
// buffered region
Expand All @@ -399,48 +399,21 @@ export default class SegmentLoader extends videojs.EventTarget {

// if the video has not yet played only, and we already have
// one segment downloaded do nothing
if (!this.hasPlayed_() && bufferedTime >= 1) {
if (!hasPlayed && bufferedTime >= 1) {
return null;
}

// if there is plenty of content buffered, and the video has
// been played before relax for awhile
if (this.hasPlayed_() && bufferedTime >= Config.GOAL_BUFFER_LENGTH) {
if (hasPlayed && bufferedTime >= Config.GOAL_BUFFER_LENGTH) {
return null;
}
mediaIndex = getMediaIndexForTime(playlist,
currentBufferedEnd + this.timeCorrection_,
this.expired_);
}

if (mediaIndex < 0 || mediaIndex === playlist.segments.length) {
return null;
currentBufferedEnd + timeCorrection,
expired);
}

segment = playlist.segments[mediaIndex];
return {
// resolve the segment URL relative to the playlist
uri: segment.resolvedUri,
// the segment's mediaIndex at the time it was requested
mediaIndex,
// the segment's playlist
playlist,
// unencrypted bytes of the segment
bytes: null,
// when a key is defined for this segment, the encrypted bytes
encryptedBytes: null,
// the state of the buffer before a segment is appended will be
// stored here so that the actual segment duration can be
// determined after it has been appended
buffered: null,
// The target timestampOffset for this segment when we append it
// to the source buffer
timestampOffset: NaN,
// The timeline that the segment is in
timeline: segment.timeline,
// The expected duration of the segment in seconds
duration: segment.duration
};
return mediaIndex;
}

/**
Expand Down Expand Up @@ -480,36 +453,81 @@ export default class SegmentLoader extends videojs.EventTarget {
return;
}

let buffered = this.sourceUpdater_.buffered();
let playlist = this.playlist_;
let currentTime = this.currentTime_();
let hasPlayed = this.hasPlayed_();
let expired = this.expired_;
let timeCorrection = this.timeCorrection_;

// see if we need to begin loading immediately
let segmentInfo = this.checkBuffer_(this.sourceUpdater_.buffered(),
this.playlist_,
this.currentTime_());
let requestIndex = this.checkBuffer_(buffered,
playlist,
currentTime,
hasPlayed,
expired,
timeCorrection);

if (requestIndex === null) {
return;
}

if (!segmentInfo) {
let isEndOfStream = detectEndOfStream(playlist,
this.mediaSource_,
requestIndex);

if (isEndOfStream) {
this.mediaSource_.endOfStream();
return;
}

if (segmentInfo.mediaIndex === this.playlist_.segments.length - 1 &&
if (requestIndex === playlist.segments.length - 1 &&
this.mediaSource_.readyState === 'ended' &&
!this.seeking_()) {
return;
}

let segment = this.playlist_.segments[segmentInfo.mediaIndex];
let startOfSegment = duration(this.playlist_,
this.playlist_.mediaSequence + segmentInfo.mediaIndex,
this.expired_);
if (requestIndex < 0 || requestIndex >= playlist.segments.length) {
return;
}

let segment = this.playlist_.segments[requestIndex];

let request = {
// resolve the segment URL relative to the playlist
uri: segment.resolvedUri,
// the segment's mediaIndex at the time it was requested
mediaIndex: requestIndex,
// the segment's playlist
playlist,
// unencrypted bytes of the segment
bytes: null,
// when a key is defined for this segment, the encrypted bytes
encryptedBytes: null,
// the state of the buffer before a segment is appended will be
// stored here so that the actual segment duration can be
// determined after it has been appended
buffered: null,
// The target timestampOffset for this segment when we append it
// to the source buffer
timestampOffset: NaN,
// The timeline that the segment is in
timeline: segment.timeline,
// The expected duration of the segment in seconds
duration: segment.duration
};

let startOfSegment = duration(playlist,
playlist.mediaSequence + request.mediaIndex,
expired);

// We will need to change timestampOffset of the sourceBuffer if either of
// the following conditions are true:
// - The segment.timeline !== this.currentTimeline
// (we are crossing a discontinuity)
// (we are crossing a discontinuity somehow)
// - The "timestampOffset" for the start of this segment is less than
// the currently set timestampOffset
segmentInfo.timestampOffset = this.sourceUpdater_.timestampOffset();
request.timestampOffset = this.sourceUpdater_.timestampOffset();
if (segment.timeline !== this.currentTimeline_ ||
startOfSegment < this.sourceUpdater_.timestampOffset()) {
segmentInfo.timestampOffset = startOfSegment;
request.timestampOffset = startOfSegment;
}

// Sanity check the segment-index determining logic by calcuating the
Expand All @@ -518,15 +536,15 @@ export default class SegmentLoader extends videojs.EventTarget {
// any way
let percentBuffered = Ranges.getSegmentBufferedPercent(startOfSegment,
segment.duration,
this.currentTime_(),
this.sourceUpdater_.buffered());
currentTime,
buffered);

if (percentBuffered >= 90) {
// Increment the timeCorrection_ variable to push the fetcher forward
// in time and hopefully skip any gaps or flaws in our understanding
// of the media
let correctionApplied =
this.incrementTimeCorrection_(this.playlist_.targetDuration / 2, 1);
this.incrementTimeCorrection_(playlist.targetDuration / 2, 1);

if (correctionApplied && !this.paused()) {
this.fillBuffer_();
Expand All @@ -535,7 +553,7 @@ export default class SegmentLoader extends videojs.EventTarget {
return;
}

this.loadSegment_(segmentInfo);
this.loadSegment_(request);
}

/**
Expand Down Expand Up @@ -916,8 +934,7 @@ export default class SegmentLoader extends videojs.EventTarget {
// fire if playback reaches that point.
let isEndOfStream = detectEndOfStream(segmentInfo.playlist,
this.mediaSource_,
currentMediaIndex,
currentBuffered);
currentMediaIndex + 1);

if (isEndOfStream) {
this.mediaSource_.endOfStream();
Expand Down
31 changes: 24 additions & 7 deletions test/segment-loader.test.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -736,6 +736,7 @@ QUnit.test('fires ended at the end of a playlist', function() {
sourceBuffers: mediaSource.sourceBuffers,
endOfStream() {
endOfStreams++;
this.readyState = 'ended';
}
};

Expand Down Expand Up @@ -1072,9 +1073,14 @@ QUnit.module('Segment Loading Calculation', {
QUnit.test('requests the first segment with an empty buffer', function() {
loader.mimeType(this.mimeType);

let segmentInfo = loader.checkBuffer_(videojs.createTimeRanges(),
playlistWithDuration(20),
let playlist = playlistWithDuration(20);
let segmentIndex = loader.checkBuffer_(videojs.createTimeRanges(),
playlist,
0,
false,
0,
0);
let segmentInfo = playlist.segments[segmentIndex];

QUnit.ok(segmentInfo, 'generated a request');
QUnit.equal(segmentInfo.uri, '0.ts', 'requested the first segment');
Expand Down Expand Up @@ -1109,14 +1115,16 @@ QUnit.test('does not download the next segment if the buffer is full', function(
QUnit.test('downloads the next segment if the buffer is getting low', function() {
let buffered;
let segmentInfo;
let segmentIndex;
let playlist = playlistWithDuration(30);

loader.mimeType(this.mimeType);
loader.playlist(playlist);

playlist.segments[1].end = 19.999;
buffered = videojs.createTimeRanges([[0, 19.999]]);
segmentInfo = loader.checkBuffer_(buffered, playlist, 15);
segmentIndex = loader.checkBuffer_(buffered, playlist, 15, true, 0, 0);
segmentInfo = playlist.segments[segmentIndex];

QUnit.ok(segmentInfo, 'made a request');
QUnit.equal(segmentInfo.uri, '2.ts', 'requested the third segment');
Expand All @@ -1125,16 +1133,20 @@ QUnit.test('downloads the next segment if the buffer is getting low', function()
QUnit.test('buffers based on the correct TimeRange if multiple ranges exist', function() {
let buffered;
let segmentInfo;
let segmentIndex;
let playlist = playlistWithDuration(40);

loader.mimeType(this.mimeType);

buffered = videojs.createTimeRanges([[0, 10], [20, 30]]);
segmentInfo = loader.checkBuffer_(buffered, playlistWithDuration(40), 8);
segmentIndex = loader.checkBuffer_(buffered, playlist, 8, true, 0, 0);
segmentInfo = playlist.segments[segmentIndex];

QUnit.ok(segmentInfo, 'made a request');
QUnit.equal(segmentInfo.uri, '1.ts', 'requested the second segment');

segmentInfo = loader.checkBuffer_(buffered, playlistWithDuration(40), 20);
segmentIndex = loader.checkBuffer_(buffered, playlist, 20, true, 0, 0);
segmentInfo = playlist.segments[segmentIndex];
QUnit.ok(segmentInfo, 'made a request');
QUnit.equal(segmentInfo.uri, '3.ts', 'requested the fourth segment');
});
Expand Down Expand Up @@ -1171,6 +1183,7 @@ QUnit.test('adjusts calculations based on expired time', function() {
let buffered;
let playlist;
let segmentInfo;
let segmentIndex;

loader.mimeType(this.mimeType);

Expand All @@ -1179,9 +1192,13 @@ QUnit.test('adjusts calculations based on expired time', function() {

loader.expired(10);

segmentInfo = loader.checkBuffer_(buffered,
segmentIndex = loader.checkBuffer_(buffered,
playlist,
40 - Config.GOAL_BUFFER_LENGTH);
40 - Config.GOAL_BUFFER_LENGTH,
true,
loader.expired_,
0);
segmentInfo = playlist.segments[segmentIndex];

QUnit.ok(segmentInfo, 'fetched a segment');
QUnit.equal(segmentInfo.uri, '2.ts', 'accounted for expired time');
Expand Down

0 comments on commit 49a03b0

Please sign in to comment.