1361629 - MediaDecoderStateMachine should call timeBeginPeriod(1) while playing rather than upon creation

Status: RESOLVED FIXED

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

Description

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

On Windows we call timeBeginPeriod(1) to ensure our timers are accurate enough to be used to drive A/V sync (normally on Windows timers have a precision of 64Hz  or 15ms). But on Windows 7 calling timeBeginPeriod(1) increases the power drain [1]. We really only need higher precision timers while playing, so we should move the calls to timeBeginPeriod(1)/timeEndPeriod(1) out of the MediaDecoderStateMachine constructor/destructor and instead set the time period when we begin playback and unset it when we're paused.

Ideally if we've shutdown the video decoder due to being in the background we could reduce the timer precision too.

[1] https://randomascii.wordpress.com/2013/07/08/windows-timer-resolution-megawatts-wasted/

Comment 1

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

I've also got 9 stacks in the recent BHR report [1] with timeBeginPeriod(1) in them, such as:

  timeBeginPeriod (in kernel32.pdb)
  mozilla::MediaDecoderStateMachine::MediaDecoderStateMachine(mozilla::MediaDecoder *,mozilla::MediaDecoderReader *) (in xul.pdb)
  mozilla::MediaSourceDecoder::CreateStateMachine() (in xul.pdb)
  mozilla::MediaSourceDecoder::Load(nsIStreamListener * *) (in xul.pdb)
  mozilla::dom::HTMLMediaElement::FinishDecoderSetup(mozilla::MediaDecoder *,mozilla::MediaResource *,nsIStreamListener * *) (in xul.pdb)
  mozilla::dom::HTMLMediaElement::LoadResource() (in xul.pdb)
  mozilla::dom::HTMLMediaElement::SelectResource() (in xul.pdb)
  mozilla::dom::HTMLMediaElement::SelectResourceWrapper() (in xul.pdb)
  mozilla::detail::RunnableMethodImpl<mozilla::Canonical<double>::Impl * const,void ( mozilla::Canonical<double>::Impl::*)(void),1,0>::Run() (in xul.pdb)
  mozilla::dom::nsSyncSection::Run() (in xul.pdb)
  mozilla::CycleCollectedJSContext::ProcessStableStateQueue() (in xul.pdb)
  XPCJSContext::AfterProcessTask(unsigned int) (in xul.pdb)


I don't understand how timeBeginPeriod could hang on us (maybe it takes a lock held by some other process also trying to set the time period?), but moving our call to timeBeginPeriod(1) onto the state machine thread would at least mean we didn't block the main thread when timeBeginPeriod did take ages.

As such, I'm marking this for quantum flow triage.

[1] https://people-mozilla.org/~mlayzell/bhr/20170429/hangs.json

Comment 2

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

Note Windows 8 and later have a tickless kernel, so don't suffer from having hi resolution timers requested.

Comment 3

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

Attachment: Bug 1361629 - Only call timeBeginPeriod/timeEndPeriod when media playback starts/stops.

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

Comment 4

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

Comment on attachment 8864716 [details]
Bug 1361629 - Only call timeBeginPeriod/timeEndPeriod when media playback starts/stops.

https://reviewboard.mozilla.org/r/136066/#review139458

::: dom/media/MediaDecoderStateMachine.cpp:2922
(Diff revision 1)
>    LOG("MaybeStartPlayback() starting playback");
>    mOnPlaybackEvent.Notify(MediaEventType::PlaybackStarted);
>    StartMediaSink();
>  
>    if (!IsPlaying()) {
> +#ifdef XP_WIN

Move this block above if (!IsPlaying()) since IsPlaying() might be already true after StartMediaSink() returns.

Comment 5

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

Comment on attachment 8864716 [details]
Bug 1361629 - Only call timeBeginPeriod/timeEndPeriod when media playback starts/stops.

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

Comment 6

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/a343a3fc0558
Only call timeBeginPeriod/timeEndPeriod when media playback starts/stops. r=jwwang

Comment 7

[Carsten Book [:Tomcat]]

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