1238433 - Add telemetry to report proportion of frame drops during video playback

Status: RESOLVED FIXED

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

Description

[Chris Pearce [:cpearce (Not reading bugmail)]]

We should add telemetry to report the proportion of frames that are dropped during playback of video. Then we can observe this fluctuating over time to figure out if we're improving performance.

I suggest that we do this in the same place that we report the MSE telemetry.

We already have the playback quality statistics returned by HTMLVideoElement::GetVideoPlaybackQuality(), it's just a matter of determining whether our media element is playing a video, and if so getting a video element pointer and calling its GetVideoPlaybackQuality function and reporting the results.

You can convert from an HTMLMediaElement to an HTMLVideoElement by calling HTMLVideoElement::FromContentOrNull(this) from inside an HTMLMediaElement function I believe, for example (but not from inside HTMLMediaElement): http://mxr.mozilla.org/mozilla-central/source/layout/base/nsLayoutUtils.cpp#7192

Comment 1

[Louis Christie [:lchristie]]

Attachment: bug-1238433-fix.patch

Comment 2

[Louis Christie [:lchristie]]

Attachment: bug-1238433-fix.patch

Comment 3

[Chris Pearce [:cpearce (Not reading bugmail)]]

Comment on attachment 8718198 [details] [diff] [review]
bug-1238433-fix.patch

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

I think this is a good start. We'll want more precise telemetry which reports frame drops for different resolutions and formats and values of hardware accelerated video decoding. But this is a good aggregate to start.

::: toolkit/components/telemetry/Histograms.json
@@ +10167,5 @@
> +    "alert_emails": ["[e-mail]", "[e-mail]"],
> +    "expires_in_version": "55",
> +    "kind": "linear",
> +    "high": "100",
> +    "n_buckets": 20,

I think you need "n_buckets": "101", as we want higher precision than just 5% increments, and in order to record both values 0 and 100% you need to include the range [0,1...100], i.e. 101 values.

Comment 4

[Louis Christie [:lchristie]]

Attachment: bug-1238433-fix.patch

Amended with cpearce's comments

Comment 5

[Benjamin Smedberg]

Comment on attachment 8718512 [details] [diff] [review]
bug-1238433-fix.patch

Seems pretty straightforward. You'll probably want correlates in another iteration (video size/frame rate in particular), but good to start with the simple thing.

Comment 6

[Louis Christie [:lchristie]]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=936611ecc231

Comment 7

[Louis Christie [:lchristie]]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=a1b2e693c7b3

Comment 8

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/c7be30a5321e

Comment 9

[Carsten Book [:Tomcat]]

backed out for bustage like https://treeherder.mozilla.org/logviewer.html#?job_id=21908898&repo=mozilla-inbound

Comment 10

[Louis Christie [:lchristie]]

Whoops, didn't upload the version in the last try push.

Comment 11

[Louis Christie [:lchristie]]

Attachment: bug-1238433-fix.patch

Comment 12

[Pulsebot]

Backout:
https://hg.mozilla.org/integration/mozilla-inbound/rev/f5a6fa884f9f

Comment 13

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/cc4950c828a1

Comment 14

[Louis Christie [:lchristie]]

Will follow up with more detailed telemetry in bug 1249706

Comment 15

[Phil Ringnalda (:philor)]

https://hg.mozilla.org/mozilla-central/rev/cc4950c828a1

Comment 16

[Phil Ringnalda (:philor)]

https://hg.mozilla.org/mozilla-central/rev/af6356a3e8c5