Closed Bug 1317131 Opened 3 years ago Closed 3 years ago

wmode=direct (NHK Realtime News) Flash Video would not play since Nighyly52.0a1(2016-Nov-12)

Categories

(Core :: Graphics, defect, P1)

52 Branch
x86
Windows 10
defect

Tracking

()

RESOLVED FIXED
mozilla53
Tracking Status
firefox49 --- unaffected
firefox50 --- unaffected
firefox51 --- unaffected
firefox52 + fixed
firefox53 + fixed

People

(Reporter: alice0775, Assigned: kechen)

References

Details

(Keywords: regression)

Attachments

(2 files, 2 obsolete files)

[Tracking Requested - why for this release]: Regression Flash Video would not play 

NHK Realtime News video would not start.

If disable HWA, it works as expected.


Reproducible : 100%

Steps To Reproduce:
1. Open http://www3.nhk.or.jp/news/realtime-3/  (the URL may be expired soon)
   (Alternative: Open http://www3.nhk.or.jp/news/ and Click [LIVE] at the right side)



Actual Results:
Upper video(Flash Player) is black. Not playback.
(Lower one is HTML5, and no problem)

Expected Results:
Upper video(Flash Player) will playback automatically when page is loaded.


Regression Window:
https://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?fromchange=2716f83ffcc0d9adbe37cc7bd4b06a54e4a81abd&tochange=ce4e1e1d0bff70ea540d62eca9e0720853610700


Suspect: 	e9db19aed6c0	Kevin Chen — Bug 1160157 - Use AutoTextureLock to manage the shared resource's mutex. r=dvander
Flags: needinfo?(kechen)
Summary: NHK Realtime News Flash Video would not play since Nighyly52.0a1(2016-Nov-12) → wmode=direct (NHK Realtime News) Flash Video would not play since Nighyly52.0a1(2016-Nov-12)
Attached file testcase wmode=direct
Reproduced on current release Flash Player 23.0.0.207 as well as beta 24.0.0.154.
Same problem is back!!! (See old Bug 1311952).

Please provide automatetest, Otherwise, any patch should not be landed!!!!
See Also: → 1311952
I confirmed via local build that Bug 1160157 causes the problem:
Last Good: 2716f83ffcc0
First Bad: e9db19aed6c0

Regressed by: 	e9db19aed6c0	Kevin Chen — Bug 1160157 - Use AutoTextureLock to manage the shared resource's mutex. r=dvander
kevin, please take a look.
Assignee: nobody → kechen
Priority: -- → P1
The reason of this error is that I didn't hold the AutoTextureLock object when constructing it in these two cases.

This problem can be fixed by the patch. I will mark r? when the try run is completed.
Flags: needinfo?(kechen)
Comment on attachment 8810322 [details] [diff] [review]
Fix the incorrect AutoTextureLock lifecycle.

Hello David, can you help me to review this patch. The detail about this patch is in comment 7.

The try run looks good[1], there are two timeout in Windows 7 VM debug mochitest, but I think it is not related to this patch.

[1] https://treeherder.mozilla.org/#/jobs?repo=try&revision=83de1d5ee17020fbd0e9de82ce04e58b1a1bdd3f
Attachment #8810322 - Flags: review?(dvander)
Comment on attachment 8810322 [details] [diff] [review]
Fix the incorrect AutoTextureLock lifecycle.

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

::: dom/plugins/ipc/D3D11SurfaceHolder.cpp
@@ -71,5 @@
>      return false;
>    }
>  
>    {
> -    AutoTextureLock(mutex, hr);

r=me on changing "class AutoTextureLock" to "class MOZ_RAII AutoTextureLock". Though the static analysis doesn't run on Windows :( that would have caught this.
Attachment #8810322 - Flags: review?(dvander) → review+
Carry r+ from comment 9.
Attachment #8810322 - Attachment is obsolete: true
Attachment #8811549 - Flags: review+
Keywords: checkin-needed
Blocks: 1312988
Blocks: 1317984
Approval Request Comment
[Feature/regressing bug #]: Bug 1160157
[User impact if declined]:
  Some shared source features on Windows platform won't work(e.g., Some flash videos can't be displayed).
[Describe test coverage new/current, TreeHerder]:
  None
[Risks and why]: 
  The risk is low, it add a local variable to hold the ref.
[String/UUID change made/needed]:
  None
Attachment #8811643 - Flags: review+
Attachment #8811643 - Flags: approval-mozilla-aurora?
applying 0001-Bug-1317131-Fix-the-incorrect-AutoTextureLock-lifecy.patch
patching file gfx/layers/d3d11/TextureD3D11.cpp
Hunk #2 FAILED at 1230
1 out of 2 hunks FAILED -- saving rejects to file gfx/layers/d3d11/TextureD3D11.cpp.rej
patch failed, unable to continue (try -v)
patch failed, rejects left in working directory
errors during apply, please fix and qrefresh 0001-Bug-1317131-Fix-the-incorrect-AutoTextureLock-lifecy.patch

has problems to apply (or so we need the uplift patch here ?
Flags: needinfo?(kechen)
Keywords: checkin-needed
Tracking 53+ for this issue - Can we get some automation to cover it so it doesn't happen again as is suggested in Comment 4?
Attachment #8811549 - Attachment is obsolete: true
Attachment #8811643 - Attachment description: [Uplift] Fix the incorrect AutoTextureLock lifecycle. → Fix the incorrect AutoTextureLock lifecycle.
Hello Carsten,

The conflict is caused by [1] which is landed several days ago.
Please apply Attachment #8811643 [details] [diff]. Thank you.

[1] https://hg.mozilla.org/mozilla-central/rev/86e2aea8417c
Flags: needinfo?(kechen)
(In reply to Marcia Knous [:marcia - use ni] from comment #13)
> Tracking 53+ for this issue - Can we get some automation to cover it so it
> doesn't happen again as is suggested in Comment 4?

Yes, I will try to add an extra test for this bug.
(In reply to Kevin Chen[:kechen] (UTC + 8) from comment #14)
> Hello Carsten,
> 
> The conflict is caused by [1] which is landed several days ago.
> Please apply Attachment #8811643 [details] [diff] [diff]. Thank you.
> 
> [1] https://hg.mozilla.org/mozilla-central/rev/86e2aea8417c

Do you forget checking-needed flag?
Flags: needinfo?(kechen)
Flags: needinfo?(kechen)
Keywords: checkin-needed
Pushed by ryanvm@gmail.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/a524931c115b
Fix the incorrect AutoTextureLock lifecycle. r=dvander
Keywords: checkin-needed
Sorry spam, please ignore comment#18
https://hg.mozilla.org/mozilla-central/rev/a524931c115b
Status: NEW → RESOLVED
Closed: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla53
See Also: → 1319354
Since the bug is closed, I open Bug 1319354 to track the automatetest mentioned in Comment 4.
I can reproduce the problem on  Nightly 2016-Nov-21 build[1]
I can verify to fix the problem on Nightly 2016-Nov-22 build[2].

[1]
https://hg.mozilla.org/mozilla-central/rev/b7f895c1dc2e91530240efbf50ac063a0f8a9cb5
Mozilla/5.0 (Windows NT 10.0; WOW64; rv:53.0) Gecko/20100101 Firefox/53.0 ID:20161121030224

[2]
https://hg.mozilla.org/mozilla-central/rev/0534254e9a40b4bade2577c631fe4cfa0b5db41d
Mozilla/5.0 (Windows NT 10.0; WOW64; rv:53.0) Gecko/20100101 Firefox/53.0 ID:20161122030216


Please up lift this to Aurora52.0a2.
Comment on attachment 8811643 [details] [diff] [review]
Fix the incorrect AutoTextureLock lifecycle.

fix regression on d3d11, take in aurora52
Attachment #8811643 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Duplicate of this bug: 1319578
You need to log in before you can comment on or make changes to this bug.