Closed Bug 1430948 Opened 3 years ago Closed 3 years ago

Add fuzzy mode to MediaTimer

Categories

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

enhancement

Tracking

()

RESOLVED FIXED
mozilla59
Tracking Status
firefox59 --- fixed

People

(Reporter: jwwang, Assigned: jwwang)

Details

Attachments

(2 files)

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.
Assignee: nobody → jwwang
Priority: -- → P3
Attachment #8943143 - Flags: review?(jyavenard)
Attachment #8943144 - Flags: review?(jyavenard)
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
Attachment #8943143 - Flags: review?(jyavenard) → review+
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
Attachment #8943144 - Flags: review?(jyavenard) → review+
Thanks!
Pushed by jwwang@mozilla.com:
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
https://hg.mozilla.org/mozilla-central/rev/d8fec54a8cc9
https://hg.mozilla.org/mozilla-central/rev/b0fdde0a9b35
Status: NEW → RESOLVED
Closed: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla59
You need to log in before you can comment on or make changes to this bug.