playbackRate support #1132
playbackRate support #1132
Conversation
| var rate = this.player_.tech.playbackRate() + 'x'; | ||
| return vjs.Component.prototype.createEl.call(this, 'div', { | ||
| className: 'vjs-playback-rate vjs-menu-button vjs-control', | ||
| innerHTML:'<div class="vjs-playaback-rate-value">' + rate + '</div>' |
|
Great stuff, thanks! Digging into it now. |
| player.on('loadstart', vjs.bind(this, function(){ | ||
| // hide playback rate controls when they're no playback rate options to select | ||
| if (( player.tech.features && player.tech.features['playbackRate'] === false) || | ||
| this.player_.options_.playbackRates.length === 0) { |
There was a problem hiding this comment.
Any var with an underscore at the end is a private var, so you should really never have two in the same reference. i.e. this should be this.player().options().playbackRates.length === 0.
|
Great job figuring out all the pieces for getting this to work! If you want to make the suggested changes I should be able to pull it in. @jnwng since you were looking into implementing this, does it do what you were hoping it would? |
|
yeah, i forked @H1D's work here and have the bare playback API working as expected. we've been using this internally already as the changes we added for playbackRate support were merged on our own fork a while ago, just took a while to come around to getting this upstream. we use an alternative front-end component for access the playbackRate, so i haven't had a chance to look at the one presented here. i do have a few test additions that i've made on the fork, which i'll work with @H1D to get merged into his fork before bringing it all back here to master. |
…ckrate-test Conflicts: test/unit/media.html5.js
|
@H1D have an open pull with an additional playbackRate test to add to this |
|
Awesome, thanks. |
Adding test for playbackRate API functionality
|
cool! |
|
Great, thanks! |
|
@heff I did what you suggested. Tests still fails =( |
Fix breaking tests for playback rate
|
damn. Tests are passing but UI is broken. Do not merge this |
| }; | ||
|
|
||
| vjs.PlaybackRateMenuButton.prototype.playbackRateSupported = function(){ | ||
| return |
There was a problem hiding this comment.
Got a little too fancy here. :) Should remove the line break after return.
|
Sorry about that... I made some comments that I think should fix it. |
|
This is now fixed and merged into master. I changed it so the playback rates don't show by default because I'm not sure if playback rate is something that everyone wants in their player. This at least lets us see how popular it becomes before making it the default. To add playback rates, supply the playbackRates option. e.g. |
|
Hello. |
|
No, it will be in 4.6.0, which will be released in the next few days. |
|
Thank you. |
|
That's great! Thx for finishing it. |
this fixes #840
But unfortunately I broke tests and don't know how to fix them =(
at file:///Users/user/proj/video.js/test/unit/player.js:41: The "playbackRate" method is not available on the playback technology's API