youtube video plays audio, but not video under e10s

VERIFIED FIXED in mozilla35

Status

()

VERIFIED FIXED
4 years ago
4 years ago

People

(Reporter: blassey, Assigned: jya)

Tracking

({regression})

unspecified
mozilla35
x86
Mac OS X
regression
Points:
---
Dependency tree / graph
Bug Flags:
qe-verify +

Firefox Tracking Flags

(e10sm3+)

Details

Attachments

(1 attachment, 1 obsolete attachment)

Both embedded http://digg.com/video/even-if-you-dont-like-rugby-charles-dance-is-going-to-get-you-fired-up-for-life and on YouTube https://www.youtube.com/watch?v=bl9ahNhH3Cc. 

In the embedded version it's black the whole time. On the YouTube site, its white with the IRB logo in the corner. It appears not to render and display the first frame, but nothing more. I'm testing in the 9/9 nightly with browser.tabs.remote.autostart set to true.
Some relevant spew (from the digg case):

[Child 47967] WARNING: Most platforms still need an optimized way to share GL cross process.: file /Users/davidp/mozilla/central/firefox/gfx/layers/client/CanvasClient.cpp, line 42
[Parent 47965] WARNING: failed to lock front buffer: file /Users/davidp/mozilla/central/firefox/gfx/layers/composite/ImageHost.cpp, line 86

(the front buffer message is repeated, likely each frame)
Assignee: nobody → davidp99
Quick note: Both videos play fine for me on Windows 8.  And both fail on mac, as reported.
Duplicate of this bug: 1066199
This is a regression, so we should bisect the regression point.
(Reporter)

Updated

4 years ago
tracking-e10s: ? → m3+
Based on comment 5.
Keywords: regression, regressionwindow-wanted
I haven't confirmed anything with old builds but, from debugging, my current guess is that the origin may be the patch in bug 1059066, "Reduce copies in AppleVTDecoder".

Code-wise, the problem is that the IOSurfaceID isn't available in the parent process even tho it was just created in the child process.  I suspect its being destroyed in the interim somehow, but right now that's a hunch.
I bisected this regression to Nightly 35 build 2014-09-09, which includes bug 1059066:

https://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=06e1ca1fbf24&tochange=6b8da5940f74
Blocks: 1059066
Keywords: regressionwindow-wanted
(Assignee)

Comment 9

4 years ago
The issue is likely that the compositor uses IOSurfaceID internally. Retrieving the IOSurfaceID itself doesn't increase the usage count, this can cause a race conditions where the IOSurface is destroyed before it gets used.

The issue was limited as the IOSurfaceIDs were only used within the compositor. Now that we pass directly the IOSurface generated by the hardware decoder, the problem is exposed.

The Apple h264 decoder itself provides a IOSurface with a properly increased usage count.

As such, the problem itself isn't in the audio/video component, as it could be triggered even with a plugin.

In the Apple IOSurface documentation, the recommended way to pass them around is to use mac_port.

Matt Woodrow will likely have more insight to this problem and what can be done about it.
(Assignee)

Updated

4 years ago
Duplicate of this bug: 1066981
Might be related to bug 1062468.
Depends on: 1062468
(Assignee)

Comment 12

4 years ago
(In reply to JW Wang [:jwwang] from comment #11)
> Might be related to bug 1062468.

unlikely... this bug is related to the use of IOSurface on mac platforms.
No longer depends on: 1062468

Updated

4 years ago
See Also: → bug 1067558
(Assignee)

Comment 13

4 years ago
Upon a bit of investigation, the IOSurfaceRef is being retrieved from the IOSurfaceID in MacIOSurface::LookupSurface

This is done via a call to IOSurface framework's IOSurfaceLookup.

This returns a nullptr.
(Assignee)

Comment 14

4 years ago
Created attachment 8490476 [details] [diff] [review]
Make media generated IOSurface global

Make IOSurface global so it can be passed between processes
Attachment #8490476 - Flags: review?(matt.woodrow)
(Assignee)

Updated

4 years ago
Assignee: davidp99 → jyavenard
Status: NEW → ASSIGNED
(Assignee)

Comment 15

4 years ago
Created attachment 8490490 [details] [diff] [review]
Make media generated IOSurface global

Update so it stays in local allocator style (NULL vs kCFAllocatorDefault)
Attachment #8490490 - Flags: review?(matt.woodrow)
(Assignee)

Updated

4 years ago
Attachment #8490476 - Attachment is obsolete: true
Attachment #8490476 - Flags: review?(matt.woodrow)
(Assignee)

Updated

4 years ago
Blocks: 1062654

Updated

4 years ago
Duplicate of this bug: 1067910
(Assignee)

Updated

4 years ago
Attachment #8490490 - Flags: review?(matt.woodrow) → review?(giles)
(Assignee)

Comment 17

4 years ago
Matt is on PTO, I don't want to delay this too much...

Note for rillian:
Declaring the IOSurface as global is how MacIOSurface::CreateIOSurface allocates a surface, so all IOSurface currently used other than the ones created by the decoder are already flagged as global.
As such, I can't foresee a problem in doing the same.
Comment on attachment 8490490 [details] [diff] [review]
Make media generated IOSurface global

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

Seems legit.
Attachment #8490490 - Flags: review?(giles) → review+
(Assignee)

Updated

4 years ago
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/9bcc7c7bfc4f
Status: ASSIGNED → RESOLVED
Last Resolved: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla35
Flags: qe-verify+

Comment 22

4 years ago
Reproduced with Nightly 2014-09-09 on Mac OS X 10.9.5 with str from comment 0.
Verified as fixed with Nightly 37.0a1 (Build ID: 20141222030202).
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.