search

kmoskwiak / videojs-resolution-switcher

Resolution switcher adds the ability to select the video quality in video.js player.
https://kmoskwiak.github.io/videojs-resolution-switcher/
Other
403 stars 243 forks source link

videojs 6 support ? #88

Open kocoten1992 opened 7 years ago

kocoten1992 commented 7 years ago

Videojs 6 released, got a bunch of error in console when tried to use, might need to tweak thing a bit.

paladox commented 7 years ago

+1. It seems that the icon has stopped working. and also it seems that changing resources can show that you are loading two which makes videojs not work sometimes.

paladox commented 7 years ago

wmf is adding support for videojs 6 here https://gerrit.wikimedia.org/r/#/c/354601/1/resources/videojs-resolution-switcher/videojs-resolution-switcher.js :)

leobr29 commented 7 years ago

Hi,

I've install the support for videojs 6 but an error occured when I switch resolution. Here is my error : 

VIDEOJS: ERROR:
TypeError: setSourcesSanitized(...) is undefined
ResolutionMenuItem<.onClick()videojs...cher.js (ligne 92)
bound()video.js (ligne 23258)
on/data.dispatcher()video.js (ligne 22996)

...(this.player_, this.src, this.options_.label, customSourcePicker).one(handleSeek...

Do you have an idea of where does the error come from ?

Here is the last line execute in the plugin before crash :

return {src: src.src, type: src.type, res: src.res};

The parameters src.src, src.type and src.res seems to be correct

Best regards, Leo.

razzbee commented 7 years ago

I have forked and fixed it to support videojs 6 , also I have submitted a pull request , for now you can you my fixed version here : https://github.com/razzbee/videojs-resolution-switcher

vijeth-aradhya commented 7 years ago

@razzbee When dynamicLabel: true is provided, it (current label) doesn't change to the next resolution when changed (the video is changed of course but for example SD still remains when it's changed to HD). Could you check once again?

Can I know which specific version in video.js v6 (6.0, 6.1 or 6.2) is your code compatible with?

I'm getting the same problem with most of video.js v5 versions with the current video-resolution-switcher release. I do not know with which version the dynamicLabel works with?! :disappointed:

Currently, I'm using specific files given in the demo. This problem doesn't occur over there (try changing SD to HD, and the current label is changed accordingly).

razzbee commented 7 years ago

@vijeth-aradhya you must set the video resolution height of each label for it to work well, example for a video of resolution: 1280x720 , you will add the res attribute of : res=720 to the tag for the script to perfectly detect your selected resolution, check the demo source below

<video id="video_player" class="video-js vjs-default-skin videoPlayer" controls preload="auto"  style='max-height:480px;width:100%;max-width:100% !important;margin-top:10px;' poster="http://static.mp4u.in/static-files/previews/thumbs//300x200/oh-ho-ho-ho-hindi-5604.jpg" data-setup='{"nativeControlsForTouch": false}'>

    <source src='http://static.mp4u.in//static-files/converted/1/oh-ho-ho-ho--hindi-medium-1494653106_5604.mp4' type='video/mp4' res=240 label='240p' />

<source src='http://static.mp4u.in//static-files/converted/2/oh-ho-ho-ho--hindi-medium-1494653106_5604.mp4' type='video/mp4' res=320 label='320p' />

<source src='http://static.mp4u.in//static-files/converted/4/oh-ho-ho-ho--hindi-medium-1494653106_5604.mp4' type='video/mp4' res=720 label='720p' />    

</video>
 <script type="text/javascript">
         videojs('video_player').videoJsResolutionSwitcher({
            default: 320         });
    </script>
vijeth-aradhya commented 7 years ago

@razzbee Hmm I have set the res option for all the sources here. Am I doing anything trivially wrong though?

(I have used your files and the recent CDNs instead of the ones provided there, but the code is the same)

razzbee commented 7 years ago

everything is fine, are you using my fork? because my pull request hasn't been merged yet

On 8 July 2017 at 15:36, euleram notifications@github.com wrote:

@razzbee https://github.com/razzbee Hmm I have set the res option for all the videos here https://gist.github.com/vijeth-aradhya/b70663c50b7f4a0a6ddab59f9112f671. Am I doing anything trivially wrong though?

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/kmoskwiak/videojs-resolution-switcher/issues/88#issuecomment-313863340, or mute the thread https://github.com/notifications/unsubscribe-auth/AC3Si--n_OiaQk0y9oj5xLcnWP73MGTNks5sL6HogaJpZM4NNsae .

vijeth-aradhya commented 7 years ago

@razzbee Yes, I cloned your repo and used your files.

Can you confirm if dynamicLabel works when using (replace 6.2 with 6.1 also), https://vjs.zencdn.net/6.2/video.min.js https://vjs.zencdn.net/6.2/video-js.min.css

Thanks! :smile:

razzbee commented 7 years ago

well I never used that, you can try different versions, in my free time, I will check for the possible cause.

AleksP commented 7 years ago

Also vjs-resolution-button applied to < button > instead of parent < div >

vijeth-aradhya commented 7 years ago

[Regarding dynamicLabel]

@razzbee I have tried the current plugin release with a lot of video.js v5 releases, but in vain. I have a created a new issue for the same over here.

I will also begin to see what is causing the issue, and try to resolve it.

vijeth-aradhya commented 7 years ago

Guys, do you know which version (exact) of video.js is the current release (v0.4.2) of video-resolution-switcher compatible with?

mkody commented 7 years ago

@vijeth-aradhya I'm running in production video-resolution-switcher v0.4.2 with Video.JS v5.19.2. I'll do a quick upgrade to v5.20.1 this weekend and see if everything's okay.

vijeth-aradhya commented 7 years ago

@mkody Thanks a lot for your quick response! I'm currently setting it up with Video.JS v5.20.1 itself. Please do let me know!

SolmazKh commented 6 years ago

So does it?! (support Videojs 6) :)

Baifann commented 6 years ago

Maybe you can try to use videojs-resolution-switcher.js I changed part of the code to adapt to the new version

surikat1978 commented 6 years ago

@razzbee Can wait to fix for version 7.0? Thanks advance!

beznosd commented 5 years ago

The solution of @razzbee works well for version 7.3.0 too. https://github.com/kmoskwiak/videojs-resolution-switcher/issues/88#issuecomment-311984273

surikat1978 commented 5 years ago

@Baifann

Maybe you can try to use videojs-resolution-switcher.js I changed part of the code to adapt to the new version

Thanks! Is it works for 7.5 version ?

Baifann commented 5 years ago

@surikat1978

Thanks! Is it works for 7.5 version ?

Sorry, I haven’t used this in a new project development recently, maybe you can try it.

surikat1978 commented 5 years ago

@Baifann

@surikat1978

Thanks! Is it works for 7.5 version ?

Sorry, I haven’t used this in a new project development recently, maybe you can try it.

I already tried on 7.3 and 7.5 versions. Unfortunately this does not work ..

surikat1978 commented 5 years ago

@beznosd

The solution of @razzbee works well for version 7.3.0 too. @beznosd #88 (comment)

It also does not work for version 7.3.0 I checked on all browsers.

surikat1978 commented 5 years ago

@Baifann, @beznosd

Sorry, I haven’t used this in a new project development recently, maybe you can try it.

And what is interesting is that it does not work even for the 6.13.0 version ?!

surikat1978 commented 5 years ago

Maybe I'm doing something wrong? On version of player 5.20.5 works fine.

<video width="640" height="360" style='display:block;margin-left:auto;margin-right:auto;' id="player1" class="video-js vjs-default-skin" poster="http://site.net/poster_1.jpg" preload="none" controls data-setup='{ "nativeControlsForTouch" : false, "html5" : { "nativeTextTracks" : true },"playbackRates" : [0.7, 0.8, 0.9, 1, 1.2, 1.5]}'>
<source src='http://site.net/file_1.mp4' type='video/mp4' res='480' label='480p'>
<source src='http://site.net/file_2.mp4' type='video/mp4' res='720' label='720p'></video>
<script>
videojs('player1').videoJsResolutionSwitcher()
</script>
Baifann commented 5 years ago

Maybe I'm doing something wrong? On version of player 5.20.5 works fine.

<video width="640" height="360" style='display:block;margin-left:auto;margin-right:auto;' id="player1" class="video-js vjs-default-skin" poster="http://site.net/poster_1.jpg" preload="none" controls data-setup='{ "nativeControlsForTouch" : false, "html5" : { "nativeTextTracks" : true },"playbackRates" : [0.7, 0.8, 0.9, 1, 1.2, 1.5]}'>
<source src='http://site.net/file_1.mp4' type='video/mp4' res='480' label='480p'>
<source src='http://site.net/file_2.mp4' type='video/mp4' res='720' label='720p'></video>
<script>
videojs('player1').videoJsResolutionSwitcher()
</script>

@surikat1978

I fixed some bugs in resolution-switcher.js, you can try my project again, but it is a Vue project.

surikat1978 commented 5 years ago

I fixed some bugs in resolution-switcher.js, you can try my project again, but it is a Vue project.

Thanks! Is it can be tried on version 7.5.0? And what is Vue ?

tolew1 commented 5 years ago

Those looking for a version 6, this seems to work. https://github.com/neilhem/videojs-resolution-switcher specifically forked for version 6.

I still have a problem with dynamic label not updating when switching though.