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

Disable HLS hack on Firefox for Android (with tests) #3586

Merged
merged 5 commits into from
Sep 29, 2016

Conversation

marco-c
Copy link
Contributor

@marco-c marco-c commented Aug 29, 2016

#3498 + tests.


assert.strictEqual(video.canPlayType,
canPlayType,
'original canPlayType and patched canPlayType should not be equal');
Copy link
Member

Choose a reason for hiding this comment

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

Looks like the assertion message needs to be updated.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Leftover from copy & paste of the other test, fixed in b762008.

@misteroneill misteroneill added patch This PR can be added to a patch release. needs: updates labels Aug 29, 2016
@misteroneill
Copy link
Member

Thanks for this, @marco-c!

LGTM

@misteroneill misteroneill added needs: LGTM Needs one or more additional approvals and removed needs: updates labels Aug 29, 2016
@gkatsev
Copy link
Member

gkatsev commented Aug 29, 2016

LGTM. We should merge this in as two separate commits when we get around to it.

@brandonocasey brandonocasey added confirmed and removed needs: LGTM Needs one or more additional approvals labels Aug 30, 2016
@@ -1319,7 +1319,7 @@ const mp4RE = /^video\/mp4/i;

Html5.patchCanPlayType = function() {
// Android 4.0 and above can play HLS to some extent but it reports being unable to do so
if (browser.ANDROID_VERSION >= 4.0) {
if (browser.ANDROID_VERSION >= 4.0 && !browser.IS_FIREFOX) {
Copy link
Member

Choose a reason for hiding this comment

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

I just saw an update to the firefox beta which said they're getting HLS support. Is this something that we should possibly account for? Maybe check if firefox version is less than 50?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The support in Firefox 50 is not polished, there are a lot of bugs (e.g. https://bugzilla.mozilla.org/show_bug.cgi?id=1301050). It will probably be disabled in 50.
We're using a hacky solution, so it will never be like a properly supported codec.
I would leave it as is and expect Firefox not to return wrong values in canPlayType, if it will ever support HLS natively, it will be returned in canPlayType.

Copy link
Member

Choose a reason for hiding this comment

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

I guess we could also always update this at a later time once/if it officially ships.

@gkatsev gkatsev added this to the 5.12 milestone Sep 27, 2016
@gkatsev gkatsev merged commit dd2aff0 into videojs:master Sep 29, 2016
@marco-c marco-c deleted the ff_android branch September 29, 2016 23:06
@curtisgibby
Copy link
Contributor

curtisgibby commented Oct 10, 2016

Any idea when this will land in the core JS that Brightcove serves with its players? (e.g. http://players.brightcove.net/659677170001/default_default/index.js) Right now, that file references Video.js version 5.11.7.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
confirmed patch This PR can be added to a patch release.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants