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

HLS support on JS (closes #555) #583

Merged
merged 26 commits into from
Oct 14, 2015
Merged

HLS support on JS (closes #555) #583

merged 26 commits into from
Oct 14, 2015

Conversation

flavioribeiro
Copy link
Member

refs #561, hound messed up the PR with tons of alerts

}
}

HLS.canPlay = function(resource, mimeType) {
HLS.canPlay = function(resource) {

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

'HLS' is not defined.

@flavioribeiro
Copy link
Member Author

can anyone review this PR before we merge it? I made some smoke tests using this html and it seems to be working fine.

@jfairley
Copy link
Contributor

@flavioribeiro, unless someone else gives the thumbs up sooner, @dviramontes or I will pull it down and give it a test drive tomorrow at the office.

@flavioribeiro
Copy link
Member Author

great @jfairley feel free to do it!

@jfairley
Copy link
Contributor

Here's the config I'm running, modified from the default test page. You can't see our .m3u8, but there's nothing special about it.

var player = new Clappr.Player({
  source: 'http://10.0.1.69/media/hlsLiveTest01/stream/hls.m3u8',
  plugins: { playback: [Clappr.HLS] },
  baseUrl: '/latest',
  poster: 'http://clappr.io/poster.png',
  mute: true,
  height: 360,
  width: 640
});

Chrome (47.0.2526.8 dev)

Everything looks great in Chrome, so it's definitely the test bed.

In live mode, the play head location is a little funky, but knowing how HLS works, I completely understand why it looks they way it does. I'm happy to see that we're getting accurate values for duration and currentTime. 👍 We both need to do some tweaking in our seek bars to make things look prettier.

Opera (32.0)

I see the link to #558 about this not working in Opera. For me in Opera, the player looks every bit as good as it does in Chrome.

I did see one glitch in frame stepping, but it seems like the buffer wasn't loaded. When I quickly played/paused, I could step. ( I did the frame stepping in our app, not the demo page. 😉 )

Firefox (42.0b5)

It's funky in Firefox, but I would expect so, since FF doesn't support HLS. I was surprised that it plays and the audio is fine. It's only the video frame that FF fails to render correctly. NBD. Use Flash in FF.

About that... I tried declaring both playbacks, but it still tried to use HLS. I suppose we'll need to determine the browser at runtime and apply the correct plugins as necessary. Is that what you would expect?

// does not work :(
plugins: { playback: [ Clappr.HLS, Clappr.FlasHLS ] }

// what I'll do instead
let playback = isFirefox ? Clappr.FlasHLS : Clappr.HLS
plugins: { playback: [ playback ] }

Safari (9.0)

In Safari, when I declare the HLS plugin, my CPU pegs at 100%, and I get the spinning beach ball (OSX). Without the plugin (defaulting to native), everything runs fine. Of course with no plugin, the live mode currentTime / duration are still 1 / ∞ (or vice versa, I don't remember).

In fact, something nice that I found is that when no plugin is applied, Clappr correctly decides to use native in Safari, while it correctly decides to use HLS when in Chrome and Opera. 👏 Unfortunately, FF also tries to use HLS, so I'll need to keep my switch to force FF to use FlasHLS.

probably...

var config = {
  source: 'my-source.m3u8',
  plugins: {},
  // other settings
}
if (isFirefox) {
  config.plugins.playback = Clappr.FlasHLS
}
var player = new Clappr.Player(config);

@flavioribeiro
Copy link
Member Author

wow @jfairley thanks for all the tests! I guess that the way to go is avoid hlsjs in ffox and safari for now.

@dviramontes
Copy link

Nice testing indeed! 👍

@leandromoreira
Copy link
Member

thanks @flavioribeiro 👏 it's a great milestone for Clappr

towerz added a commit that referenced this pull request Oct 14, 2015
@towerz towerz merged commit 144d2cd into clappr:master Oct 14, 2015
@towerz
Copy link
Member

towerz commented Oct 14, 2015

Thanks @flavioribeiro, excelent work!!! 🍻

@jfairley
Copy link
Contributor

woo hoo 🍻

@mangui
Copy link

mangui commented Oct 14, 2015

🍻 !!!

@dviramontes
Copy link

🤘

@leandromoreira
Copy link
Member

  • Firefox 42 will enable MSE by default
  • There is no whitelist anymore ; we won't be adding new sites to the whitelist

http://www.ghacks.net/2015/08/06/mozilla-to-enable-media-source-extensions-mse-for-all-websites-in-firefox-42/

https://bugzilla.mozilla.org/show_bug.cgi?id=778617

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.

None yet

7 participants