Skip to content
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

fix: fix onProgress callback not firing during iOS live streams #497

Merged
merged 4 commits into from
Nov 16, 2018
Merged

fix: fix onProgress callback not firing during iOS live streams #497

merged 4 commits into from
Nov 16, 2018

Conversation

ecozoic
Copy link
Contributor

@ecozoic ecozoic commented Oct 12, 2018

I noticed that the onProgress callback only gets fired once during live streams on iOS. This is because, on iOS, player.duration returns Infinity for live streams. Since it always returns Infinity, the progress timeout short-circuits and so the callback stops firing altogether.

I have recently had to fix this issue in a few other places, and I have found that using player.seekable.end(0) works great as an alternative. It returns the same thing as player.duration, but works on iOS as well.

In order to minimize the potential impact of this change, I explicitly check for Infinity and only then do I return player.seekable.end(0). Otherwise I continue to return value of the duration property.

Let me know if you need me to me make any updates!

@ecozoic
Copy link
Contributor Author

ecozoic commented Nov 2, 2018

Made some minor updates to improve this fix. While seekable should almost always be contiguous anyway, I now make sure to grab the LAST seekable timeRange and no longer make the assumption that it will always be at 0 index. Also added a bit of defensive programming to make sure we don't attempt to access end when seekable has 0 length (before a video is initialized/has a source for example). This would throw an error.

@cookpete cookpete merged commit 99375a9 into cookpete:master Nov 16, 2018
@cookpete
Copy link
Owner

Nice work! Sorry for the delay on this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants