Monkeypatch canPlayType on Android 4.0+ for HLS #1084
Conversation
Android devices starting with 4.0 can play HLS natively to some extent. However, they do not say so in the canPlayType method. This commit monkey patches canPlayType to respond with "maybe" for "application/x-mpegURL" and "application/vnd.apple.mpegURL". Otherwise, it'll fallback to the original canPlayType method.
| var canPlayType = HTMLVideoElement.prototype.canPlayType; | ||
|
|
||
| HTMLVideoElement.prototype.canPlayType = function(type) { | ||
| if (type && type === 'application/x-mpegURL' || type === 'application/vnd.apple.mpegURL') { |
There was a problem hiding this comment.
I was called out on this one... I think this should technically be a case-insensitive match.
There was a problem hiding this comment.
Not only that but the official mimetype is all lowercase: http://tools.ietf.org/html/draft-pantos-http-live-streaming-12#section-10
|
Would it make sense to combine this with the following function? Also, want to write a test for this? |
|
Looks like it's a bit tricky to write a test for this as it is right now. |
patchCanPlayType is called on load. It patched video#canPlayType if needed. unpatchCanPlayType will revert the patch and return the patch function. There are also corresponding tests that test patchCanPlayType, unpatchCanPlayType and also whether the patch functions themselves work correctly.
|
Reworked the monkeypatching to use a patch and unpatch methods. This, not only allows us to remove the patching if needed, but it also facilitates better testing. |
|
Double quotes are making the tests fail. |
|
Fixed up jshint errors. |
|
I dont know why it's failing... |
| (function() { | ||
| var canPlayType, | ||
| mpegurlRE = /^application\/(?:x-|vnd\.apple\.)mpegurl$/i, | ||
| mp4RE = /^video\/mp4$/i; |
There was a problem hiding this comment.
I don't think we want the end-of-line character here, in case codec info is included.
| @@ -1,5 +1,7 @@ | |||
| module('HTML5'); | |||
|
|
|||
| var oldAndroidVersion; | |||
| patchCanPlayType = video.canPlayType; | ||
| unpatchedCanPlayType = vjs.Html5.unpatchCanPlayType(); | ||
|
|
||
| strictEqual(video.canPlayType, vjs.TEST_VID.constructor.prototype.canPlayType, 'original canPlayType and unpatched canPlayType should be equal'); |
There was a problem hiding this comment.
Aren't these two references the same the same thing always?
There was a problem hiding this comment.
Yep. I'll see if I can figure out what I meant to refactor this into.
Fix up tests to not ignore some if canPlayType is not available since unpatch is no longer broken.
|
great stuff, thanks! |
| @@ -38,3 +38,59 @@ test('should re-link the player if the tech is moved', function(){ | |||
|
|
|||
| strictEqual(player, tech.el()['player']); | |||
| }); | |||
|
|
|||
| test('patchCanPlayType and unpatchCanPlayType are available on Html5 object', function() { | |||
There was a problem hiding this comment.
@gkatsev are these meant to check that the functions exist after minifying?
There was a problem hiding this comment.
um... yes. Though, I guess would we want to check that it as a string so it doesn't get minified since we want those functions exported?
There was a problem hiding this comment.
Yeah, I typically do that in the api.js file, though I guess a string key would do the same thing
There was a problem hiding this comment.
We could move this test there if you think it would be a better place for them.
|
It doesn't seem to help under android 4.0.3 which plays m3u8 natively very well but videojs throws on me with 'No compatible source was found for this video' |
Android devices starting with 4.0 can play HLS natively to some extent.
However, they do not say so in the canPlayType method. This commit
monkey patches canPlayType to respond with "maybe" for
"application/x-mpegURL" and "application/vnd.apple.mpegURL". Otherwise,
it'll fallback to the original canPlayType method.
Caveat, HLS on android is somewhat broken. In my test streams, it doesn't have a duration so you can't seek.
This fixes videojs/videojs-contrib-hls#29