1125914 - Reuse TextureClient/TextureHost for video playback

Status: RESOLVED WORKSFORME

(Core :: Graphics: Layers, defect)

Description

[Jeff Muizelaar [:jrmuizel]]

This can save allocations and expensive binding overhead.

Comment 1

[Sotaro Ikeda [:sotaro]]

Bug 1043558 might be similar bug on gonk case.

Comment 2

[Chiajung Hung [:chiajung]]

Current software decoder on Gonk platform has the problem, too.

The worst case in video playback would be software codec on B2G.

Since we default to GrallocImage, and no recycler currently.
Each new frame allocates a GraphicBuffer(gralloc allocation is rather expensive and it default to memset before returning) than fill the buffer.

Hardware codec uses GonkBufferQueue, which handles buffer recycling in a decent way.

BTW, the most efficient way to recycle GraphicBuffer is recycling the GraphicBuffer in SharedBufferManagerParent, which able to recycle buffer across process and Content Buffers. However, it would has some security consideration though.

Comment 3

[Ethan Lin[:ethlin]]

Attachment: recycle for GrallocImage and GrallocTexutureClient

I write a prototype to do the gralloc image/texture recycle. If this way is okay, we can apply to other image types.

Comment 4

[Chiajung Hung [:chiajung]]

Comment on attachment 8604422 [details] [diff] [review]
recycle for GrallocImage and GrallocTexutureClient

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

You missed WebRTC code path, f- for now.
BTW, you do not recycle TextureHost.

Comment 5

[Chiajung Hung [:chiajung]]

BTW, you should comment what case you need CreateImage(format, false) and why.

Comment 6

[Ethan Lin[:ethlin]]

(In reply to Chiajung Hung [:chiajung] from comment #4)
> Comment on attachment 8604422 [details] [diff] [review]
> recycle for GrallocImage and GrallocTexutureClient
> 
> Review of attachment 8604422 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> You missed WebRTC code path, f- for now.
> BTW, you do not recycle TextureHost.

OK. I will add WebRTC part. This patch is just a prototype for TextureClient.

Comment 7

[Ethan Lin[:ethlin]]

Attachment: WIP: recycle for Gralloc texture

In this patch, I use a ImagePool to recycle images for video playback. I only implement the part of Gralloc type. Jeff, do you have any concern of this method?

Comment 8

[Sotaro Ikeda [:sotaro]]

Can't we use TextureClientRecycleAllocator? I am not fun of creating a new recycling mechanism from scratch everywhere.

Comment 9

[Sotaro Ikeda [:sotaro]]

Bug 1135935 might be also related.

Comment 10

[Ethan Lin[:ethlin]]

sotaro, I will study the patch in Bug 1135935 and try the TextureClientRecycleAllocator.

Comment 11

[Sotaro Ikeda [:sotaro]]

(In reply to Ethan Lin[:elin] from comment #10)
> sotaro, I will study the patch in Bug 1135935 and try the
> TextureClientRecycleAllocator.

Bug 1043558 might be also related.

Image is a wrapper of video/image buffers. I do not think it is a good idea to recycle at Image level if the buffer is TextureClient.  In gralloc case, GrallocTextureClient is used as a video buffer. And actual TextureClient usage is longer than Image's lifetime. Image is until ImageContainer. But TextureClient is delivered to ImageClient and also used in ImageHost as TextureHost. The following might help to understand it.
  https://github.com/sotaroikeda/firefox-diagrams/blob/master/gfx/gfx_ImageBridge_FirefoxOS_2_1.pdf

In current gfx layers, TextureClients expects ISurfaceAllocator. In b2g, gecko's media framework always uses ImageBridge for ISurfaceAllocator. But on another platform, it needs to handle also non ImageBridge environment.

CairoImage::GetTextureClient() also does recycling. I added it for animated gif/animated png usage performance on b2g. It is a non-media framework usage.
 https://dxr.mozilla.org/mozilla-central/source/gfx/layers/ImageContainer.cpp#494

Comment 12

[Sotaro Ikeda [:sotaro]]

And when We recycle TextureClient, we always need to ensure that TextureClient is not used by CompositableHost. RemoveTextureFromCompositableTracker is used to ensure it. On b2g, it is always used related to trigger Fence delivery. But on another platform it is not always ensured.

Comment 13

[Sotaro Ikeda [:sotaro]]

On media framework, Image is used for only one CompositableForwarder. But if it is non-media framework case, Image could be used by multiple CompositableForwarder as in CairoImage::GetTextureClient().
 https://dxr.mozilla.org/mozilla-central/source/gfx/layers/ImageContainer.cpp#494

Comment 14

[Jeff Muizelaar [:jrmuizel]]

Sotaro does this bug still make sense? Don't we do this already?

Comment 15

[Sotaro Ikeda [:sotaro]]

It is already done in a different way.