Update JplayerRtmp.as #161

Closed
wants to merge 1 commit into
from

Conversation

Projects
None yet
5 participants
Contributor

iking0980 commented May 10, 2013

Fix problem with connection Amazon cloudfront signeture url

@iking0980 iking0980 Update JplayerRtmp.as
Fix problem with connection Amazon cloudfront signeture url
526da54
@ghost
Contributor

ghost commented Nov 2, 2013

The RTMP part was @rmhall baby.
Could you eyeball this PR Robert and give a nod if it should be merged.

Ideally if you or @iking0980 could provide an example RTMP url for an Amazon cloudfront signature... Then I/we can review and test it.

rmhall commented on 526da54 Nov 3, 2013

This appears to be correct based on the additional information that can be passed in as url query params for Amazon CloudFront rtmp connection strings, like this example: rtmp://s3example.cloudfront.net/cfx/st/mp4:path_to_file/more_path/my_file.mp4?Expires=xxxxxxxxx&Signature=xxxxxxxxx&Key-Pair-Id=xxxxxxxxxx&Policy=xxxxxxxxxxxx

Offhand, I don't believe it should cause any problems for other implementations that also pass in additional params, should help there as well.

Not to be nitpicky, but purely for formatting consistency and readability, I'd make sure the else statement on lines 521 and 536 wraps braces around the following streamFileName = … statements.

ghost replied Nov 3, 2013

The code does not work with the existing RTMP demo.

Investigating, the 2 lines like this look incorrect:

if ( streamFileName.indexOf("?") )

Failing to find the ? will return -1, which is truethy. So it worked fine in @iking0980 case, which would have had the ? somewhere in the url, but fails with our original RTMP url.

@ghost
Contributor

ghost commented Nov 3, 2013

Nit pick away. I always add the curlies too 👍
It is one of the recommended coding styles for Open Source projects.

Thank you for the feedback @rmhall - I plan to integrate this into the dev branch next.

@ghost
Contributor

ghost commented Nov 3, 2013

While I'm in this part of the code... I also noticed this:

if (rtmpSrc.indexOf(".mp3") != -1) {
   appName = rtmpSrc.substring(endHost,rtmpSrc.indexOf("mp3:"));

Shouldn't they both be "mp3:" @rmhall?
BTW, this code was not part of the PR commit.

@ghost ghost pushed a commit that referenced this pull request Nov 3, 2013

@thepag thepag Fixed logic errors in #161 and cleaned code styling. 5ca8187
@ghost
Contributor

ghost commented Nov 3, 2013

Code merged into the dev branch.

Release note:
[dev] Added Support: The Flash RTMP player now supports an Amazon CloudFront Signature URL. This addition was proposed here Update JplayerRtmp.as by iking0980.

Cheers to all involved in this change.

happyworm closed this Nov 3, 2013

Contributor

thepag commented Nov 12, 2014

Hello @iking0980 would you please sign our CLA

Cheers!

Contributor

thepag commented Nov 19, 2014

Hello @iking0980
Please sign the CLA

If you do not want to sign it, then please contact me and we can remove your contribution from the project.

Contributor

thepag commented Nov 19, 2014

Cheers @iking0980

I don't think this fix worked. Still can't play when the urls are signed.

RTMP is horribly broken.

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