Closed Bug 1393399 Opened 3 years ago Closed 3 years ago

Deblacklisting HW decoding and add Telemetry to see the decoder recovery time.

Categories

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

x86_64
Windows
enhancement
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla57
Tracking Status
firefox57 --- fixed

People

(Reporter: kaku, Assigned: kaku)

References

(Blocks 1 open bug)

Details

Attachments

(5 files)

One more step toward bug 1372070.

We want to know how long it takes to recover video decoder from a GPU process crash. The data will help us make a call on removing blacklist.
Blocks: 1372070
Assignee: nobody → kaku
Status: NEW → ASSIGNED
Comment on attachment 8903428 [details]
Bug 1393399 P1 - add telemetry probes;

https://reviewboard.mozilla.org/r/175268/#review180278

lgtm
Attachment #8903428 - Flags: review?(gsquelart) → review+
Comment on attachment 8903429 [details]
Bug 1393399 P2 - keep the GPU process crash time and send back to MFR;

https://reviewboard.mozilla.org/r/175270/#review180288

::: dom/media/MediaResult.h:72
(Diff revision 1)
> +  const TimeStamp& GPUCrashTimeStamp() const { return mGPUCrashTimeStamp; }
> +
>  private:
>    nsresult mCode;
>    nsCString mMessage;
> +  TimeStamp mGPUCrashTimeStamp; // Used in bug 1393399 for telemetry usage.

Can you say "... for temporary telemetry usage" in the comment, to emphasize that this member will eventually be removed, when we don't need the telemetry anymore.
Attachment #8903429 - Flags: review?(gsquelart) → review+
Comment on attachment 8903430 [details]
Bug 1393399 P3 - keep the MediaDecoderOwner's identification in MFR;

https://reviewboard.mozilla.org/r/175272/#review180282

::: commit-message-1175a:4
(Diff revision 1)
> +Bug 1393399 P3 - keep the MediaDecoderOwner's identification in MFR;
> +
> +When GPU process crashes, the MediaDecoder, MDSM, and MFR are all destroyed.
> +So, we use MediaDecoderOwner to identiy which video we're dealing with.

'identiy' -> 'identify'
Attachment #8903430 - Flags: review?(gsquelart) → review+
Comment on attachment 8903431 [details]
Bug 1393399 P4 - implement GPUProcessCrashTelemetryLogger helper class;

https://reviewboard.mozilla.org/r/175274/#review180284

::: dom/media/MediaFormatReader.cpp:52
(Diff revision 1)
>  
> +
> +typedef void* MediaDataDecoderID;
> +
> +/**
> + * This helper class is used to report telemetry of the time used to recovery a

'recovery' -> 'recover'
Attachment #8903431 - Flags: review?(gsquelart) → review+
Comment on attachment 8903432 [details]
Bug 1393399 P5 - report the recovery time telemetry;

https://reviewboard.mozilla.org/r/175276/#review180290
Attachment #8903432 - Flags: review?(gsquelart) → review+
Comment on attachment 8903428 [details]
Bug 1393399 P1 - add telemetry probes;

https://reviewboard.mozilla.org/r/175268/#review180614

This is Category 1 data.

datareview+
Attachment #8903428 - Flags: review?(francois) → review+
Thanks for the review!
We're sorry, Autoland could not rebase your commits for you automatically. Please manually rebase your commits and try again.

hg error in cmd: hg rebase -s 550d713f8508 -d e88120f2f0dc: rebasing 417532:550d713f8508 "Bug 1393399 P1 - add telemetry probes; r=francois,gerald"
merging toolkit/components/telemetry/Histograms.json
warning: conflicts while merging toolkit/components/telemetry/Histograms.json! (edit, then use 'hg resolve --mark')
unresolved conflicts (see hg resolve, then hg rebase --continue)
Pushed by tkuo@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/c88a7744a050
P1 - add telemetry probes; r=francois,gerald
https://hg.mozilla.org/integration/autoland/rev/7de2b91cef4b
P2 - keep the GPU process crash time and send back to MFR; r=gerald
https://hg.mozilla.org/integration/autoland/rev/e3f58e6eeb8f
P3 - keep the MediaDecoderOwner's identification in MFR; r=gerald
https://hg.mozilla.org/integration/autoland/rev/00e03f0e1c1e
P4 - implement GPUProcessCrashTelemetryLogger helper class; r=gerald
https://hg.mozilla.org/integration/autoland/rev/120baee2ca23
P5 - report the recovery time telemetry; r=gerald
Depends on: 1396428
https://mzl.la/2xT0nWC
I think there is something wrong that around 45% crash recoveries take more than 64 seconds.
Depends on: 1401902
Depends on: 1408693
You need to log in before you can comment on or make changes to this bug.