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

updated added RTMP support. fixes #559 #634

Closed
wants to merge 3 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
93 changes: 87 additions & 6 deletions src/js/media/flash.js
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,14 @@ vjs.Flash = vjs.MediaTechController.extend({

// If source was supplied pass as a flash var.
if (source) {
flashVars['src'] = encodeURIComponent(vjs.getAbsoluteURL(source.src));
if (source.type && vjs.Flash.isStreamingType(source.type)) {
var parts = vjs.Flash.streamToParts(source.src);
flashVars['rtmpConnection'] = encodeURIComponent(parts.connection);
flashVars['rtmpStream'] = encodeURIComponent(parts.stream);
}
else {
flashVars['src'] = encodeURIComponent(vjs.getAbsoluteURL(source.src));
}
}

// Add placeholder to player div
Expand Down Expand Up @@ -227,10 +234,16 @@ vjs.Flash.prototype.pause = function(){
};

vjs.Flash.prototype.src = function(src){
// Make sure source URL is abosolute.
src = vjs.getAbsoluteURL(src);

this.el_.vjs_src(src);
if (vjs.Flash.isStreamingSrc(src)) {
src = vjs.Flash.streamToParts(src);
this.setRtmpConnection(src.connection);
this.setRtmpStream(src.stream);
}
else {
// Make sure source URL is abosolute.
src = vjs.getAbsoluteURL(src);
this.el_.vjs_src(src);
}

// Currently the SWF doesn't autoplay if you load a source later.
// e.g. Load player w/ no source, wait 2s, set src.
Expand All @@ -240,6 +253,20 @@ vjs.Flash.prototype.src = function(src){
}
};

vjs.Flash.prototype.currentSrc = function(){
var src = this.el_.vjs_getProperty("currentSrc");
// no src, check and see if RTMP
if (src == null) {
var connection = this.rtmpConnection(),
stream = this.rtmpStream();

if (connection && stream) {
src = vjs.Flash.streamFromParts(connection, stream);
}
}
return src;
};

vjs.Flash.prototype.load = function(){
this.el_.vjs_load();
};
Expand Down Expand Up @@ -304,7 +331,7 @@ vjs.Flash.isSupported = function(){
};

vjs.Flash.canPlaySource = function(srcObj){
if (srcObj.type in vjs.Flash.formats) { return 'maybe'; }
if (srcObj.type in vjs.Flash.formats || srcObj.type in vjs.Flash.streamingFormats) { return 'maybe'; }
};

vjs.Flash.formats = {
Expand All @@ -314,6 +341,11 @@ vjs.Flash.formats = {
'video/m4v': 'MP4'
};

vjs.Flash.streamingFormats = {
'rtmp/mp4': 'MP4',
'rtmp/flv': 'FLV'
};

vjs.Flash['onReady'] = function(currSwf){
var el = vjs.el(currSwf);

Expand Down Expand Up @@ -357,6 +389,11 @@ vjs.Flash.checkReady = function(tech){
// Trigger events from the swf on the player
vjs.Flash['onEvent'] = function(swfID, eventName){
var player = vjs.el(swfID)['player'];
// ugly, but it smooths over progress and
Copy link
Member

Choose a reason for hiding this comment

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

Is there an issue that this refers to? Or can you elaborate?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@iamjem can you answer this? I don't know what this was about myself.

Copy link
Contributor

Choose a reason for hiding this comment

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

I think this was specific to the RTMP playback. I was noticing instances where playback would end, but the currenttime and duration components weren't finishing their count up or down. Its possible this has been fixed, and it may be worth re-testing since this was first implemented with version 3 of the codebase.

Copy link
Contributor

Choose a reason for hiding this comment

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

Tested streams provided from @bdeitte along with some personal Cloudfront streams with RTMP/RTMPT/RTMPE (seems RTMPS requires additional work to use, may not even be supported by Akamai/Amazon) and this bandaid doesn't seem necessary anymore. Time codes update correctly, progress bar completes, and I get the "ended" event firing at the correct time.

I've removed it in my 559-rtmp-support branch (which should also include other recent RTMP changes from Brian, and from 1 and 2). I think this covers all of the outstanding items we had.

// time control manual timer issues
if (eventName === "ended") {
player.trigger("timeupdate");
}
player.trigger(eventName);
};

Expand Down Expand Up @@ -453,3 +490,47 @@ vjs.Flash.getEmbedCode = function(swf, flashVars, params, attributes){

return objTag + attrsString + '>' + paramsString + '</object>';
};

vjs.Flash.streamFromParts = function(connection, stream) {
return connection + "&" + stream;
};

vjs.Flash.streamToParts = function(src) {
var parts = {
connection: '',
stream: ''
};

if (! src) {
return parts;
}

// Look for the normal URL separator we expect, '&'.
// If found, we split the URL into two pieces around the
// first '&'.
var connEnd = src.indexOf('&');
var streamBegin;
if (connEnd !== -1) {
streamBegin = connEnd + 1;
}
else {
// If there's not a '&', we use the last '/' as the delimiter.
connEnd = streamBegin = src.lastIndexOf('/') + 1;
if (connEnd === 0) {
// really, there's not a '/'?
connEnd = streamBegin = src.length;
}
}
parts.connection = src.substring(0, connEnd);
parts.stream = src.substring(streamBegin, src.length);

return parts;
};

vjs.Flash.isStreamingType = function(srcType) {
return srcType in vjs.Flash.streamingFormats;
};

vjs.Flash.isStreamingSrc = function(src) {
return src && src.indexOf(",") !== -1;
Copy link
Member

Choose a reason for hiding this comment

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

stream[To/From]Parts() seem to assume the src will have a & or / separating the stream parts, but isStreamingSrc looks for a comma. Does this need 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.

Thanks for catching this- something I messed with after testing and should have written a unit test for too eh. I'll fix this in the next day or so and let you know when the pull request is updated.

Copy link
Member

Choose a reason for hiding this comment

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

Let me know if there's anything I can do to help polish this off. I'd love to get it pulled in.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I really, really want to see this in as well. I just haven't had the time yet to get this done. Here's what I think still needs to be done:

  1. Change the line above to look for "rtmp" at the beginning
  2. Add a unit test for this function
  3. Retest a number of streams, both with and without the "timeupdate" change above to see if it's really needed

The third item could also potentially be done after pulling a new SWF from video-js-swf, since Tom J's NetStream changes aren't in videojs yet.

I know I won't get to any of this during the week, but I'm hopeful for the weekend again...

Copy link
Contributor Author

Choose a reason for hiding this comment

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

And to be clear, if you or someone else wants to do the above before I get to it, I'm more than happy with that. :) I can provide roughly 10 test streams (live and non-live, different CDNs) for the third item.

Copy link
Member

Choose a reason for hiding this comment

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

Cool, thanks Brian. I'm buried in other issues at the moment, but if I can get to this I'll at least try to do 1 & 2.

Copy link
Contributor

Choose a reason for hiding this comment

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

I've updated 1 and 2 in my fork's master now. Still need to look at 3, will take a bit longer. Also do we need to update the exports.js with some of these new methods?

Copy link
Member

Choose a reason for hiding this comment

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

Ah, that's a good point. We'll need to add any new flash api methods to the externs file.
https://github.com/videojs/video.js/blob/master/src/js/media/flash.externs.js

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That's great to hear @iamjem! I tried to take a look in your master branch but didn't see the commits- are they not pushed yet or did I just miss them? I'm going to email you a few test assets for 3, in case that helps you out.

};
Binary file modified src/swf/video-js.swf
Binary file not shown.
3 changes: 2 additions & 1 deletion test/index.html
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,8 @@
'test/unit/core.js',
'test/unit/media.html5.js',
'test/unit/controls.js',
'test/unit/plugins.js'
'test/unit/plugins.js',
'test/unit/flash.js'
];

var projectRoot = '../';
Expand Down
34 changes: 34 additions & 0 deletions test/unit/flash.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
module('Flash');

var streamToPartsAndBack = function(url) {
var parts = vjs.Flash.streamToParts(url);
return vjs.Flash.streamFromParts(parts.connection, parts.stream);
};

test('test using both streamToParts and streamFromParts', function() {
ok('rtmp://myurl.com/&isthis' === streamToPartsAndBack('rtmp://myurl.com/isthis'));
ok('rtmp://myurl.com/&isthis' === streamToPartsAndBack('rtmp://myurl.com/&isthis'));
ok('rtmp://myurl.com/isthis/&andthis' === streamToPartsAndBack('rtmp://myurl.com/isthis/andthis'));
});

test('test streamToParts', function() {
var parts = vjs.Flash.streamToParts('http://myurl.com/streaming&/is/fun');
ok(parts.connection === 'http://myurl.com/streaming');
ok(parts.stream === '/is/fun');

parts = vjs.Flash.streamToParts('http://myurl.com/&streaming&/is/fun');
ok(parts.connection === 'http://myurl.com/');
ok(parts.stream === 'streaming&/is/fun');

parts = vjs.Flash.streamToParts('http://myurl.com/streaming/is/fun');
ok(parts.connection === 'http://myurl.com/streaming/is/');
ok(parts.stream === 'fun');

parts = vjs.Flash.streamToParts('whatisgoingonhere');
ok(parts.connection === 'whatisgoingonhere');
ok(parts.stream === '');

parts = vjs.Flash.streamToParts();
ok(parts.connection === '');
ok(parts.stream === '');
});