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

README: Swap logic for checking Hls.isSupported() #2954

Merged
merged 2 commits into from
Aug 7, 2020

Conversation

dylanjha
Copy link
Contributor

@dylanjha dylanjha commented Aug 6, 2020

This PR will...

Update README for checking browser HLS support.

Why is this Pull Request needed?

Before Safari supported MSE then this recommendation in the README worked well because Hls.isSupported() would return false. Now that changed and it returns true.

I typically think it's a best practice to use the browser's native HLS support if it's available, otherwise fallback to hls.js

Checklist

n/a - only a README update

Before Safari supported MSE then this recommendation worked well because `Hls.isSupported()` would return `false`. Now that changed and it returns `true`. I typically think it's a best practice to use the browser's native HLS support if it's available, otherwise fallback to hls.js.
@robwalch
Copy link
Collaborator

robwalch commented Aug 6, 2020

I typically think it's a best practice to use the browser's native HLS support if it's available, otherwise fallback to hls.js

I agree (and do the same in JW Player), but not everyone thinks that way. Some folks want to use hls.js's API whenever possible to have access to:

  • manual quality selection
  • additional access to metadata and manifest tag attributes

There may be other reasons. On the down side, handling HLS streams with JavaScript is probably less power and memory efficient. There may also be places where the player is less stable than Safari (or vice versa). The latest release, v0.14.7, includes a lot of improvements for Safari playback on Desktop and iPad where MSE is available, and I would still like to encourage users to try it out and give us their feedback.

Keep the same "Getting Started" example, but add a second "Alternative Setup"
@dylanjha
Copy link
Contributor Author

dylanjha commented Aug 6, 2020

Thanks for considering @robwalch, totally makes sense. The only reason I opened this is because I've seen many folks mix this up (myself included 😁 ).

I updated the README to:

  • keep that code sample the same
  • add a second code sample titled Alternative setup which swaps the logic and explains a bit more.

Feel free to take it or leave it, I can also understand if you don't want to clutter up the "Getting started" and risk making it too confusing for folks that are seeing it for the first time. Whatever you think is best!

@robwalch
Copy link
Collaborator

robwalch commented Aug 6, 2020

Hi @dylanjha,

This is great! Thank you for improving the README and linking the comment above.

@robwalch robwalch merged commit 822da0c into video-dev:master Aug 7, 2020
@dylanjha dylanjha deleted the patch-1 branch April 29, 2021 15:22
Joel2B added a commit to Joel2B/Custom-Video-Player that referenced this pull request Jul 31, 2021
If the browser has native HLS support, use it, more info here
video-dev/hls.js#2954
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.

2 participants