-
Notifications
You must be signed in to change notification settings - Fork 793
Seek to seekable start when before the seekable window #1260
Seek to seekable start when before the seekable window #1260
Conversation
src/playback-watcher.js
Outdated
let seekableEnd = seekable.end(seekable.length - 1); | ||
|
||
// sync to live point (if VOD, our seekable was updated and we're simply adjusting) | ||
this.logger_(`Trying to seek outside of seekable at time ${currentTime} with ` + |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Since the much of this is the same, maybe just set a seekTime variable and do the log/seek at the end if it was set
let seekTime = NaN;
if (after) seekTime = seekable.end;
if (before) seekTime = seekable.start + 0.1;
if (!isNaN(seekTime)) log; setCurrentTime(seekTime);
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yep will do
Github won't let me comment on the line because no changes were made, but line 468 of playback-watcher.test.js test description is |
Reworded test
src/playback-watcher.js
Outdated
let currentTime = this.tech_.currentTime(); | ||
const seeking = this.tech_.seeking(); | ||
const seekable = this.seekable(); | ||
const seekableStart = seekable.start(0); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
seekable
can have a length of 0, in which case this will error. Both beforeSeekableWindow_
and afterSeekableWindow_
check the length and return false if it is 0, so you could not create these const here and just access seekable.start(0)
if you get into the appropriate if block. Otherwise you will have to check the length here as well
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'll move these down to their appropriate if blocks
src/playback-watcher.js
Outdated
this.outsideOfSeekableWindow_(seekable, currentTime)) { | ||
let seekableEnd = seekable.end(seekable.length - 1); | ||
if (seeking && this.afterSeekableWindow_(seekable, currentTime)) { | ||
const seekableStart = seekable.start(0); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
you switched up seekableStart
and seekableEnd
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
doh
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
At least the tests caught it
8e38b50
to
89cf1ca
Compare
Description
Seek to seekable start when before the seekable window
Specific Changes proposed
Instead of seeking to live when doing a bad seek (for example if the seekable shifts while scrubbing) we'll snap to the beginning of the live window.
Requirements Checklist