videojs / videojs-contrib-ads

A Tool for Building Video.js Ad Plugins
http://videojs.github.io/videojs-contrib-ads/
Other
384 stars 257 forks source link

Setting source on adend leads to weirdness #305

Closed ghost closed 6 years ago

ghost commented 6 years ago

We use video.js for streaming. Depending on browser support, we pick HLS or MP4 (not DASH, our streaming server serves a normal looking MP4 file) Because we keep only about the last 10 seconds of the streaming buffer on the server, playing a 15 second ad causes the following scenario:

We attempted to fix this by setting the source anew (with a timestamp in the URL to force a reload, what we're already doing whenever playback is resumed for MP4 streams) in an adend handler.

This lead to a weird race condition with up to 3 play events being fired, the player eventually recovers after 10-15 seconds, but this takes too long. We fixed this for now by using setTimeout with 0 seconds to push the call to whatever contrib-ads does after triggering the adend event.

incompl commented 6 years ago

The most success we've had providing a good experience for ads in live streams is to play the stream muted in the background while the ad plays in a container. The shouldPlayContentBehindAd check manages this: it returns true if the duration is Infinity (live stream) or if it's on a platform that doesn't support multiple video elements. This also means you can respond to ID3 metadata in the stream during ads if needed. Is this an option in your case?

ghost commented 6 years ago

I will test this, but I'd be surprised if it works as expected. Because we use a normal MP4 container for livestreaming (as the default option, due to compatibility issues with some devices), videojs does not detect it as a livestream - just as a very very long video. I've previously tried to override the prototype of the function that returns the video duration to always return Infinity and while that fixes automatic live display and the likes, it also breaks MP4 playback after a short while.

incompl commented 6 years ago

Ahh interesting. I think what we need for you is a way to explicitly opt-in to the live behavior. I made a branch that adds a live option to do just that. Would you be willing to give it a shot?

https://github.com/videojs/videojs-contrib-ads/tree/live-option

ghost commented 6 years ago

I'll give that a go on Monday, thanks for putting the work in for us!

ghost commented 6 years ago

We experienced a ton of other issue, including not being able to call play after changing source and lots of playback issues. We aren't sure if it's related to contrib-ads or videojs-ima, but we've resorted to just disposing of a player that played an ad and loading a fresh one instead. I built a localStorage check into it so you can trigger the bug when that's released, I'll let you know.

incompl commented 6 years ago

Would be really curious to learn if that approach works. Is there anything else I can do for you, or shall we close this issue for now?

ghost commented 6 years ago

The remounting does indeed work! If you want to have a look over the source, at present it still has source maps intact. The local storage flag I mentioned is localStorage.triggerTerribleAdBug.

You can find disposing in player.tag / on("before-unmount"). It's also what switches the player on our front page. We trigger it from adend and adserror if the local storage variable isn't set.

Here's out stream list, can't just link to one because they come and go

incompl commented 6 years ago

Awesome to hear you found a solution. If you want to try to keep doing this without remounting the player let me know, I'm happy to continue trying to make that work.

sunilkumarjena21 commented 6 years ago

Hi @incompl ,

We are implementing Google IMA ads at 'cuechange' metadata event listener. After Lot of try we are unable to get shouldPlayContentBehindAd integrate properly with our implementation.Please note this implementation is for live stream.We want suggestion from your side,what should be the best approach to implement this.Some code snippet with the correct position to call will do our job little easier.

Any help will be highly appreciated!

Attached code for your kind perusal.

Thanks Sunil

incompl commented 6 years ago

@sunilkumarjena21 Could you open this as a new issue? The issue you posted in is closed. Thanks!

sunilkumarjena21 commented 6 years ago

Sure..It has been created under new issue #344 Thanks