1315510 - Automatically recreate VideoDecoderManager if the GPU process crashes

Status: RESOLVED FIXED

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

Description

[Matt Woodrow (:mattwoodrow)]

Attachment: Recreate VideoDecoderManager as part of ReinitRendering

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.

Comment 1

[Matt Woodrow (:mattwoodrow)]

Attachment: Recreate VideoDecoderManager as part of ReinitRendering v2

Fixed a bug for the case where InitIPDL gets called while the decoder manager is currently dead.

Comment 2

[Matt Woodrow (:mattwoodrow)]

Attachment: Recreate VideoDecoderManager as part of ReinitRendering v3

Simplified handling of failures during InitIPDL by just reporting an error back to media and let the caller try again.

Comment 3

[David Anderson [:dvander] - inactive, e-mail if emergency]

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.

Comment 4

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/d537051ade6a
Automatically recreate VideoDecoderManager if the GPU process crashes. r=dvander

Comment 5

[Phil Ringnalda (:philor)]

Backed out in https://hg.mozilla.org/integration/mozilla-inbound/rev/3e74d390dea4 for leaks like https://treeherder.mozilla.org/logviewer.html#?job_id=38806541&repo=mozilla-inbound, and masses of assertions/crashes followed by leaks like https://treeherder.mozilla.org/logviewer.html#?job_id=38807601&repo=mozilla-inbound

Comment 6

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/95ab9f05b980
Automatically recreate VideoDecoderManager if the GPU process crashes. r=dvander

Comment 7

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/95ab9f05b980