-
Notifications
You must be signed in to change notification settings - Fork 256
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Race canplay event with a setTimeout. #136
Conversation
In some browsers, like firefox with MSE, canplay doesn't fire at the correct time. So, we need to race it against a setTimeout so that we would actually try to resume if canplay doesn't fire correctly. In the tryToResume function we then clear out the event handler and the timeout depending on which came back first.
player.one('contentcanplay', tryToResume); | ||
player.ads.tryToResumeTimeout = player.setTimeout(tryToResume, 2000); |
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 used 2 seconds here. I'm not sure what this should actually be.
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.
Probably should suffix this with an underscore so it's obvious it's an internal variable.
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.
Makes sense.
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.
Fixed.
Also, that they clear each other out.
Add a test to make sure initial setTimeout for race is called.
One comment about variable naming but otherwise LGTM |
Race canplay event with a setTimeout.
In some browsers, like firefox with MSE, canplay doesn't fire at the
correct time. So, we need to race it against a setTimeout so that we
would actually try to resume if canplay doesn't fire correctly.
In the tryToResume function we then clear out the event handler and the
timeout depending on which came back first.
Also, see videojs/videojs-contrib-hls#469