RTMP Support Updates

[tomjohnson916]

• Updated Flash.js to include rtmpStream, rtmpConnection in api.readWrite
• Updated Flash.js to replace double quote usage with single quote to validate under jsHint.
• Tested with multiple SWFs (one including IProvider.die() SWF) and RTMP streams.

[bdeitte]

Looks good to me Tom- nice to see this tied up.

Note that if this is accepted, the following two pull requests can be closed as they contain a subset of this request: #634 and #605

[heff]

@iamjem @bdeitte Would either of you guys be willing to write up a quick guide on using RTMP that I could put out there along with the new version?

[iamjem]

I can probably do this before the end of the weekend, possibly even tomorrow. I did remove the extra "ugly" code in the flash.js onEvent method, and its updated in my 559-rtmp-support branch. It doesn't appear to be necessary anymore. I can make the docs addition in this same branch of mine if thats easiest? Or should I merge it all into my master?

[iamjem]

I've added streaming details in the tech.md file in my 559-rtmp-support branch now.

[heff]

Awesome, thanks!

[heff]

@seniorflexdeveloper I pulled down the branch locally and I can't seem to get an rtmp stream to play.

My source looks like this.

<source src="rtmp://cp67126.edgefcs.net/ondemand/mp4&mediapm/ovp/content/test/video/spacealonehd_sounas_640_300.mp4" type='rtmp/mp4'>

Does that work for you?

[tomjohnson916]

Looking now to see why that doesn't work. I was using the JS programmable route, i.e. on player.ready

player.src({ type: "rtmp/mp4",
src: "rtmp://cp88098.edgefcs.net/ondemand/&mp4:media/29906152001/29906152001_2538182935001_oceans-clip.mp4"
});

[bdeitte]

I suggest trying this on the iamjem:559-rtmp-support branch. That has the most RTMP changes right now, so it should be the branch that is pulled in.

[tomjohnson916]

The stream URL @heff is using 404s in the Akamai Support Player. The URL should be; rtmp://cp67126.edgefcs.net/ondemand/mp4:mediapm/ovp/content/test/video/spacealonehd_sounas_640_300.mp4. Double checking now.

[tomjohnson916]

As an update to my previous comment, the correct value including the ampersand delineator is as follows. This works in a sandbox implementation of this PR.

[iamjem]

I've noticed two new problems after testing the RTMP updates with the most recent SWF from video-js-swf. Browser doesn't matter, just so long as you're using Flash playback with RTMP:

1. Clicking on the flash object triggers play/pause twice. So if a video is playing, you click inside the video, playback pauses and resumes immediately. If its paused and you click inside of it, it plays then pauses immediately. Probably some event firing twice.
2. When playback is paused, and you click on the progress bar, the flash seems to seek to the new point in the video BUT the progress bar doesn't update accordingly. Often times the flash object shows a frame from a totally different timecode. However, when you resume playback the flash seems to begin in the correct place, and the progress bar snaps back to where it needs to be.

[tomjohnson916]

@iamjem - I can confirm seeing that bug in Chrome, but not in Safari or Firefox (all on Flash Player version 11.5.502).

[iamjem]

Ah, it appears 1 is my own doing, I had some old onstageclick code in my dev branch, I've reset it and thats resolved now. 2 does still appear to happen though, I'll try to investigate some further.

[iamjem]

Ok, tracked down the problem. Seems Flash playback (doesn't matter if its progressive or streaming) has some problems reporting accurate times. Here's the two scenarios that exist:

1. In the case of streaming, if you seek on the progress control while video is paused, the control will call currentTime() on the player to do the seek, and immediately ask for the time back to calculate the progress bar position. The RTMP provider doesn't report the new time until its finished seeking, so it always returns the old time.
2. With progressive playback, while playback is paused, the progress bar will seek to the location of your click. Once you resume playback however, the progress control typically skips forward/backwards to adjust to where flash actually winds up seeking to. This is just a problem of progressive playback.

So, I enabled the seeking method on the Player class (it was already on both HTML5 and Flash tech, and enabled in the media API.. just wasn't "turned on" in the Player prototype). When a user seeks now, the progress control checks if the current tech is Flash and if its currently seeking.. if so it trys to use the cached lastCurrentTime value. Not the prettiest, but it fixes the problem.

This is in my 559-rtmp-support branch now.

[heff]

Thanks for digging into that Jeremy.

We've got 3 (?) pull requests continuing for this. Can @seniorflexdeveloper or @bdeitte continue to pull everything together, or should I create a new branch in the main repo where I can immediately pull any changes into?

[bdeitte]

@heff the last time I checked, @iamjem has all changes from the other two pull requests in his branch

[tomjohnson916]

@bdeitte is it the master branch? I will confirm later but looking at his flash.js file, it appears it's still missing the rtmpStream, rtmpConnectionUrl in the API readwrite.

https://github.com/videojs/video.js/blob/master/src/js/media/flash.js#L263

[bdeitte]

@seniorflexdeveloper no it's 559-rtmp-support

[iamjem]

If my branch is the closest, and something is still missing, just let me know what else needs to be merged into it. I can add a test for the seeking method, seemed like there may be several other Player method tests that could be missing when I was cruising the tests before. Seeking will always return false from the "fake" test player though, so it may not be too valuable.

[tomjohnson916]

@iamjem - https://github.com/iamjem/video.js/blob/559-rtmp-support/src/js/media/flash.js#L290 needs to include 'rtmpStream,rtmpConnection' and then I think you're set. For example, see tomjohnson916@dcc0b91#L0R293.

[iamjem]

I've merged your master into the 559-rtmp-support, looks like it just included the items you mentioned, along with changing some double quotes to single, and removing white space. Pushed to github now.

[heff]

Added through #605