googleads / videojs-ima

IMA SDK Plugin for Video.js
Apache License 2.0
450 stars 284 forks source link

Unexpected invocation to startLinearAdMode #796

Closed cristian-atehortua closed 4 years ago

cristian-atehortua commented 5 years ago

Hi, there is a bug with the PlayerWrapper class.

The bug occurs when you try to load a pre-roll ad that takes more than the time specified in the prerollTimeout setting to load.

The problem is that when the adtimeout event is triggered and then the adsready event is triggered, the pre-roll ad is displayed but UI is not updated to reflect that. For example, the vjs-ad-playing is not added to the player and the content playback is not resumed after pre-roll is ended.

The bug is caused because in the method onAdBreakStart the videojs-contrib-ads startLinearAdMode function is called, even if the adtimeout event was fired before.

This causes a WARN: 'Unexpected startLinearAdMode invocation (Preroll)', so this is an unexpected behavior for videojs-contrib-ads.

An easy way to reproduce this is set prerollTimeout in contribAdsSettings to a small value like 10.

arnaudcasame commented 5 years ago

Hi,

Were you able to reproduce the issue using vastLoadTimeout field of the videojs-ima plugin? Can you also provide the ad tag and the sample app you were using when you encountered the issue?

cristian-atehortua commented 5 years ago

I am having the issue using any placement Id from AppNexus ad server. An important thing that I didn't mention before is that I'm using autoplay feature by using videojs .play() method on player ready event:

myVideoJsPlayer.ready(function () {
  const playPromise = myVideoJsPlayer.play();
  // do things with the promise
});

I want to be emphatic in that problem only occurs when adtimeout event arrives before adsready event.

arnaudcasame commented 5 years ago

Hi,

Thank you for providing additional insights with regards to the issue. As the issue is with the videojs-contrib-ads plugin, I would recommend you to report it on their github issue tracker.

cristian-atehortua commented 5 years ago

Nope, the bug is with videojs-ima plugin because videojs-contrib-ads documentation says that you should call startLinearAdMode function only in specific scenarios and this project is not checking that scenarios. According to videojs-contrib-ads plugin integrator docs:

That validations are not present in PlayerWrapper class.

arnaudcasame commented 5 years ago

Hi

Can you share a screenshot of the issue and a walking skeleton of the sample app you were using when you encountered the issue?

dilburt commented 5 years ago

I'm having this same issue which I reported back on 3/6. Christian has done an exceptional job of documenting this problem with detailed descriptions of where it's happening and what is causing it so I don't understand how a screenshot will provide additional value. This is causing problems for a lot of publishers and needs to be fixed. Has Sean or Yuri had a chance to review this?

Thanks,

D

arnaudcasame commented 5 years ago

Hi,

The screenshot is to be sure that we're testing the same issue and we need a walking skeleton of the project capable of reproducing the behavior so that we can investigate further. I'm asking these info cause it seems that I need to modify our sample app to reproduce the behavior.

mysuf commented 5 years ago

I think that main issue is lower timeout in contrib-ads. You cannot handle it then. Ima uses one timeout value, but contrib two. You should sync one value for both or make ima's lower. When you set 10ms preroll on contrib-ads and ima uses default 8sec, then you are facing race condition.. Ima throws timeout and its covered by skipLinearAdMode, but nothing covers opposite case..

GavinThompson commented 5 years ago

We've been encountering the same bug. @cristian-atehortua has done a great job at tracking down why the problem might be happening.

@cristian-atehortua any luck on your end solving this? We're looking at cutting a branch to dig into a solution.

For us, and I'm sure many publishers, this bug is mission critical. For publishers that use video-js in digital-signage-like products, this is causing a lot of headaches.

cdatehortuab commented 5 years ago

Hi, @GavinThompson. In my case I can fix my specific problem adding this workaround to my code. Check it out! I could be good for you, too.

/* Work around (started pre-roll after timeout event) */
const AD_PLAYING_CLASS = 'vjs-ad-playing';
const EVENTS = {
  AD_STARTED: 'ads-ad-started',
  ADS_READY: 'adsready',
  AD_TIMEOUT: 'adtimeout'
};

const onAdCompleted = () => {
  player.removeClass(AD_PLAYING_CLASS);
};

const onAdStarted = () => {
  player.addClass(AD_PLAYING_CLASS);
  player.ima.addEventListener([
    imaSdk.AdEvent.Type.COMPLETE,
    imaSdk.AdEvent.Type.SKIPPED
  ], onAdCompleted);
  player.off(EVENTS.AD_STARTED, onAdStarted);
};

const onAdsReady = () => {
  player.on(EVENTS.AD_STARTED, onAdStarted);
  player.off(EVENTS.ADS_READY, onAdsReady);
};

const onAdTimeout = () => {
  player.on(EVENTS.ADS_READY, onAdsReady);
  player.off(EVENTS.AD_TIMEOUT, onAdTimeout);
};

player.on(EVENTS.AD_TIMEOUT, onAdTimeout);
/* End work around */
GavinThompson commented 5 years ago

@cdatehortuab you are the best, thanks for this! Going to implement this afternoon and see how it works; this looks like a great solution. Thank you!

*Edit: Damn unfortunately did not work for our project; may have given me a different starting point though

cdatehortuab commented 5 years ago

What is the problem that you have? The above solution only works around on adding vjs-ad-playing class to player when the issue occurs. Maybe you can edit some things to make it work for your case.

arnaudcasame commented 5 years ago

Hi @cristian-atehortua ,

I set the prerollTimeout key to 10, added the code below to the advanced videojs-ima sample, no WARN: 'Unexpected startLinearAdMode invocation (Preroll)' message was logged in the console and the content playback resumed successfully. The behavior seems to be with your implementation. Can you provide a sample app capable of reproducing the behavior?

this.player.ready(() => {
    const playPromise = this.player.play();
    // do things with the promise
  });
renandecarlo commented 5 years ago

I'm having this problem too.

It seems the problem does only occur when using autoplay and a HLS source. Using a .mp4 for instance did resume playback after the ad. It also does not happen when using the default "preroll" ad tag included in the examples. I'm using a real one, but in test mode.

Also, when it takes too long to receive an ad, it triggers the adtimeout, however, if it receives the ad afterwards, it still calls the startLinearAdMode(), which for some reason does not resume the content after the ad ended.

If you increase the prerollTimeout, it stops triggering the Unexpected startLinearAdMode() error. And it triggers the Starting ad break and Ending ad break messages. However, in my case, it still does not resume the content.

renandecarlo commented 5 years ago

I've managed to get it working in two ways.

  1. Downgrading videojs-contrib-ads to version 6.0.0. I can confirm this problem started happening somewhere between 6.0.1 and 6.1.0.

  2. I've also managed to get it working by adding a check on SdkImpl.prototype.onPlayerReadyForPreroll to check if the player is not in resuming state, as it seems videojs-contrib-ads ask for it now, and also implicitly calling a resume/play function on videojs-contrib-ads endLinearAdMode() functions. However, this last option wouldn't play ads if they loaded after the player was already resuming (unless you increased your prerollTimeout). And it seemed to happen quite a lot, so I stick with the first method.

There was also a secondary problem which seemd to happen. When the HLS was resuming from ads, it seemed to get stuck on loading sometimes. The main .m3u8 playlist was being fetched, but not the .ts segments.

I've managed to fix it by adding a play event on adend. As such:

player.on('adend', function(e) {
    player.play();
})`

Workaround 1: Use https://cdnjs.cloudflare.com/ajax/libs/videojs-contrib-ads/6.0.0/videojs-contrib-ads.js instead.

Workaround 2 (recent versions):

In videojs.ima.js, change:

SdkImpl.prototype.onPlayerReadyForPreroll = function () {
  if (this.autoPlayAdBreaks) {

To

SdkImpl.prototype.onPlayerReadyForPreroll = function () {
  if (this.autoPlayAdBreaks && !player.ads.inAdBreak() && !player.ads.isContentResuming()) {

In videojs-contrib-ads.js, wherever there is a endLinearAdMode() function, add this to the end:

var _this = this;
  this.afterLoadStart(function () {
  _this.resumeAfterNoPreroll(player);
});

You may want to increase your prerollTimeout in player.ima options too.

arnaudcasame commented 4 years ago

Hi,

I’m closing this issue because it has been inactive for more than a month. Please reopen if you still encounter this issue with the latest stable version

Thank you!

valse commented 3 years ago

Workaround 1: Use https://cdnjs.cloudflare.com/ajax/libs/videojs-contrib-ads/6.0.0/videojs-contrib-ads.js instead.

OMG it works! I'm dealing with this issue (player doesn't play after ad) from years... any downside to this workaround?