From 3f94299b2d17888e79a1f6a9d9066bc9fed4e919 Mon Sep 17 00:00:00 2001 From: Rob Walch Date: Tue, 3 Dec 2019 18:47:19 -0500 Subject: [PATCH] Handle stalls when seeking into buffer gaps --- src/controller/gap-controller.ts | 46 ++++++++++++----------- tests/functional/auto/setup.js | 49 ++++++++++++++++++++++--- tests/unit/controller/gap-controller.js | 21 +++-------- 3 files changed, 72 insertions(+), 44 deletions(-) diff --git a/src/controller/gap-controller.ts b/src/controller/gap-controller.ts index d449453a07b..a444300ad07 100644 --- a/src/controller/gap-controller.ts +++ b/src/controller/gap-controller.ts @@ -38,9 +38,9 @@ export default class GapController { */ public poll (lastCurrentTime: number) { const { config, media, stalled } = this; - const tnow = self.performance.now(); const { currentTime, seeking } = media; const seeked = this.seeking && !seeking; + const beginSeek = !this.seeking && seeking; this.seeking = seeking; @@ -50,7 +50,8 @@ export default class GapController { if (stalled !== null) { // The playhead is now moving, but was previously stalled if (this.stallReported) { - logger.warn(`playback not stuck anymore @${currentTime}, after ${Math.round(tnow - stalled)}ms`); + const stalledDuration = self.performance.now() - stalled; + logger.warn(`playback not stuck anymore @${currentTime}, after ${Math.round(stalledDuration)}ms`); this.stallReported = false; } this.stalled = null; @@ -59,8 +60,8 @@ export default class GapController { return; } - // Clear stalled state after seeked so that we don't report stalls coming out of a seek - if (seeked) { + // Clear stalled state when beginning or finishing seeking so that we don't report stalls coming out of a seek + if (beginSeek || seeked) { this.stalled = null; } @@ -79,20 +80,22 @@ export default class GapController { } if (seeking) { - if (isBuffered) { - // Waiting for seeking in a buffered range to complete + // Waiting for seeking in a buffered range to complete + const hasEnoughBuffer = bufferInfo.len > MAX_START_GAP_JUMP; + // Next buffered range is too far ahead to jump to while still seeking + const noBufferGap = !nextStart || nextStart - currentTime > MAX_START_GAP_JUMP; + if (hasEnoughBuffer || noBufferGap) { return; - } else if (nextStart < MAX_START_GAP_JUMP) { - // Reset moved state when seeking back to a point before a start gap - this.moved = false; } + // Reset moved state when seeking to a point in or before a gap + this.moved = false; } // Skip start gaps if we haven't played, but the last poll detected the start of a stall // The addition poll gives the browser a chance to jump the gap for us if (!this.moved && this.stalled) { // Jump start gaps within jump threshold - const startJump = Math.max(nextStart, ((bufferInfo.start || 0) - currentTime)); + const startJump = Math.max(nextStart, bufferInfo.start || 0) - currentTime; if (!seeking && startJump > 0 && startJump <= MAX_START_GAP_JUMP) { this._trySkipBufferHole(null); return; @@ -100,6 +103,7 @@ export default class GapController { } // Start tracking stall time + const tnow = self.performance.now(); if (stalled === null) { this.stalled = tnow; return; @@ -112,7 +116,7 @@ export default class GapController { } const bufferedWithHoles = BufferHelper.bufferInfo(media, currentTime, config.maxBufferHole); - this._tryFixBufferStall(bufferInfo, bufferedWithHoles, stalledDuration); + this._tryFixBufferStall(bufferedWithHoles, stalledDuration); } /** @@ -121,7 +125,7 @@ export default class GapController { * @param stalledDurationMs - The amount of time Hls.js has been stalling for. * @private */ - private _tryFixBufferStall (bufferInfo: BufferInfo, bufferedWithHoles: BufferInfo, stalledDurationMs: number) { + private _tryFixBufferStall (bufferInfo: BufferInfo, stalledDurationMs: number) { const { config, fragmentTracker, media } = this; const currentTime = media.currentTime; @@ -141,16 +145,14 @@ export default class GapController { // we may just have to "nudge" the playlist as the browser decoding/rendering engine // needs to cross some sort of threshold covering all source-buffers content // to start playing properly. - if (stalledDurationMs > config.highBufferWatchdogPeriod * 1000) { - if (bufferedWithHoles.len > config.maxBufferHole || - bufferInfo.len === 0 && bufferInfo.nextStart && bufferInfo.nextStart < config.maxBufferHole) { - logger.warn('Trying to nudge playhead over buffer-hole'); - // Try to nudge currentTime over a buffer hole if we've been stalling for the configured amount of seconds - // We only try to jump the hole if it's under the configured size - // Reset stalled so to rearm watchdog timer - this.stalled = null; - this._tryNudgeBuffer(); - } + if (bufferInfo.len > config.maxBufferHole && + stalledDurationMs > config.highBufferWatchdogPeriod * 1000) { + logger.warn('Trying to nudge playhead over buffer-hole'); + // Try to nudge currentTime over a buffer hole if we've been stalling for the configured amount of seconds + // We only try to jump the hole if it's under the configured size + // Reset stalled so to rearm watchdog timer + this.stalled = null; + this._tryNudgeBuffer(); } } diff --git a/tests/functional/auto/setup.js b/tests/functional/auto/setup.js index 2f7eefe2de7..c2b2184f1bb 100644 --- a/tests/functional/auto/setup.js +++ b/tests/functional/auto/setup.js @@ -89,7 +89,7 @@ async function testLoadedData (url, config) { url, config ); - expect(result.code).to.equal('loadeddata'); + expect(result, JSON.stringify(result, null, 2)).to.have.property('code').which.equals('loadeddata'); } async function testSmoothSwitch (url, config) { @@ -121,7 +121,7 @@ async function testSmoothSwitch (url, config) { url, config ); - expect(result.code).to.be.true; + expect(result, JSON.stringify(result, null, 2)).to.have.property('code').which.equals(true); } async function testSeekOnLive (url, config) { @@ -142,7 +142,7 @@ async function testSeekOnLive (url, config) { url, config ); - expect(result.code).to.equal('seeked'); + expect(result, JSON.stringify(result, null, 2)).to.have.property('code').which.equals('seeked'); } async function testSeekOnVOD (url, config) { @@ -163,7 +163,7 @@ async function testSeekOnVOD (url, config) { url, config ); - expect(result.code).to.equal('ended'); + expect(result, JSON.stringify(result, null, 2)).to.have.property('code').which.equals('ended'); } async function testSeekEndVOD (url, config) { @@ -184,7 +184,7 @@ async function testSeekEndVOD (url, config) { url, config ); - expect(result.code).to.equal('ended'); + expect(result, JSON.stringify(result, null, 2)).to.have.property('code').which.equals('ended'); } async function testIsPlayingVOD (url, config) { @@ -218,7 +218,37 @@ async function testIsPlayingVOD (url, config) { url, config ); - expect(result.playing).to.be.true; + expect(result, JSON.stringify(result, null, 2)).to.have.property('playing').which.is.true; +} + +async function testSeekBackToStart (url, config) { + const result = await browser.executeAsyncScript( + (url, config) => { + const callback = arguments[arguments.length - 1]; + self.startStream(url, config, callback); + const video = self.video; + video.ontimeupdate = function () { + if (video.currentTime > 0 && !video.paused) { + self.setTimeout(function () { + video.onseeked = function () { + delete video.onseeked; + video.ontimeupdate = function () { + if (video.currentTime > 0 && !video.paused) { + delete video.ontimeupdate; + callback({ playing: true }); + } + }; + }; + video.currentTime = 0; + delete video.ontime; + }, 500); + } + }; + }, + url, + config + ); + expect(result, JSON.stringify(result, null, 2)).to.have.property('playing').which.is.true; } describe(`testing hls.js playback in the browser on "${browserDescription}"`, function () { @@ -330,6 +360,13 @@ describe(`testing hls.js playback in the browser on "${browserDescription}"`, fu testLoadedData.bind(null, url, config) ); + if (stream.startSeek) { + it( + `seek back to start and play for ${stream.description}`, + testSeekBackToStart.bind(null, url, config) + ); + } + if (stream.abr) { it( `should "smooth switch" to highest level and still play(readyState === 4) after 12s for ${stream.description}`, diff --git a/tests/unit/controller/gap-controller.js b/tests/unit/controller/gap-controller.js index 8111debdf45..ced0e9e41eb 100644 --- a/tests/unit/controller/gap-controller.js +++ b/tests/unit/controller/gap-controller.js @@ -72,21 +72,10 @@ describe('checkBuffer', function () { describe('_tryFixBufferStall', function () { it('should nudge when stalling close to the buffer end', function () { - const mockBufferInfo = { len: 0.5, nextStart: 1 }; - const mockBufferWithHoles = { len: 1 }; - const mockStallDuration = (config.highBufferWatchdogPeriod + 1) * 1000; - const nudgeStub = sandbox.stub(gapController, '_tryNudgeBuffer'); - gapController._tryFixBufferStall(mockBufferInfo, mockBufferWithHoles, mockStallDuration); - expect(nudgeStub).to.have.been.calledOnce; - }); - - it('should nudge when in between buffered ranges', function () { - media.currentTime = 4; - const mockBufferInfo = { len: 0, nextStart: 4.08 }; - const mockBufferWithHoles = { len: 5 }; + const mockBufferInfo = { len: 1 }; const mockStallDuration = (config.highBufferWatchdogPeriod + 1) * 1000; const nudgeStub = sandbox.stub(gapController, '_tryNudgeBuffer'); - gapController._tryFixBufferStall(mockBufferInfo, mockBufferWithHoles, mockStallDuration); + gapController._tryFixBufferStall(mockBufferInfo, mockStallDuration); expect(nudgeStub).to.have.been.calledOnce; }); @@ -94,15 +83,15 @@ describe('checkBuffer', function () { const mockBufferInfo = { len: 1 }; const mockStallDuration = (config.highBufferWatchdogPeriod / 2) * 1000; const nudgeStub = sandbox.stub(gapController, '_tryNudgeBuffer'); - gapController._tryFixBufferStall(mockBufferInfo, mockBufferInfo, mockStallDuration); + gapController._tryFixBufferStall(mockBufferInfo, mockStallDuration); expect(nudgeStub).to.have.not.been.called; }); - it('should not nudge when too close to the buffer end', function () { + 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 = sandbox.stub(gapController, '_tryNudgeBuffer'); - gapController._tryFixBufferStall(mockBufferInfo, mockBufferInfo, mockStallDuration); + gapController._tryFixBufferStall(mockBufferInfo, mockStallDuration); expect(nudgeStub).to.have.not.been.called; });