We really need to support reading back videos decoded on the GPU, as this is how we draw videos to <canvas>. We'll probably need a sync message on the VideoDecoderManager protocol that reads back into a shmem and returns it.

Assuming GetAsSourceSurface is the underlying graphics call used in canvas.drawImage(video, x, y), there is overlap between waiting on suspend/resume and waiting on IPC.

We want to keep the shmem around when we do a readback so that we can use it directly for drawing. Unfortunately this means that it won't be released on the video decoder manager thread, so we need to handle this.

I realize SurfaceDescriptor isn't normally meant to be used, but I think this use case (similar to reftests) is an exception. We're very much directly passing ownership of a surface from one process to another, rather than having persisted shared ownership like we do with PTexture.

Comment on attachment 8806196 [details] [diff] [review] Part 3: Implement readback for GPU videos Review of attachment 8806196 [details] [diff] [review]: ----------------------------------------------------------------- ::: dom/media/ipc/VideoDecoderManagerChild.cpp @@ +199,5 @@ > + RefPtr<VideoDecoderManagerChild> mAllocator; > + SurfaceDescriptor mSD; > +}; > + > +void DeleteSurfaceDescriptor(void* aClosure) DeleteSurfaceDescriptorUserData? ::: gfx/layers/GPUVideoImage.h @@ +44,5 @@ > + { > + if (!mTextureClient) { > + return nullptr; > + } > + GPUVideoTextureData* data = static_cast<GPUVideoTextureData*>(mTextureClient->GetInternalData()); Is there any way to do this with a safe casting pattern, versus a static cast? Even if the "safe" accessor is just a helper on TextureClient that does an unsafe cast. ::: gfx/layers/ipc/ImageBridgeChild.h @@ +395,5 @@ > }; > > +// Helper that creates a monitor and a "done" flag, then enters the monitor. > +// This can go away when we switch ImageBridge to an XPCOM thread. > +class MOZ_STACK_CLASS SynchronousTask Bonus points if this goes in a new header file.

Comment on attachment 8806194 [details] [diff] [review] Part 1: Allow asynchronous deletion of Shmem for VideoDecoderManagerChild Review of attachment 8806194 [details] [diff] [review]: ----------------------------------------------------------------- ::: gfx/layers/ipc/ImageBridgeChild.cpp @@ +1095,4 @@ > } > > SynchronousTask task("AllocatorProxy Dealloc"); > + bool result; This needs to be initialized false - that or assign it at the start of ProxyDeallocShmemNow.

Pushed by mwoodrow@mozilla.com: https://hg.mozilla.org/integration/mozilla-inbound/rev/5771d02d895e Part 1: Allow asynchronous deletion of Shmem for VideoDecoderManagerChild. r=dvander https://hg.mozilla.org/integration/mozilla-inbound/rev/65e8b3590892 Part 2: Expose SurfaceDescriptor helpers. r=nical https://hg.mozilla.org/integration/mozilla-inbound/rev/7817ed7afb84 Part 3 - Implement readback for GPU videos. r=dvander,nical

https://hg.mozilla.org/mozilla-central/rev/5771d02d895e https://hg.mozilla.org/mozilla-central/rev/65e8b3590892 https://hg.mozilla.org/mozilla-central/rev/7817ed7afb84