Add support for videojs 5.x

[wmcmurray]

NOTE: I copied the relevant changes from this pull request #86 and I fixed what wasn't working. I did this because I needed a working version ASAP for my colleagues.

I also did this pull request on top of my other pull request (#105) because I needed those changes applied.

...So, it works, but I've seen some strange stuff that (I believe) are not related to this plugin :

• The player's timeupdate event is not triggered on flash ads, which cause the remaining time to stay stuck at -0:46 in the demo page (it's the duration of the video that will play after the ad).
• The full screen button is not aligned right because when there is no timeline, the button seems to take it's place (which is different from the UI of the 4.x version).
• There is also a double "skip ad" button in on of the sample ads of the demo, but that was there before :)

I hope you'll merge this soon, thanks !

Will fix issue #74

[carpasse]

@wmcmurray and @kahwee thanks a lot for your PRs. I am very sorry it took us so long to take a look at them.

[carpasse]

Will this component work on videojs 4.x??

[wmcmurray]

We could still use the init property instead of constructor because it is still supported even if it is deprecated (as described in the second exemple here).

But other parts of this PR will break with videojs 4.x anyway (like the videojs.getComponent method)... Is that okay if this plugin support only 5.x ? An other plugin I need to use seems to support only the latest 5.x version, that's why I had to add support for 5.x in your plugin 😉

[carpasse]

@wmcmurray The think is that we need to support 4.x for a few months still.
We could create a VJS5 branch and try to keep them both updated until we find a better solution.
Would that work for you?

[wmcmurray]

@carpasse Ok then I suggest that you merge my other pull request #105 (that we have closed) in the master branch, it contains fixes that make the debug page usable, then I hope those changes will disappear from this pull request (hehe) as they will already be merged, then you can make a branch for 5.x and merge this pull request inside.

After that, I'll probably add more features support in the master branch, that we'll merge into 5.x branch when needed.

Sounds good ?

[carpasse]

@wmcmurray yep. Lets do it like that I have already reopened the PR

[alex-phillips]

Very interested in seeing this get merged as we need VAST/VPAID support in our version 5 player.

I've found something while testing out this branch that if an a swf ad is to be played, the video is not paused properly as I hear the ad audio as well as the video audio playing. This is not the case when the ad is an mp4 source.

I'd be more than happy to dig in and help resolve the issue, but figured I'd mention it here first in case you can find / solve it faster than me.

EDIT: not sure if it matters, but the original content we're playing before the ad begins is an RTMP stream.

EDIT 2: also, just found that vast.contentStart never gets triggered as a result.

[carpasse]

@alex-phillips can you reproduce this issue the latest release of the plugin?

[alex-phillips]

@carpasse Actually, yes. My mistake. This is an issue in the current master branch as well.

[alex-phillips]

@carpasse I've opened a PR for this fix. From what I can tell, it's a race condition where the VAST code is calling pause on the player but the content has not actually loaded in yet. This PR adds an interval that does not proceed until the player is actually paused (#111).

Hopefully this can get merged in and @wmcmurray can rebase to include this fix as I'll be using his branch since our player is version 5.

[wmcmurray]

So... I rebased this branch and fixed some new bugs.

About this fix that fixed issue #73, I had to remove it because videojs 5.x seems to prevent plugins to mess with the player tech (as explained here ). I tried to use the player.reset() method (doc here) but in order to do so I had to transform the playerUtils.restorePlayerSnapshot into an async function, which crashed a ton of unit tests... I tried to patch them all but it just didn't work.

Then I finally realized that in fact, there was no bug anyway. As specified here if you pass the type to the .src function (which we do in the snapshot restore function), the player will find the proper tech to play the video, if the current tech can play the video it will use it. That's why I think the tech stays to flash after the flash ad given in bug #73 , because if the following video is an MP4, it can be played with the flash tech so there is no need to return to HTML5.

@carpasse could you do the 5.x branch soon ? :)

[kahwee]

Sure, no problem. Thanks!

[whatvn]

@wmcmurray did you test this fix with mobile browser like Safari on Iphone and Chrome on Android/Iphone. I cannot make it work on mobile browser, although there's no errorr at all, the plugin initialized and then freeze, video playback cannot start too.

[wmcmurray]

@whatvn Oh ! No I haven't tested it on mobile browsers :/

[Fetz]

Hi everyone, I will base in your commits and update the scripts so the plugin can work in both videojs 4.x as well 5.x.

branch that I will start working https://github.com/MailOnline/videojs-vast-vpaid/tree/videojs-5x

[whatvn]

@Fetz as I mentioned in last comment, v5 branch of this project does not work on mobile, I double checked with safari and android on iOS.
The reason is, this plugin relies on "play" event of videojs, it wait for player to trigger player.trigger("play") and start tryToPlayPrerollAd, but for some unknown reason, on mobile device, this event is not fired by videojs.

I did a simple trick to make it works. I manually trigger player.trigger("play") after initializing vastClient, then Ads and media can be played fine.
Sometimes ads is stuck but media still can play. Without that trick, player hangs with no Ad and video playing.

[wmcmurray]

Cool, thanks @Fetz and @whatvn !

[carpasse]

@wmcmurray We have finally added support for videojs 5.x including your changes. If you are not happy with it please reopen the PR.