1010206 - test layout/reftests/webm-video/bug686957.html is racy when using async-video

Status: RESOLVED FIXED

(Core :: Graphics: Layers, defect)

Description

[Nicolas Silva [:nical]]

With async-video, delivering frames to the compositor asynchronous with respect to the main thread's transaction. As a result, it is not guaranteed that a video frame will be composited in the composition that follows a seek (although in most cases it will, because async-video tends to deliver frames faster than main-thread transaction can.

The reftest bug686957.html seeks and takes a screenshot asap using setTimout(..., 0); which intermittently fails on Windows with async-video. giving 15ms instead of 0 in setTimeout (completely arbitray value) makes the test always pass (in theory the ImageBridgeChild thread could still catch a cold and we have no absolute guarantee that the frame will be delivered in time for the screenshot, since there is no synchronizaion between async video frames and the rest of the world, but in practice it seems good enough).

Comment 1

[Nicolas Silva [:nical]]

try push with the fix + OMTC + async-video: https://tbpl.mozilla.org/?tree=Try&rev=289959cb0a8f (we are only interested in the win7 reftests here, the other one is for another bug. It used to failed 40% of the time.

Comment 2

[Nicolas Silva [:nical]]

Attachment: Use a 15ms timeout to make the screenshot less likely to race with async-video

Comment 3

[Nicolas Silva [:nical]]

Comment on attachment 8422388 [details] [diff] [review]
Use a 15ms timeout to make the screenshot less likely to race with async-video

Review of attachment 8422388 [details] [diff] [review]:
-----------------------------------------------------------------

We need this to land the Windows OMTC work (which we want to land asap), so asking Paul for an express-review :)

Comment 4

[Paul Adenot (:padenot)]

Comment on attachment 8422388 [details] [diff] [review]
Use a 15ms timeout to make the screenshot less likely to race with async-video

Review of attachment 8422388 [details] [diff] [review]:
-----------------------------------------------------------------

::: layout/reftests/webm-video/bug686957.html
@@ +5,5 @@
>  <video src="frames.webm"
>         preload="auto"
>         id="v"
>         onloadedmetadata="document.getElementById('v').currentTime=document.getElementById('v').duration"
> +       onseeked="setTimeout(function(){document.documentElement.className = '';}, 15);"

This is terrible but was also not guaranteed to work in the first place. Throw in a comment about the fact that if this fail, we need to increase 
the timeout as another terrible workaround. 

Also, we should find a way of making this not racy, for example by looking at the frame painted count, which is available from content. Open a bug about that in Video/Audio, please.

r+ anyways because this should not block OMTC Windows.

Comment 5

[Nicolas Silva [:nical]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/ac0218ff7156

Comment 6

[Ryan VanderMeulen [:RyanVM]]

This patch had the misfortune of landing in the middle of a large bustage pileup on inbound that led to me having to revert to a last-good cset in lieu of waiting 3-4 hours for green instead. Unfortunately, that means this was backed out along with the others. If this was confirmed green on Try, it can re-land at your convenience.

https://hg.mozilla.org/integration/mozilla-inbound/rev/eb2a6f7785a2

Comment 7

[Nicolas Silva [:nical]]

relanded: https://hg.mozilla.org/integration/mozilla-inbound/rev/aac9539a4a0a

Comment 8

[Phil Ringnalda (:philor)]

https://hg.mozilla.org/mozilla-central/rev/aac9539a4a0a