1731135 - Video layer updates must be made atomic with other layer updates

Status: NEW

(Core :: Graphics, defect, P3)

Description

[Brad Werth [:bradwerth]]

As noted in review of D116643 by Markus:

Enqueuing the surface has immediate effect and can update the screen outside of the CoreAnimation transaction. As a follow-up (and before we enable the pref), I think we need to handle the case where there are other layer changes that should happen in the same composite. In those cases we should make it so that those changes all apply atomically. For example, we may have to recreate the video layer in that case.

Opening this bug to track this issue.

Comment 1

[Brad Werth [:bradwerth]]

Markus, do you feel this is solved by Bug 1741982?

Comment 2

[Markus Stange [:mstange]]

No, it's not - bug 1741982 only changes what happens if the video is the only thing that's updating. But this bug is about what happens when both a video frame update and some other layer tree update are happening in the same composite. That scenario wasn't changed in bug 1741982.
To fix this scenario, I think we'll need to detect composites in which we have both kinds of updates, and then tell the video layer to recreate its AVSampleBufferDisplayLayer.

Before:

1. CATransaction begins.
2. Some CALayer stuff is mutated, but doesn't end up on the screen yet because the transaction isn't committed yet.
3. Video frame update is enqueued - may reach the screen immediately.
4. Some more CALayer mutations.
5. CATransaction is committed. Now these changes end up on the screen.

We need the video frame update in step 3 to get to the screen at the same time as the CALayer mutations which are committed in step 5.

After:

1. CATransaction begins.
2. Some CALayer stuff is mutated, but doesn't end up on the screen yet because the transaction isn't committed yet.
3. Video layer sees that it has an update which needs to be atomic with the transaction. It creates a new AVSampleBufferDisplayLayer, and enqueues the frame on it. This update does not reach the screen because the screen still displays the old AVSampleBufferDisplayLayer which still has the old frame in it.
4. Some more CALayer mutations.
5. CATransaction is committed. Now the new video layer, together with the other CALayer mutations, reaches the screen.

Comment 3

[Brad Werth [:bradwerth]]

Attachment: Bug 1731135: Make NativeLayerCA::UpdateType into a bitfield and check cases independently.

This change attempts to ensure that video updates happen atomically with every
other type of layer update scheduled in the same Commit. It is not successful
and introduces flashing black regions when a video update is done in the same
transaction as any other type of update.

Comment 4

[Brad Werth [:bradwerth]]

(In reply to Markus Stange [:mstange] from comment #2)

We need the video frame update in step 3 to get to the screen at the same time as the CALayer mutations which are committed in step 5.

After:

1. CATransaction begins.
2. Some CALayer stuff is mutated, but doesn't end up on the screen yet because the transaction isn't committed yet.
3. Video layer sees that it has an update which needs to be atomic with the transaction. It creates a new AVSampleBufferDisplayLayer, and enqueues the frame on it. This update does not reach the screen because the screen still displays the old AVSampleBufferDisplayLayer which still has the old frame in it.
4. Some more CALayer mutations.
5. CATransaction is committed. Now the new video layer, together with the other CALayer mutations, reaches the screen.

The patch attempts this approach but introduces visual flickering when a video update happens alongside other updates (which creates a new layer). It's possible that the AVSampleBufferDisplayLayer is doing some asynchronous decoding and so it can't guarantee that it has a new frame to display when it is just created. We might be able circumvent this by doing some combination of warming up the layer with the old frame and flushing the layer when it gets its first frame. I'll try some more ideas.

Comment 5

[Brad Werth [:bradwerth]]

Attachment: WIP: Bug 1731135: Make NativeLayerCA only enqueue samples during video updates.

We have two methods of displaying video frames:

1. Enqueued: This can only be done by an AVSampleBufferDisplayLayer. This
   method updates the layer outside of the CATransaction. We only want to use this
   method when there are no other layer updates to perform. Using this method
   allows the device to operate in the low-power "detached mode".

2. Synchronized: This can be done by any CALayer, including by an AV layer.
   This method keeps the video update atomic with other layer updates. This
   method prevents the use of detached mode.

So we want to transition between the two methods and do so without a visual
interruption. This patch has the followin problems:

Enqueued -> Synchronized: This is causing a noticeable visual stutter.

Synchronized -> Enqueued: This is visually smooth but detached mode is no
longer achieved. It seems that once an AV layer has been updated in
synchronized mode, it is "poisoned" for eligibility to use detached mode.

Comment 6

[Brad Werth [:bradwerth]]

As I try out different things, everything I try creates a frame stutter as we transition from enqueueSampleBuffer to synchronized display of video content. My current theory is that enqueueing a sample in an AV layer doesn't immediately display the sample, even with the kCMSampleAttachmentKey_DisplayImmediately key set. The sample doesn't display until some later time, outside of the current CATransaction. That is, after all, the motivation for this bug. We can observe this effect when we see that creating a new AV layer and enqueueing a sample creates a flickering effect since the layer initially appears empty.

So that means that the transition from enqueued back to synchronized will always create a jump forward in time. At best, that jump in time will be less than a frame's duration and be minimally noticeable. I'll keep trying to find a solution that completes this skip within the duration of one frame.