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

Fix issues with incorrect RTMP video size. #131

Closed
wants to merge 1 commit into from

Conversation

Projects
None yet
2 participants
@danbrianwhite
Copy link

commented Jan 25, 2013

There is an issue with the RTMP video playback. This issue is that all videos are set to the incorrect size and do not update with the metaData dimensions from the video.
This fix removes the "myStatus.metaDataReady = true;" line so the flag will stay "false" and allow the video's size to be set in the "onMetaDataHandler" function. The incorrect sizing of the RTMP video can be seen in the RTMP Demo at the following URL: "http://jplayer.org/latest/demo-01-video-supplied-rtmpv/"

The change is on line 366 (this line is commented out). I could not get the file to only show the difference of this one line.

Update actionscript/happyworm/jPlayer/JplayerRtmp.as
There is an issue with the RTMP video playback. This issue is that all videos are set to the incorrect size and do not update with the metaData dimensions from the video.
This fix removes the "myStatus.metaDataReady = true;" line so the flag will stay "false" and allow the video's size to be set in the "onMetaDataHandler" function. The incorrect sizing of the RTMP video can be seen in the RTMP Demo at the following URL: "http://jplayer.org/latest/demo-01-video-supplied-rtmpv/"

The change is on line 366 (this line is commented out). I could not get the file to only show the difference of this one line.
@ghost

This comment has been minimized.

Copy link
Contributor

commented Jan 28, 2013

Thank you for this. This is backing up the PR of #98 which also had this strange problem with all the lines changing.

You solution is slightly different, but essentially the same.

I have made the change manually, but I do not see a difference with the RTMP demo video aspect ratio.

Did you test that RTMP url with your version?
NB: The poster image (before it is played) should not be used for comparison.

Otherwise, do you have a RTMP url that I can use to test my version with. ie., One that is wrong before and fixed with your change. Than I can verify that I've made the correct change.

@ghost

This comment has been minimized.

Copy link
Contributor

commented Jan 28, 2013

Scratch that. I started again and did it your way. I had followed the other solution, but must have messed something up. I had initially confused the onMetadataHandler and the onResult handler coding...

The RTMP demo on jplayer.org now shows the video in a different aspect ratio. Before it was pillarbox and now it is slightly letterbox with a little bit of black top and bottom. I have not investigated the actual RTMP video size at this time and mismatch between the 360p player GUI may well be correct.

@ghost

This comment has been minimized.

Copy link
Contributor

commented Jan 28, 2013

Hmm... After reviewing the code a little further, I am reminded that the video size info is being passed to the JavaScript. However, the JS does nothing with it. I had previously considered adding the video width and height to the jPlayer status so that responsive GUIs could know how big the Flash was supposed to be. A niche, hence why it has not happened yet.

After I push this change I expect that I'll take a quick look into getting the video size info for both HTML5 and Flash. HTML5 has it available as does the flash.

On a tangent here, but will push this change soon.

Cheers!

happyworm pushed a commit that referenced this pull request Jan 28, 2013

@happyworm happyworm closed this Jan 28, 2013

@ghost

This comment has been minimized.

Copy link
Contributor

commented Jan 28, 2013

FYI - The 100% file diff was due to the line endings format getting cocked up at some point.

My windows machine was using MAC line endings for that file, and in turn my GitBash was assuming it had windows line endings. Corrected it now so future pull requests are seamless.

@danbrianwhite

This comment has been minimized.

Copy link
Author

commented Jan 29, 2013

Good evening,

Thank you for looking not my pull request. I tested the RTMP video that is used on your site and verified that the change will specifically fix your demo. I am not sure if there is a better way to fix the RTMP bug but I assumed this would be the simplest solution. In regards to one of your other emails, you stated that there is some functionality to have the flash player to send the javascript the actual video size. Is this feature already implemented? This feature would actually be quite useful for me as I am working on developing a responsive layout system for eLearning courses, and today I was just thinking about how I could get the video size from jPlayer.

I also have a question for you pertaining to the work you have done with popcorn.js. In you experience, would it be useful to use it or develop should I create my own simple solution for syncing content with audio and video. Sadly I also have to develop full functionality in IE6 so I don't think popcorn.js will work for my situation.

Thanks,

Dan White

On Jan 28, 2013, at 12:05 PM, happyworm notifications@github.com wrote:

FYI - The 100% file diff was due to the line endings format getting cocked up at some point.

My windows machine was using MAC line endings for that file, and in turn my GitBash was assuming it had windows line endings. Corrected it now so future pull requests are seamless.


Reply to this email directly or view it on GitHub.

@ghost

This comment has been minimized.

Copy link
Contributor

commented Jan 29, 2013

I plan to implement the video size information on the jPlayer status next. The HTML5 video element has the information, as does the Flash. All I need to do is make it available in a sensible manner. That would be through the status, which is available on the event object or directly.

If you have to develop for IE6 then popcorn.js will not be much use to you. At most it will only ever work with IE8. They provide an IE8 shim that plugs the holes in the IE8 JavaScript and DOM APIs. I suppose you could write a shim suitable for IE6 if you are very good at that sort of thing.

In practice, we try and make a client see sense when they desire the latest cutting edge features, but demand IE6 functionality.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.