Fix resuming live playback after long pauses #1006
Conversation
…e, seek back to live point
…fter sync to prevent excessive seeks
src/ranges.js
Outdated
|
|
||
| for (let i = 0; i < range.length; i++) { | ||
| if (i > 0) { | ||
| str += ', '; |
There was a problem hiding this comment.
You can avoid this if you make str and array pushing each start+end and return str.join(','). (It's also better for performance, not that these ranges will ever be large enough for that to matter)
test/playback-watcher.test.js
Outdated
| this.player.tech_.trigger('waiting'); | ||
| assert.equal(seeks.length, 2, 'did not seek'); | ||
|
|
||
| // no check for 0 with seekable false because that should be handled by live falloff |
src/playback-watcher.js
Outdated
| @@ -79,6 +79,12 @@ export default class PlaybackWatcher { | |||
| * @private | |||
| */ | |||
| checkCurrentTime_() { | |||
| if (this.tech_.seeking() && this.fixBadSeeks_()) { | |||
There was a problem hiding this comment.
Can we please, please rename fixBadSeeks_ to fixesBadSeeks_ because that would make me soooo happy.
src/playback-watcher.js
Outdated
| let seekable = this.seekable(); | ||
| let currentTime = this.tech_.currentTime(); | ||
|
|
||
| if (this.tech_.seeking() && |
There was a problem hiding this comment.
It's probably safer to leave this check in here, just thought I'd note that I don't think we can get into this function with seeking is false since the check this.tech_.seeking() && this.fixBadSeeks_() won't call fixBadSeeks_ if seeking returns false
There was a problem hiding this comment.
Yeah, it's a tough one to decide between.
src/playback-watcher.js
Outdated
| @@ -123,6 +154,10 @@ export default class PlaybackWatcher { | |||
| let seekable = this.seekable(); | |||
| let currentTime = this.tech_.currentTime(); | |||
|
|
|||
| if (this.tech_.seeking() && this.fixBadSeeks_()) { | |||
There was a problem hiding this comment.
Why is this check made in both checkCurrentTime_ and waiting_?
There was a problem hiding this comment.
The waiting event can fire and cause waiting_ to run independent of checkCurrentTime_
src/playback-watcher.js
Outdated
| let seekable = this.seekable(); | ||
| let currentTime = this.tech_.currentTime(); | ||
|
|
||
| if (this.tech_.seeking() && |
There was a problem hiding this comment.
Should this function ever run in VOD?
It seems wise to check to see if the duration is "Infinity" before taking any other measures.
There was a problem hiding this comment.
I don't think its bad to have this for VOD. Seekable doesn't change in VOD anyway, so if someone tries seeking outside of the duration of the video, this would just adjust it to 0 or end of video depending on which direction they overseeked
|
LGTM! |
Description
After a live video is paused long enough that the playlist no longer contains any segments previously requested, our seekable range is off and we often can't resume playback. This was for two primary reasons: (1) we were not updating seekable after the playlist updated enough that we no longer had any known segments and (2) if the segment times did not match up perfectly with the specified durations in the playlist, our seekable range would be off and we'd never try to adjust after seeking.
Specific Changes proposed
This seeks to resolve resuming live playback by keeping the seekable range updated, and checking to see if we are outside of a seekable range when we resume (after getting new sync info).
Requirements Checklist