Resolution switching between 'n' number of streams #233

Closed
wants to merge 38 commits into
from

Conversation

Projects
None yet
6 participants
Contributor

onyxrev commented Oct 18, 2012

I'm a freelance developer hired by Vidcaster to implement resolution switching ala YouTube in VideoJS. Here's my contribution:

Here's how to specify multiple streams:

<video id="vid1" class="video-js vjs-default-skin" controls preload="auto" width="640" height="264" poster="http://video-js.zencoder.com/oceans-clip.png" data-setup='{}'> <source src="http://video-js.zencoder.com/oceans-clip_hd.mp4" type='video/mp4' res="HD"> <source src="http://video-js.zencoder.com/oceans-clip_sd.mp4" type='video/mp4' res="SD" default="true"> <source src="http://video-js.zencoder.com/oceans-clip.webm" type='video/webm'> <source src="http://video-js.zencoder.com/oceans-clip.ogv" type='video/ogg'> <p>Video Playback Not Supported</p> </video>

Or if you're passing in the sources programatically:

myPlayer.src([ { type: "video/mp4", src: "http://www.example.com/path/to/hd_video.mp4", res: "HD" }, { type: "video/mp4", src: "http://www.example.com/path/to/sd_video.mp4", res: "SD", default: true }, { type: "video/webm", src: "http://www.example.com/path/to/video.webm" }, { type: "video/ogg", src: "http://www.example.com/path/to/video.ogv" } ]);

A limitation of the implementation is that only homogeneous video types are resolution switchable. So you can't specify a webm SD and an mp4 HD and expect the player to pick it up. That might be correctable, but it might involve changing player "technologies" mid-play ie. HTML5 -> Flash. Technically my implementation does restart the tech on resolution change, but I didn't want to mess with actually changing technologies for this rev.

Here's what it looks like:
http://i.imgur.com/IA5el.png

Any number of resolution streams are supported. The 'res' parameter is arbitrary and doubles as the label. Could be '720p', '360p', and '240p' if you wanted. it displays in the order specified in the HTML markup or the params array.

I'd be happy to write tests if Zencoder can provide alternate bitrate sources of the 'ocean' video for use in the testing suite.

Here's the result of my testing:

Chrome (Mac & Win): pass
Safari (Mac): pass
Firefox (Mac & Win): pass
Android: pass
IE9: pass
IE8: fail (tried with Flash) however the zencoder repo without my edits didn't work for me on IE8 either

These commits only address the HTML5 side of things, not the Flash side.

Sorry about the closed pull request. I was gonna rebase but I already pushed these to my master and didn't want to screw anything up.

onyxrev added some commits Oct 8, 2012

@onyxrev onyxrev nuke trailing white space b5f801a
@onyxrev onyxrev more white space nuking 5412140
@onyxrev onyxrev adding helpers for reduce, objKeys, and objValues to lib ad4ab52
@onyxrev onyxrev adding bucket method for techs so that we can easily group sa keyed b…
…y tech and then by resolution
2774f37
@onyxrev onyxrev I think we probably want to keep the bucketized sources ordered so th…
…at people can choose their resolution preferences by order.
50a4aa2
@onyxrev onyxrev selectSource now parses through an array of resolutions to find the b…
…est available resolution from those provided
5c5b0c4
@onyxrev onyxrev now pulling in the 'res' attribute from sources tags
Please enter the commit message for your changes. Lines starting
ff78278
@onyxrev onyxrev adding cookie setter, getter, eraser 90ac46a
@onyxrev onyxrev can probably do without eraseCookie 284e195
@onyxrev onyxrev pulling preferred res from cookie 05d48d9
@onyxrev onyxrev whitespace nuke for tracks.js 572cbe8
@onyxrev onyxrev storing resolutions for use in switching after-the-fact 613f03f
@onyxrev onyxrev adding initial stab at a resolution changing button a6175a3
@onyxrev onyxrev whitespace controls.js e948348
@onyxrev onyxrev I think resolution button should live in controls.js, not tracks.js b1ffe5a
@onyxrev onyxrev adding resolutions.js module 80754c2
@onyxrev onyxrev adding resolutions.js to various require statements 57d5403
@onyxrev onyxrev resolutions button now rendering but sans resolutions 4c75bef
@onyxrev onyxrev resolutions now rendering in the new resolutions button 81cb5d4
@onyxrev onyxrev I don't think we really need this Resolutions class at all 641701c
@onyxrev onyxrev adding css rule for resolution button - just using the 'HD' glyph for…
… now
b2495fd
@onyxrev onyxrev changeResolution now supports kicking off a new stream and seeking to…
… the last playback position
25c49f5
@onyxrev onyxrev ignore my local test HTML file bfd3b51
@onyxrev onyxrev fire resolutionchange event on resolution change 53a7b42
@onyxrev onyxrev add support for 'default' attribute on sources with multiple resolutions 87d74a7
@onyxrev onyxrev resolution switcher now clears the menu item focus pseudoclass based …
…on whether the menu item's source is the current source
95ed77b
@onyxrev onyxrev switch the 'gear' icon because YouTube uses that for resolution changes 6806589
@onyxrev onyxrev I just noticed the localStorage support - so we don't need the cookie…
… code.
a345816
@onyxrev onyxrev using events to set default resolution menu item selection and rememb…
…ering new selections in localStorage
68dbc63
@onyxrev onyxrev forcing resolution index to be an integer e499649
@onyxrev onyxrev adding some more comments for new lib methods and resolution-related …
…player functionality
534149a
@onyxrev onyxrev resolution selector no longer displays if not more than one available…
… source
2ec01b2
@onyxrev onyxrev oops. actualRes variable leaking into global namespace due to ; inste…
…ad of ,
46fd3b8
@onyxrev onyxrev locaStorage was saving the resolution offset as a string. forcing to …
…integer.
489025c
@onyxrev onyxrev Changing video tag src attributes will fail in IE9 if source elements…
… are present. It will always play the first source even if you set this.el.src. We must remove the source elements before trying to change the src attribtue programatically.
e26aa1b
@onyxrev onyxrev potential workaround for flash not being ready to seek when it fires …
…the 'play' event
74ae7fc
@onyxrev onyxrev 'loadeddata' appears to be the reliable event to queue seeking off of…
… for resolution changes. no more timeout needed!
d94c281
@onyxrev onyxrev Fix for 'default' not properly being detected in some browers. Needed…
… to check for presence of preferred resolution value in addition to localstorage itself.
08a74d3
Owner

heff commented Oct 29, 2012

Very cool, thanks Dan. I haven't had a chance to dig into this yet but hope to soon. Cheers!

Contributor

onyxrev commented Oct 30, 2012

Great! Let me know if you encounter anything. From my testing it seems the Flash component inherited the feature fine as well.

Contributor

mente commented Nov 6, 2012

Would be great to see it in upstream. Anything I can help with to see this PR merged?

Contributor

onyxrev commented Nov 28, 2012

We've discovered an issue with this code. Basically the HTML5 spec doesn't provide a way to stop a video from buffering once it's started. In fact in Chrome and other browsers the video continues to download even after destroying the video entirely and removing it from the DOM. This means that each time you switch resolutions you initiate a new concurrent download. Ouch.

There's a hacky workaround:
http://stackoverflow.com/questions/4071872/html5-video-force-abort-of-buffering

But for me it breaks at least IE9.

Quite a bummer. I'll be looking at it further.

Contributor

onyxrev commented Dec 7, 2012

With regards to aborting the buffering - we've found this varies from CDN/host to CDN/host. One of the customers serving from an Ubuntu/Apache host suffers from the concurrent downloads whereas one serving from S3 does not - the video aborts when the streams change.

It's most likely something deep in the HTTP exchange. Your mileage may vary.

CorwinT commented Dec 10, 2012

I've also been looking into resolution switching (amongst other things) for the company I work for and it would be great if this or something similar could be integrated into the main project.

One suggestion to add to the proposed implementation however: res and default are not standard html5 tags, but custom data tags are (eg. data-res, data-res-default).

http://www.w3.org/html/wg/drafts/html/master/dom.html#embedding-custom-non-visible-data-with-the-data-*-attributes

Could we use these instead? In our solution we have player.js expose all custom attributes to allow for greater flexibility later.

if (c.nodeName == "SOURCE") {
    var source = {};
    source["src"] = c.getAttribute("src");
    source["type"] = c.getAttribute("type");
    source["media"] = c.getAttribute("media");
    source["title"] = c.getAttribute("title");

    for (var k = 0; k < c.attributes.length; k++) {
        var attribute = c.attributes[k];
        if (attribute.name.match("^data-")) {
            source[attribute.name] = c.getAttribute(attribute.name)
        }
    }

    options.sources.push(source);
}

cowai commented Feb 26, 2013

Is this planned to get into master?

Owner

heff commented Apr 4, 2013

@onyxrev Still working with Vidcaster? If so this would be great as a plugin.
https://github.com/zencoder/video-js/wiki/Plugins

We're currently drawing the line of the core at features supported by the native video element (except also supporting them in flash) and everything else will be a plugin.

PS. Sorry, it's taken so long to get to this. Things got crazy.

heff closed this Apr 4, 2013

Contributor

onyxrev commented May 11, 2013

Yes, looks as though we will probably plugin-ize this guy somehow.

xdmx commented Jun 25, 2013

@onyxrev any news on the plugin for resolution switching?

Contributor

onyxrev commented Jun 25, 2013

@xdmx yes, I've pluginized it in Vidcaster's private repo and it's looking pretty good but we're running into stability issues with VideoJS 4 at large on mobile platforms. I's been delayed a bit as the stability issues make testing resolutions against mobile impossible.

xdmx commented Jun 25, 2013

I hope you'll figure out that issues and release it to the public, I'd love to use it in a small project I'm developing!

xdmx commented Jun 26, 2013

@onyxrev have you been able to address resolution switching for the flash player as well? or will it be just for the html5 version? If the latter, @heff do you plan to add resolution switching in the core of videojs (so both html5+flash)?

Contributor

onyxrev commented Jun 26, 2013

@xdmx the plugin addresses both Flash and HTML5. I'll talk to Vidcaster to see if we can release the code to get the community involved sooner rather than later.

xdmx commented Jun 26, 2013

That would be really awesome!!! ❤️

Owner

heff commented Jun 26, 2013

@xdmx Resolution switching should always be a plugin since (I'm assuming) it won't ever be added to the HTML5 video spec. That's our line for core features. Though I'm looking forward to this plugin myself.

@onyxrev Make sure the mobile issues you're running into get reported if they're not yet. We are working through them, even if it's slowly.

Contributor

onyxrev commented Jul 30, 2013

@heff @xdmx @kylegregory Vidcaster has open-sourced my work on the videojs resolutions plugin. It's available here: https://github.com/vidcaster/video-js-resolutions

It's not perfect but it's pretty good. Please fork and improve!

Owner

heff commented Jul 30, 2013

Nice!

Bonus Points: Set up a quick demo page on codepen.io or jsbin showing this off. You could fork one of these and drop the plugin code into the js section.
http://codepen.io/heff/pen/EarCt
http://jsbin.com/axedog/7/edit
(If you don't want to upload your own clips, you could use the same oceans source and pretend it's different versions for demonstration purposes)

Ref #413

Contributor

onyxrev commented Jul 30, 2013

Yeah, it'd also be nice to have some test coverage. The plugin will ignore URLs that look the same, so we'll at least need streams with different endpoints. Guess I could add a garbage parameter or something.

Owner

heff commented Jul 30, 2013

Yeah, that could work.

Noticed the target IE was 10. What keeps this from being compatible with IE9?

Contributor

onyxrev commented Jul 30, 2013

When we built the resolution switching fork of 3.2.2 we experienced issues switching streams on IE9 so Vidcaster dropped resolution switching support for it. It's possible 4.x will fare better but we haven't tried it. If I can get a demo page up we can give it a go.

Owner

heff commented Jul 30, 2013

Cool, sounds good.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment