1127646 - Report join latency and mean time between rebuffers in telemetry

Status: RESOLVED FIXED

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

Description

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

In addition to the MSE telementry in bug 1119947, we should also report the rebuffer rate, and the mean time between rebuffers.

"rebuffer" here refers to the time spent buffering, i.e. time where the throbber shows up.

Comment 1

[Chris Peterson [:cpeterson]]

The primary metrics YouTube cares about are:

* Above the Fold Time (AFT): time between _page_ onload and video playback.
* Join Latency: time between _video_ load and video playback.
* Mean Time Between Rebuffering (MTBR): play time between rebuffering hiccups.

Can we add telemetry measuring AFT and join latency for HTML5 video? These measurements would probably be impractical for Flash video.

Comment 2

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

Attachment: WIP Patch

Some fixes on top of my patch in bug 1119947.

Also changes when we report the telemetry to be (hopefully) more reliable, and fixes an issue with the buffered-ranges based test wasn't working properly.

Comment 3

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

cajbir: Are you able to take this off me?

Comment 4

[cajbir (:cajbir)]

Yep.

Comment 5

[Lawrence Mandel [:lmandel] (use needinfo)]

Tracking all MSE P1 bugs for Firefox 37.

Comment 6

[cajbir (:cajbir)]

Try: https://treeherder.mozilla.org/#/jobs?repo=try&revision=254fd669d1ea

Comment 7

[cajbir (:cajbir)]

Attachment: Report join latency and MTBR stats

Reports mean time between rebuffering and join latency. :cpearce for media changes, :bsmedberg for telemetry changes.

Comment 8

[cajbir (:cajbir)]

New try build with unused function removed: https://treeherder.mozilla.org/#/jobs?repo=try&revision=51be4f4ed4f2

Comment 9

[cajbir (:cajbir)]

Attachment: Report join latency and MTBR stats

Remove unused function from last patch which broke try. Review requested from :cpearce for media code and :bsmedberg for telemetry code.

Comment 10

[Benjamin Smedberg]

I don't quite understand the "mean" measurement here. Is there a reason we need to accumulate this "mtbf" separately instead of just having a histogram for total play time and a separate histogram for rebuffering length? That would let us calculate MTBR plus look at the distribution of rebuffering delays (many short ones or a few long ones).

Comment 11

[cajbir (:cajbir)]

We want to track mtbr and see whether it is increasing or decreasing over time. If we accumulate them value separately as you suggest is this still possible? How would one calculate the mtbr from the telemetry dashboard? I have no problem changing it if it's possible.

Comment 12

[cajbir (:cajbir)]

needinfo :bsmedberg for comment 11.

Comment 13

[Benjamin Smedberg]

The telemetry.mozilla.org infra can't show you that because it's a relation between two separate datapoints. But with a histogram of pauses and a counter/histogram of total time, the realtime dashboard can graph that over time. File a dashboard bug and it'll be quick!

Comment 14

[cajbir (:cajbir)]

Comment on attachment 8570189 [details] [diff] [review]
Report join latency and MTBR stats

Remove review request until I address :bsmedberg's comments.

Comment 15

[cajbir (:cajbir)]

Attachment: Report join latency and MTBR stats

Splits MTBR up into total play time and count of buffering as suggested by :bsmedberg. I'll raise the telemetry dashboard bug to enable reporting on these as a combined MBTR value shortly.

Review requested from :bsmedberg for telemetry code, :cpearce for the rest.

Comment 16

[cajbir (:cajbir)]

Raised bug 1141915 for the telemetry dashboard change mentioned in comment 13.

Comment 17

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

Comment on attachment 8575766 [details] [diff] [review]
Report join latency and MTBR stats

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

Do we want to report the mean rebuffering time too? If that number goes down over time, our users' experience gets better right?

::: dom/html/HTMLMediaElement.cpp
@@ +2541,5 @@
>    return rv;
>  }
>  
> +static double
> +Rate(const HTMLMediaElement::TimeDurationAccumulator& a,

Rate() is unused, can be removed.

Comment 18

[Chris Peterson [:cpeterson]]

(In reply to Chris Pearce (:cpearce) from comment #17)
> Do we want to report the mean rebuffering time too? If that number goes down
> over time, our users' experience gets better right?

YouTube measures MTBR, Mean Time *Between* Rebuffering, where bigger is better. This patch's mRebufferTime looks like it is measuring something different: the cumulative time the user spends watching a rebuffering spinner.

I don't know which metric would be a better indicator for user experience.

Comment 19

[Benjamin Smedberg]

Comment on attachment 8575766 [details] [diff] [review]
Report join latency and MTBR stats

Is this VIDEO_MSE_BUFFERING_COUNT instead of a histogram VIDEO_MSE_BUFFERING_LENGTHS because a histogram of lengths is too complex to code? I'm ok with this version, but having an actual histogram of lengths would let us calculate the amount of time spent buffering as well.

Also ISTM you wouldn't need the TimeDurationAccumulator if you wrote it like that: you could save the last played time and accumulate the histogram frequently, rather than once at the end.

I'm going to mark r+ but I feel like this could be simpler...

Comment 20

[cajbir (:cajbir)]

(In reply to Chris Peterson [:cpeterson] from comment #18)
> YouTube measures MTBR, Mean Time *Between* Rebuffering, where bigger is
> better. This patch's mRebufferTime looks like it is measuring something
> different: the cumulative time the user spends watching a rebuffering
> spinner.

This patch reports the total play time (ie. time spent not watching the spinner) and the total number of times buffering occurred (ie. count of times the spinner happened). From this we can work out the average time spent consecutively playing before a rebuffer happens. Bigger is better. Is that not what MTBR is?

Comment 21

[cajbir (:cajbir)]

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

Comment 22

[Chris Peterson [:cpeterson]]

Yes, that sounds correct. Reading the patch, I was confused that we were starting and stopping a timer called mRebufferTime. I didn't see that we are only reporting telemetry for mRebufferTime.Count().

Comment 23

[cajbir (:cajbir)]

https://treeherder.mozilla.org/#/jobs?repo=mozilla-inbound&revision=afb6a4080533

Comment 24

[Carsten Book [:Tomcat]]

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

Comment 25

[Lawrence Mandel [:lmandel] (use needinfo)]

This Telemetry addition is a little more involved than most. I had tracked this for 37 as it was marked as an MSE P1. Do you think that this is required for 37 or can we take this in 38 or 39 instead?

Comment 26

[Lawrence Mandel [:lmandel] (use needinfo)]

Ralph is OOO. ni on cpearce instead.

Comment 27

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

I am a little preoccupied, cajbir can you please handle the uplift of this, if we need it?

Comment 28

[cajbir (:cajbir)]

(In reply to Lawrence Mandel [:lmandel] (use needinfo) from comment #25)
> This Telemetry addition is a little more involved than most. I had tracked
> this for 37 as it was marked as an MSE P1. Do you think that this is
> required for 37 or can we take this in 38 or 39 instead?

It's not required for 37. We're waiting on a telemetry dashboard fix to view the data so it's not urgent.

Comment 29

[Ralph Giles (:rillian)]

Comment on attachment 8575766 [details] [diff] [review]
Report join latency and MTBR stats

I think we should get this into 38. It's now had a week to bake on nightly.

Approval Request Comment
[Feature/regressing bug #]: MSE
[User impact if declined]: Test data for feature evaluation becomes available more slowly.
[Describe test coverage new/current, TreeHerder]: Landed on m-c.
[Risks and why]: Change to add new telemetry is isolated and easy to disable. Risk looks low to me.
[String/UUID change made/needed]: None.

Comment 30

[Ralph Giles (:rillian)]

https://hg.mozilla.org/releases/mozilla-aurora/rev/d496848bee5f