1280839 - Recycle TextureClient in SharedPlanarYCbCrImage

Status: RESOLVED FIXED

(Core :: Graphics: Layers, defect)

Description

[Sotaro Ikeda [:sotaro]]

It is nice to recycle video buffer of FFmpegVideoDecoder.

Comment 1

[Sotaro Ikeda [:sotaro]]

ImageContainer::CreatePlanarYCbCrImage() creates SharedPlanarYCbCrImage. Then it is nice if video buffers are recycled is handled under SharedPlanarYCbCrImage.

Comment 2

[Sotaro Ikeda [:sotaro]]

Attachment: patch part 1 - Update SendRecycleTexture usage

Comment 3

[Sotaro Ikeda [:sotaro]]

Attachment: patch part 2 - Recycle TextureClient in SharedPlanarYCbCrImage

Comment 4

[Sotaro Ikeda [:sotaro]]

Attachment: patch part 1 - Update SendRecycleTexture usage

Comment 5

[Sotaro Ikeda [:sotaro]]

Attachment: patch part 1 - Update SendRecycleTexture usage

Comment 6

[Sotaro Ikeda [:sotaro]]

Attachment: patch part 2 - Handle race condition in CompositableClient::GetTextureClientRecycler()

Comment 7

[Sotaro Ikeda [:sotaro]]

Attachment: patch part 3 - Recycle TextureClient in SharedPlanarYCbCrImage

Comment 8

[Sotaro Ikeda [:sotaro]]

Attachment: patch part 3 - Recycle TextureClient in SharedPlanarYCbCrImage

Comment 9

[Sotaro Ikeda [:sotaro]]

Comment on attachment 8763823 [details] [diff] [review]
patch part 1 - Update SendRecycleTexture usage

:nical, can you feedback to the patches?

Comment 10

[Nicolas Silva [:nical]]

Comment on attachment 8764494 [details] [diff] [review]
patch part 3 - Recycle TextureClient in SharedPlanarYCbCrImage

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

::: gfx/layers/BufferTexture.cpp
@@ +226,5 @@
>    return ImageDataSerializer::SizeFromBufferDescriptor(mDescriptor);
>  }
>  
> +gfx::IntSize
> +BufferTextureData::GetCbCrSize() const

Nit: this could return a Maybe<gfx::IntSize> (not very important).

::: gfx/layers/client/TextureClientRecycleAllocator.cpp
@@ +88,5 @@
> +YCbCrTextureClientAllocationHelper::IsCompatible(TextureClient* aTextureClient)
> +{
> +  MOZ_ASSERT(aTextureClient->GetFormat() == gfx::SurfaceFormat::YUV);
> +
> +  BufferTextureData* bufferData = aTextureClient->GetInternalData()->AsBufferTextureData();

Ideally we shouldn't be poking at the internal data, but I guess we don't have a good API to do what you want to do here so it will be ok for now.

Comment 11

[Nicolas Silva [:nical]]

Comment on attachment 8764476 [details] [diff] [review]
patch part 2 - Handle race condition in CompositableClient::GetTextureClientRecycler()

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

::: gfx/layers/client/CompositableClient.cpp
@@ +276,5 @@
> +  ImageBridgeChild::GetSingleton()->GetMessageLoop()->PostTask(runnable.forget());
> +
> +  // should stop the thread until done.
> +  while (!done) {
> +    barrier.Wait();

Hopefully we don't run into this case too often...

Comment 12

[Sotaro Ikeda [:sotaro]]

> 
> ::: gfx/layers/client/TextureClientRecycleAllocator.cpp
> @@ +88,5 @@
> > +YCbCrTextureClientAllocationHelper::IsCompatible(TextureClient* aTextureClient)
> > +{
> > +  MOZ_ASSERT(aTextureClient->GetFormat() == gfx::SurfaceFormat::YUV);
> > +
> > +  BufferTextureData* bufferData = aTextureClient->GetInternalData()->AsBufferTextureData();
> 
> Ideally we shouldn't be poking at the internal data, but I guess we don't
> have a good API to do what you want to do here so it will be ok for now.

Yea, we do not have a good API for now, then I used a bit dirty way.

Comment 13

[Sotaro Ikeda [:sotaro]]

Attachment: patch part 3 - Recycle TextureClient in SharedPlanarYCbCrImage

Apply the comment.

Comment 14

[Sotaro Ikeda [:sotaro]]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=27da497a23de&selectedJob=22926201

Comment 15

[Sotaro Ikeda [:sotaro]]

Attachment: patch part 4 - Change to ImageContainer::ClearCachedResources()

Comment 16

[Sotaro Ikeda [:sotaro]]

Attachment: patch part 4 - Change to ImageContainer::ClearCachedResources()

Comment 17

[Sotaro Ikeda [:sotaro]]

Attachment: patch part 3 - Recycle TextureClient in SharedPlanarYCbCrImage

Rebased.

Comment 18

[Sotaro Ikeda [:sotaro]]

Attachment: patch part 4 - Change to ImageContainer::ClearCachedResources()

Rebased.

Comment 19

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/00203ba4e8bf
Recycle SharedPlanarYCbCrImage r=nical

Comment 20

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/00203ba4e8bf