Closed Bug 1315510 Opened 3 years ago Closed 3 years ago

Automatically recreate VideoDecoderManager if the GPU process crashes

Categories

(Core :: Audio/Video: Playback, defect)

defect
Not set

Tracking

()

RESOLVED FIXED
mozilla52
Tracking Status
firefox52 --- fixed

People

(Reporter: mattwoodrow, Assigned: mattwoodrow)

References

Details

Attachments

(1 file, 2 obsolete files)

Rather than having the video decoder manager thread block on the main thread in order to create a new manager, we should have the main thread just do it for us and post a message to the manager thread when it's done.

We can defer reporting any errors to media until after this completes so that recreated decoders will access the the new process.

This avoids a potential deadlock where the manager thread was blocking on the main thread, but readback was blocking the other direction.

There's still one bug remaining, which is that MediaFormatReader holds a pointer to the LayerManager to help it decide which decoder to use. This will still point to the old one, so won't know if we've switched to BasicCompositor.

I'll do a follow-up that adds listener support for ReinitRendering so we can notify media of the new LayerManager backend.
Attachment #8807926 - Flags: review?(dvander)
Fixed a bug for the case where InitIPDL gets called while the decoder manager is currently dead.
Attachment #8807926 - Attachment is obsolete: true
Attachment #8807926 - Flags: review?(dvander)
Attachment #8807928 - Flags: review?(dvander)
Simplified handling of failures during InitIPDL by just reporting an error back to media and let the caller try again.
Attachment #8807928 - Attachment is obsolete: true
Attachment #8807928 - Flags: review?(dvander)
Attachment #8807932 - Flags: review?(dvander)
Comment on attachment 8807932 [details] [diff] [review]
Recreate VideoDecoderManager as part of ReinitRendering v3

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

::: dom/ipc/PContent.ipdl
@@ +429,5 @@
>      async InitRendering(
>        Endpoint<PCompositorBridgeChild> compositor,
>        Endpoint<PImageBridgeChild> imageBridge,
> +      Endpoint<PVRManagerChild> vr,
> +	  Endpoint<PVideoDecoderManagerChild> video);

nit: hard tab leaked in here, below too

::: dom/media/ipc/VideoDecoderManagerChild.h
@@ +59,5 @@
> +  // notified that the old manager is being destroyed.
> +  // Can only be called from the manager thread.
> +  void RunWhenRecreated(already_AddRefed<Runnable> aTask);
> +
> +  bool CanSend() { return mCanSend; }

Please add a thread assertion here, since it's only safe on the IPDL thread.
Attachment #8807932 - Flags: review?(dvander) → review+
Pushed by mwoodrow@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/d537051ade6a
Automatically recreate VideoDecoderManager if the GPU process crashes. r=dvander
Pushed by mwoodrow@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/95ab9f05b980
Automatically recreate VideoDecoderManager if the GPU process crashes. r=dvander
https://hg.mozilla.org/mozilla-central/rev/95ab9f05b980
Status: NEW → RESOLVED
Closed: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla52
You need to log in before you can comment on or make changes to this bug.