stuttering/corruption with 4k youtube and external monitor

RESOLVED FIXED in Firefox 38

Status

()

RESOLVED FIXED
4 years ago
4 years ago

People

(Reporter: rillian, Assigned: jya)

Tracking

(Blocks: 1 bug)

Trunk
mozilla40
x86
macOS
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox38+ fixed, firefox39+ fixed, firefox40+ fixed)

Details

(URL)

Attachments

(1 attachment)

Trying Youtube with a uhd monitor I ran into serious playback issues. Dell P2715Q was set the multistream secondary mode, so laptop could only drive it at 2160p30. (This is because of a hardware limitation with my older laptop; newer macs can drive it at 2160p60 is in single stream displayport mode.)

Visiting https://www.youtube.com/watch?v=iNJdPyoqt8U and selecting "2160p4k" from the quality popup under the gear menu in the youtube player, I see a number of issues:

Playback stutters like frame ordering is incorrect. This is especially obvious near scene changes where it will flip between frames from either side of the cut. It also sometimes plays backwards for a few frames.

There are also occasional corrupt frames with glitches like areas of white blocks, or the left and right halves of the frame swapped (like the base pointer was incorrect) or tearing between different frames.

I see the issues even when not playing back at full screen, but I don't see them when no external monitor is connected.

Reproduced with an early 2013 macbook pro running 10.8.5 in Aurora 39 and Nightly 40, and in Nightly 40 on a late 2013 macbook pro running 10.10.3. Former has nVidia GeForce GT 650M and 1 GB of video ram. Later has GT 750M with 2 GB of video ram (and also integrated intel GPUs, but I don't think those were being used at the time).

Note that the video is very large. "Stats for Nerds" claimed between 10 and 70 Mbps for traffic, so without a fast connection it will just buffer all the time.
(Assignee)

Comment 1

4 years ago
How does it play with safari?
(Assignee)

Updated

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

Comment 3

4 years ago
In http://uri-labs.com/macosx_headers/CVPixelBufferIOSurface_h/index.html:
can read:
"The CVPixelBuffer will retain the IOSurface. IMPORTANT NOTE: If you are using IOSurface to share CVPixelBuffers between processes and those CVPixelBuffers are allocated via a CVPixelBufferPool, it is important that the CVPixelBufferPool does not reuse CVPixelBuffers whose IOSurfaces are still in use in other processes. CoreVideo and IOSurface will take care of this for if you use IOSurfaceCreateMachPort and IOSurfaceLookupFromMachPort, but NOT if you pass IOSurfaceIDs.
"

Matt, we are increasing the usage count of the IOSurface, but we aren't using mach port anywhere in the MacIOSurface.

Sounds like this is what is happening here. The CVPixelBuffer is re-use before the IOSurface has the chance to be rendered. So what is displayed appears out of order as a frame could be displayed more than once.
Flags: needinfo?(matt.woodrow)
We're seeing this in the non-e10s case, so I don't think this is specific to cross process sharing.

Maybe we need to do something else to stop the IOSurface being re-used though?

We do share IOSurfaces to the compositor thread using the IOSurfaceID, but we should be ensuring that at least one MacIOSurface is alive at all times (which is what is holding the use-count reference), so that should be fine.

It's possible that last assertion isn't true, and we let all MacIOSurfaces die while still holding an IOSurfaceID?
Flags: needinfo?(matt.woodrow)
You should try making the MacIOSurfaceTextureClientOGL destructor [1] hold a ref to the MacIOSurface until the compositor side has been torn down too, that would make it impossible for this to happen. We have a 'KeepUntilFullDeallocation' method for this, see what TextureClientD3D11 [2] does.

[1] http://mxr.mozilla.org/mozilla-central/source/gfx/layers/opengl/MacIOSurfaceTextureClientOGL.cpp#18
[2] http://mxr.mozilla.org/mozilla-central/source/gfx/layers/d3d11/TextureD3D11.cpp#187
(Assignee)

Comment 6

4 years ago
The MacIOSurface does hold a reference to the IOSurface.

Is the MacIOSurface released before the compositor finished with the IOSurface itself?
MacIOSurfaceTextureClientOGL holds a MacIOSurface, as does MacIOSurfaceTextureHostOGL.

Our layers infrastructure asynchronously constructs the Host after the Client is created, and does so by serializing the IOSurfaceID.

It is however possible that the Client is destroyed before the Host is created, so we'd have a brief period where no MacIOSurfaces exist for the IOSurfaceID that has been serialized.
(Assignee)

Comment 8

4 years ago
Matt, you champion.. That did fix the issue I was seeing, especially that one https://roystanross.wordpress.com/super-mario-64-hd/
Attachment #8591314 - Flags: review?(matt.woodrow)
(Assignee)

Updated

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

Updated

4 years ago
Duplicate of this bug: 1144401
Comment on attachment 8591314 [details] [diff] [review]
Ensure IOSurface isn't released before being composited

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

Awesome!
Attachment #8591314 - Flags: review?(matt.woodrow) → review+
(Assignee)

Comment 12

4 years ago
Comment on attachment 8591314 [details] [diff] [review]
Ensure IOSurface isn't released before being composited

Approval Request Comment
[Feature/regressing bug #]:1153469
[User impact if declined]: Invalid/Corrupted frame being displayed.
[Describe test coverage new/current, TreeHerder]: Locally tested on videos known to have problems. 
[Risks and why]: It's a wonder it ever worked without it
[String/UUID change made/needed]: None
Attachment #8591314 - Flags: approval-mozilla-beta?
Attachment #8591314 - Flags: approval-mozilla-aurora?
status-firefox38: --- → affected
status-firefox39: --- → affected
Tracking this for 38+.
tracking-firefox38: --- → +
tracking-firefox39: --- → +
tracking-firefox40: --- → +
https://hg.mozilla.org/mozilla-central/rev/262e8120ccb2
Status: ASSIGNED → RESOLVED
Last Resolved: 4 years ago
status-firefox40: affected → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla40
Comment on attachment 8591314 [details] [diff] [review]
Ensure IOSurface isn't released before being composited

Should be in 38 beta 4.
Attachment #8591314 - Flags: approval-mozilla-beta?
Attachment #8591314 - Flags: approval-mozilla-beta+
Attachment #8591314 - Flags: approval-mozilla-aurora?
Attachment #8591314 - Flags: approval-mozilla-aurora+
(Assignee)

Comment 17

4 years ago
remote:   https://hg.mozilla.org/releases/mozilla-aurora/rev/22ea07794266
status-firefox39: affected → fixed
(Assignee)

Comment 18

4 years ago
remote:   https://hg.mozilla.org/releases/mozilla-beta/rev/7efd806788be
status-firefox38: affected → fixed
(Assignee)

Updated

4 years ago
Duplicate of this bug: 1149007
Depends on: 1187103
You need to log in before you can comment on or make changes to this bug.