Closed Bug 1064847 Opened 7 years ago Closed 7 years ago

youtube video plays audio, but not video under e10s


(Core :: Audio/Video, defect)

Not set



Tracking Status
e10s m3+ ---


(Reporter: blassey, Assigned: jya)



(Keywords: regression)


(1 file, 1 obsolete file)

Both embedded and on YouTube 

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.
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.
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.
Duplicate of this bug: 1066981
Might be related to bug 1062468.
Depends on: 1062468
(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
See Also: → 1067558
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.
Make IOSurface global so it can be passed between processes
Attachment #8490476 - Flags: review?(matt.woodrow)
Assignee: davidp99 → jyavenard
Update so it stays in local allocator style (NULL vs kCFAllocatorDefault)
Attachment #8490490 - Flags: review?(matt.woodrow)
Attachment #8490476 - Attachment is obsolete: true
Attachment #8490476 - Flags: review?(matt.woodrow)
Blocks: 1062654
Duplicate of this bug: 1067910
Attachment #8490490 - Flags: review?(matt.woodrow) → review?(giles)
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+
Keywords: checkin-needed
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla35
Flags: qe-verify+
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).
You need to log in before you can comment on or make changes to this bug.