Amphtml: amp-ima-video: browser freezes after pre-roll ad plays

[sanchezedgar]

What's the issue?

amp-ima-video with pre-roll ads will freeze browser tab when network conditions are poor. The tab freezes right about when the regular video starts to play after the pre-roll. You can see the moment right after the pre-roll ad that the CPU climbs up to 100%:

We tried to also replicate this issue using the regular amp-video component and could not. This appears to only occur with amp-ima-video.

How do we reproduce the issue?

https://jsbin.com/makubutuca/1/edit?html,output

1) Go to JS Bin link above
2) Turn off "Auto-run JS"
3) Control case - Press "Run with JS" - should see sample preroll & then video play.
Everything at this point is interactive and working as expected.
4) Go to network and set throttle (Slow 3g or slower)
5) Press 'Run with JS' again
6) Wait for the video to load & after pre roll plays (In most of our testing, it would take up to minute and a half to get to this point). Not able to interact with footer (or browser at all)

You can try the above steps on a regular amp-video component in the JS Bin link below to verify that it works as expected using amp-video:
https://jsbin.com/jefikolugo/edit?html,output

What browsers are affected?

Tested on Mac:
Chrome, Safari, Firefox, Edge

Which AMP version is affected?

1907091704050

[torch2424]

Hello!

Nice to e-see you @sanchezedgar 😄

And thanks for the issue!

SO I went ahead and tried to reproduce this, but I can't get the bug to happen. In both the JSBin and our examples/ima-video.amp.html example. See the gif below:

Also, in the gif, I set slow 3G before pressing play, and the video starts late because of some lag on me pressing "start recording" haha!

Let me know if I am doing something wrong. Thank you! 😄

[sanchezedgar]

Hey @torch2424 👋

We can still reproduce this bug. We can even reproduce the issue on the raw HTML example you used in the screenshot you provided:
https://github.com/ampproject/amphtml/blob/master/examples/ima-video.amp.html

A few notes:

• Sometimes a Slow 3G connection wasn't enough. Try it again with a more aggressive throttle.
• It can sometimes play through with no freezing. Reloading and trying a few times should cause it to freeze.
• In the /examples/ima-video.amp.html file, adding autoplay to the first video tag helped in causing the bug to occur.

[torch2424]

@sanchezedgar Awesome! Will try the additional steps above 😄

[torch2424]

@sanchezedgar

So By using a custom network profile, 250 down, 50 up, 300ms latency I was able to reproduce the bug 👍 And yeah totally it pegs the Page / CPU after the ad is finished.

Getting the performance profile was very difficult since, the page is completely frozen. But I finally got lucky, and got a performance profile. You can download it here, and it's a 42MB file haha! And the website I used was the ima-video amp.dev example.

For the "why" this is happening, the profile shows the following:

(anonymous) @ video-manager-impl.js:1494 points to our analytics handling code for all of our video players: https://github.com/ampproject/amphtml/blob/master/src/service/video-manager-impl.js#L1491

And which you can see it constantly being called by all of the events being fired. 😢

@prateekbh and @zhouyx Suggested because the network is poor, and we are listening to the 'playing' event, instead of the 'play' event, the video must be constantly stopping and starting, thus continually firing this event over and over.

Thank you @sanchezedgar for opening this issue! Super glad you caught this, will work on a fix ASAP 😄

cc @alanorozco because this probably affects all AMP video 👍

[zhouyx]

haha only learnt about the playing event yesterday 😄glad that we figured out the reason : )
Add @cvializ for ideas on how to measure the time played with poor network connection.

[torch2424]

@zhouyx Looked into the 'playing' event, and if I rename it to 'play', the bug is fixed (probably because it is not called as often).

However, this could lead to completely different analytics / behavior. Perhaps a debouncing solution is the way to go? 🤔

EDIT: I could not reproduce this on a normal <amp-video> element, specifically on the amp.dev example. But perhaps it is missing some analytics hooks?

[cvializ]

I think the approach we are considering is to change the event to play from playing and communicate it. See #23509

[zhouyx]

Thanks @cvializ. Given this bug we should prioritize #23509
My question is on the SECONDS_PLAYED implementation. Haven't looked into the implementation, but does it depends on the playing event?

[cvializ]

Good question. So SECONDS_PLAYED uses entry.getPlayingState() !== PlayingStates.PAUSED and the playing state is updated with playing. It looks like we might want to keep playing for SECONDS_PLAYED if a video stopping because it ran out of buffer triggers pause.

[zhouyx]

I see. My concern is that if listening to the playing event is going to be too expensive with poor network connection. @torch2424 proposed something like debounce the event a little bit. However that's going to make the SECONDS_PLAY not so accurate.

@torch2424 Can we look at the analytics trigger in this file. Let's figure out if it's Analytics PLAY or SECONDS_PLAY or both that cause the freeze.

@cvializ please let us know if there are other analytics trigger which is listening to the playing event.

Thanks

[cvializ]

Anything that interacts with the state changed by video-manager-impl#videoPlayed_ is affected by the playing event. It seems like sometimes playing is what we want, and sometimes it isn't, so I don't think we can replace it with play in all cases.

I agree that throttling the network requests is probably a good idea since in poor network conditions, emitting more network requests probably makes the problem worse. But throttling the event triggering is going to cause state to get out of sync.

https://github.com/ampproject/amphtml/blob/cfd87a5663645c008cdee8d64ca2cb57ab9d2b0c/src/service/video-manager-impl.js#L566-L598

[slsimon]

Hi @torch2424 and @alanorozco - any updates on getting this fixed? We are getting more reports on it occurring so just wanted to check in

[sanchezedgar]

Hey @zhouyx @cvializ

Wanted to bump, hoping for any new information (especially since @torch2424 focus is shifted elsewhere). I tried and was unsuccessful in determine if PLAY and/or SECONDS_PLAYED cause the freeze.

Thank you all!

[lannka]

@powerivq could you PTAL?

[zhouyx]

@micajuine-ho I find this to be a good issue to dig into. Let's first test if it's the playing or the seconds_played here that's causing the freeze.

[micajuine-ho]

Update: The issue was actually a race condition caused by our ima-player-data, where analyticsEvent() would continuously get called by our percentage-played tracker.

Going to merge the fix as soon as I check all of the other video players and confirm that the analytics are working as intended.

[micajuine-ho]

To shed a little more light onto the problem, here's what was happening:

The video-manager would receive the VideoEvents.LOADED signal from the ima-video to say that the video was ready to be played. However, it was not guaranteed that the actual duration of the ima-video had been updated.
So when we throttled the network, the LOADED event would fire, but the duration would be NaN.
The NaN duration was propagated down to here:

calculate() {
    // calculations
    timer.delay(calculate, NaN);
   // call analytics event  
}

which caused the browser to crash, because it was being called every so frequently.

The fix applied was to not allow calculations before the duration was actually loaded (aka not 1s).