1041416 - [OMTC] White frames flash in the middle of video playback for some (webm) videos

Status: VERIFIED FIXED

(Core :: Audio/Video, defect)

Description

[w0wkin]

User Agent: Mozilla/5.0 (Windows NT 6.3; Win64; x64; rv:33.0) Gecko/20100101 Firefox/33.0 (Beta/Release)
Build ID: 20140717170629

Steps to reproduce:

https://www.youtube.com/watch?v=1BGxLOIXe2g

You need to open http://joyreactor.cc/3996 for example


Actual results:

some video is blinking with white frames, it so called strobo or flashgun effect.
This could cause epileptic attack, but may be not. At least it drives me crazy. :)
The problem persists at least a month or even more with 33 branch. 


Expected results:

video must run smoothly

Comment 1

[w0wkin]

http://joyreactor.cc/post/1446895 sample, direct link to open with 33alpha branch.

Comment 2

[Paul Silaghi, QA [:pauly]]

Confirmed 33.0a1 (2014-07-20) Win 7 x64.

Comment 3

[:Gijs (he/him)]

[Tracking Requested - why for this release]: Regression with pretty bad consequences, so I think we should track this for 33 - potentially earlier if we find out it regressed before 32.

Comment 4

[Alice0775 White]

setting layers.offmainthreadcomposition.enabled = false helps

Comment 5

[Alice0775 White]

Regression window with forcing layers.offmainthreadcomposition.enabled= true and layers.async-video.enabled = true
Good:
https://hg.mozilla.org/integration/mozilla-inbound/rev/fda65b2f990a
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:32.0) Gecko/20100101 Firefox/32.0 ID:20140517200633
Bad:
https://hg.mozilla.org/integration/mozilla-inbound/rev/62d1670e37d9
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:32.0) Gecko/20100101 Firefox/32.0 ID:20140517201828
Pushlog:
http://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?fromchange=fda65b2f990a&tochange=62d1670e37d9

Suspect: Bug 1009616

Comment 6

[w0wkin]

With latest build looks OK now.

Comment 7

[Alice0775 White]

It still happen for me
https://hg.mozilla.org/mozilla-central/rev/a91ec42d6a9c
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:34.0) Gecko/20100101 Firefox/34.0 ID:20140724030201

Screen capture: http://youtu.be/0Pu_JebbD44

Comment 8

[Nicolas Silva [:nical]]

The fix for bug 1015718 needs to be implemented with the D3D9 equivalent texture type. I Suspect that this is what's missing here.

Comment 9

[Nicolas Silva [:nical]]

Attachment: Avoid race condition when between the deserialization and the destruction of DXGI handles whith D3D9

Comment 10

[Bas Schouten (:bas.schouten)]

Comment on attachment 8462592 [details] [diff] [review]
Avoid race condition when between the deserialization and the destruction of DXGI handles whith D3D9

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

::: gfx/layers/d3d9/TextureD3D9.cpp
@@ -917,5 @@
>  DXGITextureHostD3D9::Lock()
>  {
>    MOZ_ASSERT(!mIsLocked);
> -  DeviceManagerD3D9* deviceManager = gfxWindowsPlatform::GetPlatform()->GetD3D9DeviceManager();
> -  if (!deviceManager) {

You removed this check, that's not a good idea.

@@ +957,2 @@
>  
> +  mIsLocked = !!mTextureSource || OpenSharedHandle();

This is a little confusing, just have mIsLocked call OpenSharedHandle(), it will early return when !!mTextureSource anyway, you might even rename OpenSharedHandle to something like EnsureTextureHost, but I'll leave that up to you.

Comment 11

[Nicolas Silva [:nical]]

(In reply to Bas Schouten (:bas.schouten) from comment #10)
> Comment on attachment 8462592 [details] [diff] [review]
> Avoid race condition when between the deserialization and the destruction of
> DXGI handles whith D3D9
> 
> Review of attachment 8462592 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> ::: gfx/layers/d3d9/TextureD3D9.cpp
> @@ -917,5 @@
> >  DXGITextureHostD3D9::Lock()
> >  {
> >    MOZ_ASSERT(!mIsLocked);
> > -  DeviceManagerD3D9* deviceManager = gfxWindowsPlatform::GetPlatform()->GetD3D9DeviceManager();
> > -  if (!deviceManager) {
> 
> You removed this check, that's not a good idea.

Instead I am checking for GetDevice() which is pretty muchequivalent, although there is one confusing thing that I meant to ask you about and then forgot: In some places in this file we get the device from the Compositor, and in other we get it from gfxPlatform. all of the D3D11 code uses the device in gfxPlatform while the GL code might have different contexts for different compositors.

What do we want to do for D3D9? always use the same D3D device and always get it from gfxWindowsPlatform? I don't know the implications.

> 
> @@ +957,2 @@
> >  
> > +  mIsLocked = !!mTextureSource || OpenSharedHandle();
> 
> This is a little confusing, just have mIsLocked call OpenSharedHandle(), it
> will early return when !!mTextureSource anyway, you might even rename
> OpenSharedHandle to something like EnsureTextureHost, but I'll leave that up
> to you.

I wanted to make it apparent that we are only deserializing the handle if we don't have a TextureSource. But I agree that it is a bit awkward.

Comment 12

[Nicolas Silva [:nical]]

Attachment: Avoid race condition between the deserialization and the destruction of DXGI handles whith D3D9

Comment 13

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/22a61b3f1390

Comment 14

[Nicolas Silva [:nical]]

Comment on attachment 8463420 [details] [diff] [review]
Avoid race condition between the deserialization and the destruction of DXGI handles whith D3D9

Approval Request Comment
[Feature/regressing bug #]:
[User impact if declined]: Windows OMTC blocker: white flashes during video playback.
[Describe test coverage new/current, TBPL]:
[Risks and why]: Very low, the same fix for the D3D11 backend was already uplifted without issues, easy to backout
[String/UUID change made/needed]:

Comment 15

[Sylvestre Ledru [:Sylvestre]]

Nical, you confirm that this bug does not affect 32?
Thanks

Comment 16

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/aee4463ef99d

Comment 17

[[SV Manager] Florin Mezei, QA (:FlorinMezei)]

Easily reproduced the initial issue with http://joyreactor.cc/post/1446895, on Windows 7 x64, using Nightly from July 22nd (ftp://ftp.mozilla.org/pub/mozilla.org/firefox/nightly/2014/07/2014-07-22-03-02-01-mozilla-central/).

The issue no longer reproduced with:
- Firefox 33 Beta - BuildID=20140902214533
- latest Firefox 34 Aurora - BuildID=20140903004002

Not closing the issue right now, to see if anyone still encounters this problem.

Comment 18

[[SV Manager] Florin Mezei, QA (:FlorinMezei)]

Since there is no more feedback on this and I still cannot reproduce the issue on latest Firefox 33 Beta 5 (BuildID: 20140918174809) or latest Aurora 34 (BuildID: 20140922004004) I'm closing this bug.