1430948 - Add fuzzy mode to MediaTimer

Status: RESOLVED FIXED

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

Description

[JW Wang [:jwwang] [:jw_wang]]

It is observed that due to the inaccuracy of the XPCOM timer, a timer sometimes fires slightly before the scheduled target. This causes MediaTimer to fail the test at https://searchfox.org/mozilla-central/rev/48cbb200aa027a0a379b6004b6196a167344b865/dom/media/MediaTimer.cpp#128 and need to schedule a timer again with a very small timeout.

This is inefficient and sometimes unnecessary. E.g. MDSM schedule a timer to run MediaDecoderStateMachine::UpdatePlaybackPositionPeriodically() about every 40ms. It is fine for UpdatePlaybackPositionPeriodically() to run 39ms later instead precisely 40ms.

We will add a fuzzy mode to MediaTimer so the client code can turn it on if it allow the timer to be slightly (< 1ms) inaccurate.

Comment 1

[JW Wang [:jwwang] [:jw_wang]]

Attachment: Bug 1430948. P1 - add fuzzy mode to MediaTimer.

We will resolve the timer promise even if it is fired slightly before
the scheduled target. This is used by MDSM which doesn't require high-res
timers.

Review commit: https://reviewboard.mozilla.org/r/213436/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/213436/

Comment 2

[JW Wang [:jwwang] [:jw_wang]]

Attachment: Bug 1430948. P2 - turn on fuzzy mode for MDSM which doesn't require high resolution timers.

Review commit: https://reviewboard.mozilla.org/r/213438/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/213438/

Comment 3

[Jean-Yves Avenard [:jya]]

Comment on attachment 8943143 [details]
Bug 1430948. P1 - add fuzzy mode to MediaTimer.

https://reviewboard.mozilla.org/r/213436/#review219390

::: dom/media/MediaTimer.cpp:124
(Diff revision 1)
> +MediaTimer::IsExpired(const TimeStamp& aTarget, const TimeStamp& aNow)
> +{
> +  MOZ_ASSERT(OnMediaTimerThread());
> +  mMonitor.AssertCurrentThreadOwns();
> +  // Treat this timer as expired in fuzzy mode even if it is fired
> +  // slight (< 1ms) before the schedule target. So we don't need to schedule a

slightly

::: dom/media/MediaTimer.cpp:125
(Diff revision 1)
> +{
> +  MOZ_ASSERT(OnMediaTimerThread());
> +  mMonitor.AssertCurrentThreadOwns();
> +  // Treat this timer as expired in fuzzy mode even if it is fired
> +  // slight (< 1ms) before the schedule target. So we don't need to schedule a
> +  // timer with very small timout again when the client doesn't need a high-res

very small timeout

Comment 4

[Jean-Yves Avenard [:jya]]

Comment on attachment 8943144 [details]
Bug 1430948. P2 - turn on fuzzy mode for MDSM which doesn't require high resolution timers.

https://reviewboard.mozilla.org/r/213438/#review219392

Comment 5

[JW Wang [:jwwang] [:jw_wang]]

Comment on attachment 8943143 [details]
Bug 1430948. P1 - add fuzzy mode to MediaTimer.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/213436/diff/1-2/

Comment 6

[JW Wang [:jwwang] [:jw_wang]]

Comment on attachment 8943144 [details]
Bug 1430948. P2 - turn on fuzzy mode for MDSM which doesn't require high resolution timers.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/213438/diff/1-2/

Comment 7

[JW Wang [:jwwang] [:jw_wang]]

Thanks!

Comment 8

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/d8fec54a8cc9
P1 - add fuzzy mode to MediaTimer. r=jya
https://hg.mozilla.org/integration/autoland/rev/b0fdde0a9b35
P2 - turn on fuzzy mode for MDSM which doesn't require high resolution timers. r=jya

Comment 9

[Dorel Luca [:dluca]]

https://hg.mozilla.org/mozilla-central/rev/d8fec54a8cc9
https://hg.mozilla.org/mozilla-central/rev/b0fdde0a9b35