Skip to content

Commit

Permalink
Prevent excessive bufferStall errors through a 1 second debounce on r…
Browse files Browse the repository at this point in the history 
…eporting stalls (video-dev#1678)

Triggers bufferStall if the playhead has not moved for >1 second (and we're expecting it to be moving).
  • Loading branch information
@johnBartos
johnBartos authored May 15, 2018
1 parent b6a5746 commit 7a7a739
Show file tree
Hide file tree
Showing 2 changed files with 41 additions and 28 deletions.
9 changes: 6 additions & 3 deletions src/controller/stream-controller.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1358,6 +1358,7 @@ class StreamController extends TaskLoop {
*/
_checkBuffer () {
const { config, media } = this;
const stallDebounceInterval = 1000;
if (!media || media.readyState === 0) {
// Exit early if we don't have media or if the media hasn't bufferd anything yet (readyState 0)
return;
Expand Down Expand Up @@ -1389,13 +1390,16 @@ class StreamController extends TaskLoop {
} else if (expectedPlaying) {
// The playhead isn't moving but it should be
// Allow some slack time to for small stalls to resolve themselves
const stalledDuration = tnow - this.stalled;
const bufferInfo = BufferHelper.bufferInfo(media, currentTime, config.maxBufferHole);
if (!this.stalled) {
this.stalled = tnow;
return;
} else if (stalledDuration >= stallDebounceInterval) {
// Report stalling after trying to fix
this._reportStall(bufferInfo.len);
}

const bufferInfo = BufferHelper.bufferInfo(media, currentTime, config.maxBufferHole);
const stalledDuration = tnow - this.stalled;
this._tryFixBufferStall(bufferInfo, stalledDuration);
}
}
Expand Down Expand Up @@ -1447,7 +1451,6 @@ class StreamController extends TaskLoop {
const currentTime = media.currentTime;
const jumpThreshold = 0.5; // tolerance needed as some browsers stalls playback before reaching buffered range end

this._reportStall(bufferInfo.len);
const partial = this.fragmentTracker.getPartialFragment(currentTime);
if (partial) {
// Try to skip over the buffer hole caused by a partial fragment
Expand Down
60 changes: 35 additions & 25 deletions tests/unit/controller/check-buffer.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,8 @@ describe('checkBuffer', function () {
let config;
let media;
let triggerSpy;
const sandbox = sinon.sandbox.create();

beforeEach(function () {
media = document.createElement('video');
const hls = new Hls({});
Expand All @@ -21,6 +23,10 @@ describe('checkBuffer', function () {
triggerSpy = sinon.spy(hls, 'trigger');
});

afterEach(function () {
sandbox.restore();
});

describe('_tryNudgeBuffer', function () {
it('should increment the currentTime by a multiple of nudgeRetry and the configured nudge amount', function () {
for (let i = 1; i < config.nudgeMaxRetry; i++) {
Expand Down Expand Up @@ -66,52 +72,42 @@ describe('checkBuffer', function () {
});

describe('_tryFixBufferStall', function () {
let reportStallSpy;
beforeEach(function () {
reportStallSpy = sinon.spy(streamController, '_reportStall');
});

it('should nudge when stalling close to the buffer end', function () {
const mockBufferInfo = { len: 1 };
const mockStallDuration = (config.highBufferWatchdogPeriod + 1) * 1000;
const nudgeStub = sinon.stub(streamController, '_tryNudgeBuffer');
const nudgeStub = sandbox.stub(streamController, '_tryNudgeBuffer');
streamController._tryFixBufferStall(mockBufferInfo, mockStallDuration);
assert(nudgeStub.calledOnce);
assert(reportStallSpy.calledOnce);
});

it('should not nudge when briefly stalling close to the buffer end', function () {
const mockBufferInfo = { len: 1 };
const mockStallDuration = (config.highBufferWatchdogPeriod / 2) * 1000;
const nudgeStub = sinon.stub(streamController, '_tryNudgeBuffer');
const nudgeStub = sandbox.stub(streamController, '_tryNudgeBuffer');
streamController._tryFixBufferStall(mockBufferInfo, mockStallDuration);
assert(nudgeStub.notCalled);
assert(reportStallSpy.calledOnce);
});

it('should not nudge when too far from the buffer end', function () {
const mockBufferInfo = { len: 0.25 };
const mockStallDuration = (config.highBufferWatchdogPeriod + 1) * 1000;
const nudgeStub = sinon.stub(streamController, '_tryNudgeBuffer');
const nudgeStub = sandbox.stub(streamController, '_tryNudgeBuffer');
streamController._tryFixBufferStall(mockBufferInfo, mockStallDuration);
assert(nudgeStub.notCalled);
assert(reportStallSpy.calledOnce);
});

it('should try to jump partial fragments when detected', function () {
sinon.stub(streamController.fragmentTracker, 'getPartialFragment').returns({});
const skipHoleStub = sinon.stub(streamController, '_trySkipBufferHole');
sandbox.stub(streamController.fragmentTracker, 'getPartialFragment').returns({});
const skipHoleStub = sandbox.stub(streamController, '_trySkipBufferHole');
streamController._tryFixBufferStall({ len: 0 });
assert(skipHoleStub.calledOnce);
assert(reportStallSpy.calledOnce);
});

it('should not try to jump partial fragments when none are detected', function () {
sinon.stub(streamController.fragmentTracker, 'getPartialFragment').returns(null);
const skipHoleStub = sinon.stub(streamController, '_trySkipBufferHole');
sandbox.stub(streamController.fragmentTracker, 'getPartialFragment').returns(null);
const skipHoleStub = sandbox.stub(streamController, '_trySkipBufferHole');
streamController._tryFixBufferStall({ len: 0 });
assert(skipHoleStub.notCalled);
assert(reportStallSpy.calledOnce);
});
});

Expand All @@ -132,6 +128,7 @@ describe('checkBuffer', function () {

describe('_checkBuffer', function () {
let mockMedia;
let reportStallSpy;
beforeEach(function () {
mockMedia = {
readyState: 1,
Expand All @@ -140,9 +137,12 @@ describe('checkBuffer', function () {
}
};
streamController.media = mockMedia;
reportStallSpy = sandbox.spy(streamController, '_reportStall');
});

function setExpectedPlaying () {
streamController.loadedmetadata = true;
streamController.immediateSwitch = false;
mockMedia.paused = false;
mockMedia.readyState = 4;
mockMedia.currentTime = 4;
Expand All @@ -155,40 +155,38 @@ describe('checkBuffer', function () {
});

it('should seek to start pos when metadata has not yet been loaded', function () {
const seekStub = sinon.stub(streamController, '_seekToStartPos');
const seekStub = sandbox.stub(streamController, '_seekToStartPos');
streamController._checkBuffer();
assert(seekStub.calledOnce);
assert(streamController.loadedmetadata);
});

it('should not seek to start pos when metadata has been loaded', function () {
const seekStub = sinon.stub(streamController, '_seekToStartPos');
const seekStub = sandbox.stub(streamController, '_seekToStartPos');
streamController.loadedmetadata = true;
streamController._checkBuffer();
assert(seekStub.notCalled);
assert(streamController.loadedmetadata);
});

it('should not seek to start pos when nothing has been buffered', function () {
const seekStub = sinon.stub(streamController, '_seekToStartPos');
const seekStub = sandbox.stub(streamController, '_seekToStartPos');
mockMedia.buffered.length = 0;
streamController._checkBuffer();
assert(seekStub.notCalled);
assert.strictEqual(streamController.loadedmetadata, undefined);
});

it('should complete the immediate switch if signalled', function () {
const levelSwitchStub = sinon.stub(streamController, 'immediateLevelSwitchEnd');
const levelSwitchStub = sandbox.stub(streamController, 'immediateLevelSwitchEnd');
streamController.loadedmetadata = true;
streamController.immediateSwitch = true;
streamController._checkBuffer();
assert(levelSwitchStub.called);
});

it('should try to fix a stall if expected to be playing', function () {
streamController.loadedmetadata = true;
streamController.immediateSwitch = false;
const fixStallStub = sinon.stub(streamController, '_tryFixBufferStall');
const fixStallStub = sandbox.stub(streamController, '_tryFixBufferStall');
setExpectedPlaying();
streamController._checkBuffer();

Expand All @@ -206,13 +204,25 @@ describe('checkBuffer', function () {
streamController.nudgeRetry = 1;
streamController.stalled = 4200;
streamController.lastCurrentTime = 1;
const fixStallStub = sinon.stub(streamController, '_tryFixBufferStall');
const fixStallStub = sandbox.stub(streamController, '_tryFixBufferStall');
streamController._checkBuffer();

assert.strictEqual(streamController.stalled, null);
assert.strictEqual(streamController.nudgeRetry, 0);
assert.strictEqual(streamController.stallReported, false);
assert(fixStallStub.notCalled);
});

it('should trigger reportStall when stalling for 1 second or longer', function () {
setExpectedPlaying();
const clock = sandbox.useFakeTimers(0);
clock.tick(1000);
streamController.stalled = 1;
streamController._checkBuffer();
assert(reportStallSpy.notCalled);
clock.tick(1001);
streamController._checkBuffer();
assert(reportStallSpy.calledOnce);
});
});
});

0 comments on commit 7a7a739

Please sign in to comment.