Closed Bug 1293145 Opened 3 years ago Closed 3 years ago

Telemetry to support background video decoder suspend: Percentage video-decode-suspended/total play time

Categories

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

50 Branch
defect
Not set

Tracking

()

RESOLVED FIXED
mozilla51
Tracking Status
firefox51 --- fixed

People

(Reporter: gerald, Assigned: gerald)

References

(Blocks 2 open bugs)

Details

Attachments

(2 files)

To help tweak the decoder-suspend functionality, we want to measure how much video-decoding could actually be suspended (while hidden). This is measured by counting the time from the moment a video would stop being fully decoded (given by a pref'd time after it becomes hidden), until is it visible again and decoding would resume.
Unplayed or invalid videos are not counted.
Telemetry will be keyed by audio presence ('AV' with audio, 'V' video only), and by resolution ranges (e.g. '720<h<=1080'). An 'All' key will also accumulate all numbers.
Comment on attachment 8778753 [details]
Bug 1293145 - Histogram for VIDEO_INFERRED_DECODE_SUSPEND_PERCENTAGE -

https://reviewboard.mozilla.org/r/69918/#review67406

datareview+
Attachment #8778753 - Flags: review?(francois) → review+
Comment on attachment 8778753 [details]
Bug 1293145 - Histogram for VIDEO_INFERRED_DECODE_SUSPEND_PERCENTAGE -

https://reviewboard.mozilla.org/r/69918/#review67452

![Ship it!](https://qph.ec.quoracdn.net/main-qimg-0bf1d3de309a89d9ed70ae91f6271a7f)
Attachment #8778753 - Flags: review?(dglastonbury) → review+
Comment on attachment 8778754 [details]
Bug 1293145 - Simulate video-decode-suspend for telemetry purposes -

https://reviewboard.mozilla.org/r/69920/#review67456

![Ship it!](https://qph.ec.quoracdn.net/main-qimg-0bf1d3de309a89d9ed70ae91f6271a7f) (with nits)

::: dom/html/HTMLMediaElement.h:1129
(Diff revision 1)
>      GetPaused(&isPaused);
>      return isPaused;
>    }
>  
> +  /**
> +   * Video has been playing while hidden for a certain time.

* Video has been playing while hidden and, if feature is enabled, would trigger suspending decoder.

::: dom/html/HTMLMediaElement.h:1137
(Diff revision 1)
> +  static void VideoDecodeSuspendTimerCallback(nsITimer* aTimer, void* aClosure);
> +  /**
> +   * Video is playing while hidden.
> +   * Used to track hidden-video telemetry.
> +   */
> +  void VideoPlayingHidden();

HiddenVideoStart{|s|ed}??!

::: dom/html/HTMLMediaElement.h:1142
(Diff revision 1)
> +  void VideoPlayingHidden();
> +  /**
> +   * Video is not playing (anymore) while hidden.
> +   * Used to track hidden-video telemetry.
> +   */
> +  void VideoNotPlayingHidden();

HiddenVideo{Stop|Paused}??!
Attachment #8778754 - Flags: review?(dglastonbury) → review+
Pushed by gsquelart@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/dc798effc862
Histogram for VIDEO_INFERRED_DECODE_SUSPEND_PERCENTAGE - r=francois,kamidphish
https://hg.mozilla.org/integration/autoland/rev/41138b630cdc
Simulate video-decode-suspend for telemetry purposes - r=kamidphish
Backout by ihsiao@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/6cfdded0e980
Backed out changeset 41138b630cdc for nsCOMPtr crash \
https://hg.mozilla.org/integration/autoland/rev/8b38a9d10149
Backed out changeset dc798effc862 \
sorry have to back you out for the nsCOMPtr crash, https://treeherder.mozilla.org/logviewer.html#?job_id=1627052&repo=autoland#L10446
Flags: needinfo?(gsquelart)
My fault, I forgot to cancel the timer in its owner's destructor, and then I ignored the try issue because I thought it was the "usual" intermittent. :-(
Patch on the way...
Flags: needinfo?(gsquelart)
Comment on attachment 8778754 [details]
Bug 1293145 - Simulate video-decode-suspend for telemetry purposes -

https://reviewboard.mozilla.org/r/69920/#review67528
(In reply to Gerald Squelart [:gerald] from comment #11)
> My fault, I forgot to cancel the timer in its owner's destructor, and then I
> ignored the try issue because I thought it was the "usual" intermittent. :-(
> Patch on the way...

How about an auto timer that will cancel automatically to prevent UAF?
(In reply to JW Wang [:jwwang] from comment #14)
> How about an auto timer that will cancel automatically to prevent UAF?

Good idea! This should be compulsory, I'll investigate...

However, for consistency with existing timers in that class, I'll keep the manual cancellation+destruction for now.
Pushed by gsquelart@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/801017876064
Histogram for VIDEO_INFERRED_DECODE_SUSPEND_PERCENTAGE - r=francois,kamidphish
https://hg.mozilla.org/integration/autoland/rev/9040927a7db7
Simulate video-decode-suspend for telemetry purposes - r=kamidphish
Blocks: 1293922
https://hg.mozilla.org/mozilla-central/rev/801017876064
https://hg.mozilla.org/mozilla-central/rev/9040927a7db7
Status: NEW → RESOLVED
Closed: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla51
Blocks: 1299065
You need to log in before you can comment on or make changes to this bug.