Closed
Bug 1064847
Opened 10 years ago
Closed 10 years ago
youtube video plays audio, but not video under e10s
Categories
(Core :: Audio/Video, defect)
Tracking
()
VERIFIED
FIXED
mozilla35
Tracking | Status | |
---|---|---|
e10s | m3+ | --- |
People
(Reporter: blassey, Assigned: jya)
References
Details
(Keywords: regression)
Attachments
(1 file, 1 obsolete file)
1.75 KB,
patch
|
rillian
:
review+
|
Details | Diff | Splinter Review |
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.
Comment 1•10 years ago
|
||
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
Comment 2•10 years ago
|
||
Quick note: Both videos play fine for me on Windows 8. And both fail on mac, as reported.
Comment 3•10 years ago
|
||
Comment 5•10 years ago
|
||
This is a regression, so we should bisect the regression point.
Reporter | ||
Updated•10 years ago
|
Comment 7•10 years ago
|
||
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.
Comment 8•10 years ago
|
||
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•10 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 | ||
Comment 12•10 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.
Assignee | ||
Comment 13•10 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•10 years ago
|
||
Make IOSurface global so it can be passed between processes
Attachment #8490476 -
Flags: review?(matt.woodrow)
Assignee | ||
Updated•10 years ago
|
Assignee: davidp99 → jyavenard
Status: NEW → ASSIGNED
Assignee | ||
Comment 15•10 years ago
|
||
Update so it stays in local allocator style (NULL vs kCFAllocatorDefault)
Attachment #8490490 -
Flags: review?(matt.woodrow)
Assignee | ||
Updated•10 years ago
|
Attachment #8490476 -
Attachment is obsolete: true
Attachment #8490476 -
Flags: review?(matt.woodrow)
Assignee | ||
Updated•10 years ago
|
Attachment #8490490 -
Flags: review?(matt.woodrow) → review?(giles)
Assignee | ||
Comment 17•10 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 18•10 years ago
|
||
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•10 years ago
|
Keywords: checkin-needed
Comment 19•10 years ago
|
||
https://hg.mozilla.org/integration/mozilla-inbound/rev/9bcc7c7bfc4f
Keywords: checkin-needed
Comment 20•10 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/9bcc7c7bfc4f
Status: ASSIGNED → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla35
Updated•10 years ago
|
Flags: qe-verify+
Comment 22•10 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.
Description
•