Add forceSafariHLS option for FilePlayer

[nabeards]

I have a need for the Safari native HLS player to load on macOS so I've added an option to do so.

The reason for this is because hls.js loads the <video> element with a src that's a blob. This breaks certain <canvas> features when working with the <video> DOM element on Safari 15.x.

Question: is there a way to locally install this as an NPM module for local development? Other modules I've worked with have been able to work that way, but trying with react-player doesn't work that way.

[cookpete]

Updated prop name to forceSafariHLS – "apple" is a bit ambiguous, and now the prop is consistent with the IS_SAFARI matching logic.

Also tidied the logic a bit to match the forceHLS logic.

Nice work!

[nabeards]

Thanks @cookpete !

[nabeards]

@cookpete Was about to integrate the changes, but I just realized that I think you reversed the logic here. The idea is to force the native HLS player on macOS Safari. Instead, you've made it force the hls.js player.

[nabeards]

@cookpete just following up on this message for feedback. Should I submit a new PR for my use case?

[lvnam96]

@nabeards If you submit new PR, I think you should name the prop to forceNativeHls then have your own isSafariOnMacOS boolean passing to that prop, & react-player should not init hls.js instance if the boolean is truthy.

@cookpete is this prop forceSafariHLS a bit redundant? As I understand, shouldUseHls() will only skip iOS devices. I don't know why we need this prop to bypass HLS extension check at the end of this method.

[nabeards]

@lvnam96 my original name was forceSafariHls but forceNativeHls is good as well. Would love to hear from @cookpete if they are open to this change. 🙂

[cookpete]

@cookpete Was about to integrate the changes, but I just realized that I think you reversed the logic here. The idea is to force the native HLS player on macOS Safari. Instead, you've made it force the hls.js player.

@nabeards Yep, you’re right. I misinterpreted forceAppleHLS to mean "force Apple desktop (Safari) to use hls.js". My bad! forceNativeHLS is much more clear.

@cookpete is this prop forceSafariHLS a bit redundant? As I understand, shouldUseHls() will only skip iOS devices. I don't know why we need this prop to bypass HLS extension check at the end of this method.

@lvnam96 I think you’re right. The question is – would removing it be considered a breaking change that requires a major version bump? I am tempted to say no for quality of life, but happy to hear opinions.