Harden PVideoDecoder against process crashes

RESOLVED FIXED in Firefox 52

Status

()

Core
Audio/Video: Playback
P3
normal
RESOLVED FIXED
2 years ago
2 years ago

People

(Reporter: mattwoodrow, Assigned: mattwoodrow)

Tracking

(Depends on: 1 bug)

Trunk
mozilla52
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox51 affected, firefox52 fixed)

Details

Attachments

(1 attachment, 2 obsolete attachments)

(Assignee)

Description

2 years ago
We want to be able to handle crashes of either the GPU process, or the child process without crashing the other.

If it's the GPU process that crashes, we want to resume decoding as soon as it's available again, probably from the next keyframe.
Priority: -- → P3
(Assignee)

Updated

2 years ago
Depends on: 1305361
(Assignee)

Comment 1

2 years ago
Created attachment 8806188 [details] [diff] [review]
(lazily) Rebuild VideoDecoderManager when we restart the GPU process
Assignee: nobody → matt.woodrow
Attachment #8806188 - Flags: review?(dvander)
(Assignee)

Comment 2

2 years ago
Created attachment 8806555 [details] [diff] [review]
(lazily) Rebuild VideoDecoderManager when we restart the GPU process
Attachment #8806188 - Attachment is obsolete: true
Attachment #8806188 - Flags: review?(dvander)
Attachment #8806555 - Flags: review?(dvander)
(Assignee)

Comment 3

2 years ago
Created attachment 8806567 [details] [diff] [review]
(lazily) Rebuild VideoDecoderManager when we restart the GPU process v3
Attachment #8806555 - Attachment is obsolete: true
Attachment #8806555 - Flags: review?(dvander)
Attachment #8806567 - Flags: review?(dvander)
Comment on attachment 8806567 [details] [diff] [review]
(lazily) Rebuild VideoDecoderManager when we restart the GPU process v3

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

::: dom/media/ipc/VideoDecoderManagerChild.cpp
@@ +84,5 @@
> +    RefPtr<VideoDecoderManagerChild> manager;
> +    RefPtr<Runnable> createManager = NS_NewRunnableFunction([&]() {
> +      Endpoint<PVideoDecoderManagerChild> endpoint;
> +      if (!ContentChild::GetSingleton()->SendInitVideoDecoderManager(&endpoint)) {
> +        return;

return nullptr? (below too)

slightly better form to have this function return RefPtr<VideoDecoderManagerChild>, but okay either way now that sDecoderManager is single-thread.
Attachment #8806567 - Flags: review?(dvander) → review+

Comment 5

2 years ago
Pushed by mwoodrow@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/c13ed43a9cc2
Rebuild VideoDecoderManager when we restart the GPU process. r=dvander

Comment 6

2 years ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/c13ed43a9cc2
Status: NEW → RESOLVED
Last Resolved: 2 years ago
status-firefox52: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla52
You need to log in before you can comment on or make changes to this bug.