Amphtml: amp-story: animations not working in amp-video

Created on 10 Apr 2019  路  8Comments  路  Source: ampproject/amphtml

Hi, I have a problem with APM story video (amp-video). I create a simple story with 4 pages. Pages have video inside amp-video tag, it works fine, but when I add amp-animations they often do not start, although the amp-video contains animate-in attribute.

example story - https://test-cutnut-tv.cdn.ampproject.org/c/s/test.cutnut.tv/amp/66c1cd03-2fab-4a0b-9c8e-dcfa79f20bef

Can you help me, please? what I do wrong?

image

Soon Bug stories

Most helpful comment

I found the bug but I'm not sure about the best way to fix it yet.

There's a lot of context here, but here's a short summary:
Somewhere in the animation code, it waits for the animated element layoutCallback to complete. When it's an amp-video, the code ends up in this method that, in the context of a story, sometimes never resolves because of a race condition with the internal media pool. The media pool starts loading the video earlier, and the loadstart event already fired when we start listening for it.

An easy fix would be to simply look at the videoEl.readyState to know if the video is already preloaded, but that doesn't work reliably enough.

I believe the right fix would be to improve this loadPromise helper to better support HTMLMediaElement.

All 8 comments

/to @newmuis, let me know if amp-video needs to do anything for this.

Are there any updates here?

@gmajoulet can you take a look when you get back?

I found the bug but I'm not sure about the best way to fix it yet.

There's a lot of context here, but here's a short summary:
Somewhere in the animation code, it waits for the animated element layoutCallback to complete. When it's an amp-video, the code ends up in this method that, in the context of a story, sometimes never resolves because of a race condition with the internal media pool. The media pool starts loading the video earlier, and the loadstart event already fired when we start listening for it.

An easy fix would be to simply look at the videoEl.readyState to know if the video is already preloaded, but that doesn't work reliably enough.

I believe the right fix would be to improve this loadPromise helper to better support HTMLMediaElement.

I think isLoaded in event-helper.js should be fixed to handle media elements properly same way loadPromise is special casing media elements.

Wow, thanks for finding that @gmajoulet!

@gmajoulet, You will plan to fix this bug? or we should try to fix and create a pull request?

Thanks for offering to help!
I already wrote a fix, that can hopefully be merged today: https://github.com/ampproject/amphtml/pull/21972

Was this page helpful?
0 / 5 - 0 ratings

Related issues

torch2424 picture torch2424  路  3Comments

choumx picture choumx  路  3Comments

mkhatib picture mkhatib  路  3Comments

Download picture Download  路  3Comments

aghassemi picture aghassemi  路  3Comments