-
Notifications
You must be signed in to change notification settings - Fork 1.5k
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
Add Aurora.js solution #246
Conversation
I have given this the initial eyeball review and on the whole it looks fine. I see the use of The amount of code added appears acceptable. See below. I know I may appear a little slow right now, but I want to bring in all the changes that I've made from the dev branch before making this merge. And I also have the website to get there too. So bear with me. Your contribution is appreciated. It should just fine since I moved all the existing repo files through This reminds me of a previous suggestion that made the internal players more generic. So we have a "jPlayer player plugin" for each solution. Then we can have various benefits and just swap the pointers around and have a common structure to play and pause and so on. This may also play nice with possible build options to optimise the code size. |
jQuery 1.7 for the on() but that is fine. I have been thinking of doing it before, since the playlist code uses on() already. |
Thanks for the review and your roadmap summary. I will keep an eye on your progress ;) I used on() because of playlist code in fact, I didn't noticed about the minimum version requirement change. If you want to bump minimum jQuery version to 1.7 that's fine then. |
Looking at this next... I think I'll add most of your 1st post to the readme while merging this PR. I will also prepare a demo ready for it too. This will mean copying in those aurora files to my /lib/ folder in the repo so that the examples will work. Plus I'm wondering how to describe the dependancy in the JSON files describing the package... jQuery is required, but these are an optional extra with conditional dependancy. |
I'm have a merge conflict problem, but that is not the strange part. It is trying to merge your changes with the copy of the source file we put into the dist folders. There should not be a relationship between that file in dist, since it is a new file. During my refactor I took care to always move the existing files through git so the relationship was maintained. I have not run into this issue with any of the other PRs over the past few days. |
The conflict was in the comments and was easy to fix. I changed my comment about the SWF filename and you change the comment about the solution option. They were on different lines, but it threw a conflict. Would you be able to fix the pull request so that it affects the correct file? Your changes should point at the source here: I do not know how that went wrong. It looks like your PR points at the old file in the repo that I |
Ok :) About the composer dependency, I never maintains it (it's also something in my todo for some projects ^^), @thormeier? |
I will give it a try again now. I do not use the merge pull request button on githib.com preferring to manually do it and review and diff before I merge it in. It helps catch stuff like yesterday. I think for the dependancy, we just leave aurora off the list and add it to the documentation. I am referring to all 3 JSON file, package.json, bower.json and composer.jons. Cheers. |
@Afterster this merge is looking fine. Thank you for sorting it out. I am in the process of making a demo and adding the Aurora files to the lib folder, when I noticed your support list and that AAC seemed to be missing for the m4a format and then there are 2 formats, Opus and OGG, for the oga format. The mp3 and flac formats line up. And while trying to make a sensible demo for this, I wonder if we should show a single FLAC media file being used, and how jPlayer tested the HTML and Flash before deciding that the Aurora solution could play it. If we use mp3 then the html or flash would be used... Hmm... I think I'll create a new batch of demos for this, they will be like demo-01 and the main one will be like I described above for Flac and jPlayer picking Aurora to play it. Then have some others forcing Aurora and mp3, and ogg and so on so we can review each of the Aurora players specifically. |
OK I see now that the command AV.Player.fromURL(url) appear to deal with all that. I also note that some codecs are built in, but it is the WAV codec and some others I'm not familiar with: So we could add wav to the list of formats in the defaults for Aurora. I am also considering trimming that list down, to just "wav mp3" or maybe only "wav" since that is what aurora core supports. |
These Aurora codecs do not seem to like each other very much. Including aac.js was breaking the ogg.js and then after removing aac.js, the vorbis.js was not playing our OGG files very well at all. After that, I started only including the codec I was interested in. The mp3.js worked well until you change the currentTime and then it dies. The acc.js seemed to work the best. I've not got round to testing the flac yet. I'll make some flac files to test it with. Not tested with opus either... But I am hoping the flac works well, and then I will only demo that. I suspect that codec works pretty well since browsers do not support it. |
The |
Indeed, I also noticed some codec conflict, sometimes this conflict are better handled according to the browser used. |
I'm finding that Decoder.seek() is causing problems with the object it returning being undefined. At this point, I'd be happy if we can get a demo working. The only one that works is the AAC - with the progress bar too - many of them play - but then break if you click the progress bar - but the ACC one is flawed due to sending seconds from jPlayer and their AAC must have that error... But that nuts because it is the only one that actually works with seek LOL. Calling it a night. |
I begin to understand why it is not documented 😄 . |
Nope, I was using Chrome. |
All i did was press play, listen to a bit of music play and then click on the progress bar. Instant death. ATM, I am starting to question the need to add this player. This may be added as an experimental unsupported feature that we do not recommend anyone uses... So why bother putting it in if it is such a knife edge whether it works or not. |
The main issue is on seeking, otherwise streaming and other interactions are working fine on my side. |
I was just getting a bit stressed, since i no longer have any time to do this. I will merge with the project and then we can fix it on route. I will make a demo, and explain that Aurora.js is an experimental feature, with limitations. I think I'll leave the Seek in seconds like you had it, since the only demo that works is AAC so I can have something to show people. |
Ok good. |
That sounds good. I should have some time in the last week of December to look at merging a solution plugin system. Maybe if you make a pull request once you have something, we can begin reviewing and revising its requirements at the weekends, with the aim of merging sometime in late december. For example:
|
Oh and try to follow the jshint rules in the .jshint files. We conform with all of the jQuery ones, except the double quotes rule. I try and use doubles now for new additions, but as you may notice, I forgot just now in that example for the 'object' string. |
Yes I agree, the plugin should be an independant instance. |
I've started #266 for the player plugin discussion. This thread will close when dev branch is merged with master. |
This add Aurora.js solution to jPlayer, like are html / flash solutions.
I personally did this for the Ampache project to avoid server transcoding when it is only used because of browser compatibility issues.
To test it, get the branch and set the following jPlayer options:
Then include Aurora.js scripts / decoders:
I added a new auroraFormats option because Aurora.js can be extended with new decoders and we cannot determine which decoder manage which format from the js api.
Aurora.js provides a limited set of events, I will attempt to pull request on Aurora.js to improve their api but better to talk here before ; if this has some interest.