909356 - Figure out the best way to unlock gralloc buffers in the compositor

Status: RESOLVED INCOMPLETE

(Core :: Graphics: Layers, defect)

Description

[Nicolas Silva [:nical]]

Both the ways we do it on b2g18 and trunk are bad for performances :(

Comment 1

[Nicolas Silva [:nical]]

A bit of backround:

there are two ways to unlock a gralloc buffer N that has been locked for composition:
* 1) bind it to a null EGLImage: This is terribly slow on some drivers and that is not the way the API has been desined for (it not what android does either). That's what we do on b2g18
* 2) Bind the gralloc buffer N+1 to the same GL texture. That's what android doesn and it is supposed to be the right way.

On trunk we do 2) but we don't do it the right way: We use the same GLTexture for all the gralloc buffers that will be bound for a given texture unit, and we do it when compositing.

As a result it can happen that the last buffer to be composited is not unlocked when we send it back to the content process at the next transaction when receiving a new buffer.
Because since the transaction and the composition happen asynchronously we send back the buffer N (at the end of the transaction) before compositing buffer (N+1). So we send back something that has not been unlocked yet. And that produces sadness.

Comment 2

[Nicolas Silva [:nical]]

Jeff proposed during one of the gfx meetings that we move away from using one temporary texture per texture unit at the level of the compositor (meaning all the layers use it), and instead use one temporary texture per texture unit at the compositable (read slowly, compositABLE not compositor) level, and that we bind the gralloc buffer to the GL texture during the Transaction instead of during compositing.

I think this is the right way to go, because it ensures that at the end of the transaction, the previously bound gralloc buffer that we will send back to the content side (to the camera) will be unlocked.

Comment 3

[Nicolas Silva [:nical]]

I am curious about whether flavors of D3D have any similar API or if this notion of temporary texture slots are only for OpenGL stuff.

My propostion:

Add a class TemporaryTextureSlots that contains the temporary gl textures (that are currently held by the compositor on trunk), and let the compositable have a reference to it and ask the compositor to create the object only if needed.

I want the CompositableHost to not be specific to GL/Gralloc so it is important to not have gralloc/gl code in CompositableHost and to delegate this to a separate object.

A very nice side-effect of this is that we would then have a simple TextureSourceOGL with just a GL texture instead of having GrallocTextureSourceOGL and SharedTextureSourceOGL (they were needed because they did the glue between the shared texture and the gl texture at composition time but if the TextureHost does the glue in the transaction we don't need them anymore and taht would be pretty nice).

This is going to be annoying when we get to the point where we want to support having the same texture used by several CompositableHosts though.

Comment 4

[Nicolas Silva [:nical]]

Update: Actually I was wrong about item 2) in comment 1.

Unlocking buffer N-1 seems to happen when we render buffer N rather than when we bind the texture. As a result we can't ensure that the gralloc buffer N-1 will be unlocked at the end of the transaction adding buffer N.

As deadlines are approaching fast the safest course is to get back to soultion 1) as we do in b2g18 even though it is not optimal and figure out a better way after b2g 1.2.

Comment 5

[Nicolas Silva [:nical]]

Quick update on the situation: We have a per compositable (per layer that is), gl texture that we can bind our gralloc buffers to. For everly gralloc buffer that the layers receive, binding it to a texture locks it and the previous gralloc buffer will be asynchronously unlocked somewhere between that moment and the next swap of the screen.

This works well when a gralloc buffer is used by only one layer. We have some use cases where we want to use the same texture with several layers and in those cases what we do makes gralloc behave very poorly. Binding a gralloc buffer to several gl textures at the same time is bad.

This blocks bug 912897.

Comment 6

[Sotaro Ikeda [:sotaro]]

From android JB, we are going to use Fence instead of genlock. When we use HWComposer rendering, correct handling of Fence seems necessary. Part of the problem is discussed in Bug 925444. We are already facing the video rendering problem in nexus-4. We need to handle Fence in all layers in future. For the time being, gecko needs to support Fence handling for Video playback.

The following link is a diagram around HWComposer and fence.
https://github.com/sotaroikeda/android-diagrams/blob/master/graphics/HWComposer_4.3.pdf?raw=true

Comment 7

[Sotaro Ikeda [:sotaro]]

Compare to genlock, Fence seems to support multiple Fence object's merge by Fence::merge().
  http://androidxref.com/4.4_r1/xref/frameworks/native/libs/ui/Fence.cpp#72

The function is used like following.

http://androidxref.com/4.4_r1/xref/frameworks/av/services/camera/libcameraservice/device3/Camera3IOStreamBase.cpp#222

http://androidxref.com/4.4_r1/xref/frameworks/av/services/camera/libcameraservice/device3/StatusTracker.cpp#185

http://androidxref.com/4.4_r1/xref/frameworks/native/libs/gui/ConsumerBase.cpp#216

Comment 8

[Milan Sreckovic [:milan] (needinfo for best results)]

Have we resolved this question with fences?

Comment 9

[Sotaro Ikeda [:sotaro]]

(In reply to Milan Sreckovic [:milan] from comment #8)
> Have we resolved this question with fences?

I think the problem is resolved by fence since JB gonk.

Original problem in Comment 0 seems to happens because of bad way of composition at that time. At that time, Compositor uses only one OpenGL texutre for composition. It made GL driver very very busy and it causes composition performance very very bad. In current gecko, gralloc buffers lock/unlock and bind/unbind are already relatively minimum.

The scope of this bug is not clear now. It seems better to close this bug and to create a new bug, if we found a concrete problem about gralloc buffers lock/unlock.

Comment 10

[Nicolas Silva [:nical]]

I agree with Sotaro.