1064847 - youtube video plays audio, but not video under e10s

Status: VERIFIED FIXED

(Core :: Audio/Video, defect)

Description

[Brad Lassey [:blassey] (use needinfo?)]

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

[David Parks [:handyman]]

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)

Comment 2

[David Parks [:handyman]]

Quick note: Both videos play fine for me on Windows 8.  And both fail on mac, as reported.

Comment 5

[Chris Peterson [:cpeterson]]

This is a regression, so we should bisect the regression point.

Comment 6

[Gian-Carlo Pascutto [:gcp]]

Based on comment 5.

Comment 7

[David Parks [:handyman]]

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

[Chris Peterson [:cpeterson]]

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

Comment 9

[Jean-Yves Avenard [:jya]]

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.

Comment 11

[JW Wang [:jwwang] [:jw_wang]]

Might be related to bug 1062468.

Comment 12

[Jean-Yves Avenard [:jya]]

(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.

Comment 13

[Jean-Yves Avenard [:jya]]

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.

Comment 14

[Jean-Yves Avenard [:jya]]

Attachment: Make media generated IOSurface global

Make IOSurface global so it can be passed between processes

Comment 15

[Jean-Yves Avenard [:jya]]

Attachment: Make media generated IOSurface global

Update so it stays in local allocator style (NULL vs kCFAllocatorDefault)

Comment 17

[Jean-Yves Avenard [:jya]]

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

[Ralph Giles (:rillian)]

Comment on attachment 8490490 [details] [diff] [review]
Make media generated IOSurface global

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

Seems legit.

Comment 19

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/9bcc7c7bfc4f

Comment 21

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

https://hg.mozilla.org/mozilla-central/rev/9bcc7c7bfc4f

Comment 22

[Ada [:adalucinet]]

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).