Closed Bug 1288311 Opened 4 years ago Closed 4 years ago

Update media telemetry probes expiring in Firefox 50.

Categories

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

defect

Tracking

()

RESOLVED FIXED
mozilla50
Tracking Status
firefox50 --- fixed

People

(Reporter: cpearce, Assigned: cpearce)

Details

Attachments

(3 files)

The following histograms are expiring in 50:

* VIDEO_ADOBE_GMP_DISAPPEARED expires in version 50.0a1 (watched by cpearce@mozilla.com) - Whether or not the Adobe EME GMP was expected to be resident on disk but mysteriously isn't.
* VIDEO_ADOBE_GMP_MISSING_FILES expires in version 50.0a1 (watched by cpearce@mozilla.com) - Adobe EME GMP files missing (0=none, or sum of: 1=library, 2=info, 4=voucher)
* VIDEO_CAN_CREATE_AAC_DECODER expires in version 50.0a1 (watched by cpearce@mozilla.com) - Whether at startup we report we can playback MP4 (AAC) audio. This is single value is recorded at every startup.
* VIDEO_CAN_CREATE_H264_DECODER expires in version 50.0a1 (watched by cpearce@mozilla.com) - Whether at startup we report we can playback MP4 (H.264) video. This is single value is recorded at every startup.
* VIDEO_OPENH264_GMP_DISAPPEARED expires in version 50.0a1 (watched by cpearce@mozilla.com, rjesup@mozilla.com) - Whether or not the OpenH264 GMP was expected to be resident on disk but mysteriously isn't.
* VIDEO_OPENH264_GMP_MISSING_FILES expires in version 50.0a1 (watched by cpearce@mozilla.com, rjesup@mozilla.com) - OpenH264 GMP files missing (0=none, or sum of: 1=library, 2=info)


We still want the CAN_CREATE_*_DECODER probes, as we're actively trying to drive the percentage of people who *can't* create to 0.

The missing GMP probes can go I think.
We're actively trying to encourage people to install Windows system decoders to
drive the "can't create" number to 0. So we need this probe to measure our
progress.

Review commit: https://reviewboard.mozilla.org/r/66252/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/66252/
Attachment #8773562 - Flags: review?(francois)
Attachment #8773563 - Flags: review?(francois)
Attachment #8773564 - Flags: review?(spohl.mozilla.bugs)
The telemetry results indicate there's not a big problem here that we need to
solve. So doesn't seem worth while keeping the probes.

Review commit: https://reviewboard.mozilla.org/r/66254/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/66254/
The telemetry result indicate that unexplained install failures are very rare,
so we don't need to bother keeping this probe.

We should still need to check whether the GMP files disappear from disk, as
telemetry indicates this does happen, though quite rarely.

Review commit: https://reviewboard.mozilla.org/r/66256/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/66256/
Comment on attachment 8773564 [details]
Bug 1288311 - Remove GMP install failed/unsupported telemetry reporting code.

https://reviewboard.mozilla.org/r/66256/#review63090

::: toolkit/mozapps/extensions/internal/GMPProvider.jsm:515
(Diff revision 1)
>      // Installed -> Check if files are missing.
> -    let status = this._arePluginFilesOnDisk();
> -    status.installed = true;
> -    status.mismatchedABI = false;
> -    status.valid = true;
> -    status.missing = [];
> +    let filesOnDisk = this._arePluginFilesOnDisk();
> +    return {
> +      installed: true,
> +      valid: filesOnDisk,
> +      missing: !filesOnDisk

The only place where we seem to be checking the value of |missing| is in the logging call in |GMPProvider::startup|. What do you think about just changing that log call and removing the |missing| property entirely?
Attachment #8773564 - Flags: review?(spohl.mozilla.bugs) → review+
Comment on attachment 8773563 [details]
Bug 1288311 - Remove GMP unsupported/installation failure telemetry probes.

https://reviewboard.mozilla.org/r/66254/#review63306

::: toolkit/components/telemetry/histogram-whitelists.json:2226
(Diff revision 1)
>      "URLCLASSIFIER_LOOKUP_TIME",
>      "URLCLASSIFIER_PS_CONSTRUCT_TIME",
>      "URLCLASSIFIER_PS_FAILURE",
>      "URLCLASSIFIER_PS_FALLOCATE_TIME",
>      "URLCLASSIFIER_PS_FILELOAD_TIME",
>      "VIDEO_ADOBE_GMP_DISAPPEARED",

Did you mean to leave these two here?
Attachment #8773563 - Flags: review?(francois)
Comment on attachment 8773562 [details]
Bug 1288311 - Extend VIDEO_CAN_CREATE_*_DECODER telemetry probes.

https://reviewboard.mozilla.org/r/66252/#review63308
Attachment #8773562 - Flags: review?(francois) → review+
(In reply to Stephen A Pohl [:spohl] from comment #4)
> Comment on attachment 8773564 [details]
> Bug 1288311 - Remove GMP install failed/unsupported telemetry reporting code.
> 
> https://reviewboard.mozilla.org/r/66256/#review63090
> 
> ::: toolkit/mozapps/extensions/internal/GMPProvider.jsm:515
> (Diff revision 1)
> >      // Installed -> Check if files are missing.
> > -    let status = this._arePluginFilesOnDisk();
> > -    status.installed = true;
> > -    status.mismatchedABI = false;
> > -    status.valid = true;
> > -    status.missing = [];
> > +    let filesOnDisk = this._arePluginFilesOnDisk();
> > +    return {
> > +      installed: true,
> > +      valid: filesOnDisk,
> > +      missing: !filesOnDisk
> 
> The only place where we seem to be checking the value of |missing| is in the
> logging call in |GMPProvider::startup|. What do you think about just
> changing that log call and removing the |missing| property entirely?

Sure. The only place that sets valid to false is the files missing path, so we can infer from the logging that there's files missing anyway.
Comment on attachment 8773562 [details]
Bug 1288311 - Extend VIDEO_CAN_CREATE_*_DECODER telemetry probes.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/66252/diff/1-2/
Attachment #8773563 - Flags: review?(francois)
Comment on attachment 8773563 [details]
Bug 1288311 - Remove GMP unsupported/installation failure telemetry probes.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/66254/diff/1-2/
Comment on attachment 8773564 [details]
Bug 1288311 - Remove GMP install failed/unsupported telemetry reporting code.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/66256/diff/1-2/
https://reviewboard.mozilla.org/r/66254/#review63306

> Did you mean to leave these two here?

Nope! Fixed.
Attachment #8773563 - Flags: review?(francois) → review+
Comment on attachment 8773563 [details]
Bug 1288311 - Remove GMP unsupported/installation failure telemetry probes.

https://reviewboard.mozilla.org/r/66254/#review63684
Comment on attachment 8773562 [details]
Bug 1288311 - Extend VIDEO_CAN_CREATE_*_DECODER telemetry probes.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/66252/diff/2-3/
Comment on attachment 8773563 [details]
Bug 1288311 - Remove GMP unsupported/installation failure telemetry probes.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/66254/diff/2-3/
Comment on attachment 8773564 [details]
Bug 1288311 - Remove GMP install failed/unsupported telemetry reporting code.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/66256/diff/2-3/
Pushed by cpearce@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/44c1818d4f7d
Extend VIDEO_CAN_CREATE_*_DECODER telemetry probes. r=francois
https://hg.mozilla.org/integration/autoland/rev/0c933f4358cc
Remove GMP unsupported/installation failure telemetry probes. r=francois
https://hg.mozilla.org/integration/autoland/rev/3c891b1d6811
Remove GMP install failed/unsupported telemetry reporting code. r=spohl
You need to log in before you can comment on or make changes to this bug.