Closed
Bug 1300676
Opened 8 years ago
Closed 8 years ago
Implement GetAsSourceSurface for GPUVideoImage
Categories
(Core :: Audio/Video: Playback, defect, P3)
Core
Audio/Video: Playback
Tracking
()
RESOLVED
FIXED
mozilla52
People
(Reporter: mattwoodrow, Assigned: mattwoodrow)
References
Details
Attachments
(3 files)
(deleted),
patch
|
dvander
:
review+
|
Details | Diff | Splinter Review |
(deleted),
patch
|
nical
:
review+
|
Details | Diff | Splinter Review |
(deleted),
patch
|
dvander
:
review+
nical
:
review+
|
Details | Diff | Splinter Review |
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.
Assignee | ||
Updated•8 years ago
|
Summary: Impemenet GetAsSourceSurface for GPUVideoImage → Implement GetAsSourceSurface for GPUVideoImage
Updated•8 years ago
|
Priority: -- → P3
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.
Assignee | ||
Updated•8 years ago
|
Assignee: nobody → matt.woodrow
Assignee | ||
Comment 2•8 years ago
|
||
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.
Attachment #8806194 -
Flags: review?(dvander)
Assignee | ||
Comment 3•8 years ago
|
||
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.
Attachment #8806195 -
Flags: review?(nical.bugzilla)
Assignee | ||
Comment 4•8 years ago
|
||
Attachment #8806196 -
Flags: review?(dvander)
Assignee | ||
Updated•8 years ago
|
Attachment #8806196 -
Flags: review?(nical.bugzilla)
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.
Attachment #8806196 -
Flags: review?(dvander) → review+
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.
Attachment #8806194 -
Flags: review?(dvander) → review+
Updated•8 years ago
|
Attachment #8806195 -
Flags: review?(nical.bugzilla) → review+
Updated•8 years ago
|
Attachment #8806196 -
Flags: review?(nical.bugzilla) → review+
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
Comment 8•8 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/5771d02d895e
https://hg.mozilla.org/mozilla-central/rev/65e8b3590892
https://hg.mozilla.org/mozilla-central/rev/7817ed7afb84
Status: NEW → RESOLVED
Closed: 8 years ago
status-firefox52:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla52
Updated•6 years ago
|
Updated•6 years ago
|
You need to log in
before you can comment on or make changes to this bug.
Description
•