Closed Bug 1361629 Opened 3 years ago Closed 3 years ago

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

Categories

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

Unspecified
Windows 7
enhancement
Not set

Tracking

()

RESOLVED FIXED
mozilla55
Tracking Status
firefox55 --- fixed

People

(Reporter: cpearce, Unassigned)

References

(Blocks 1 open bug)

Details

(Whiteboard: [qf][bhr])

Attachments

(1 file)

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/
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
Blocks: QuantumFlow
Whiteboard: [qf]
Note Windows 8 and later have a tickless kernel, so don't suffer from having hi resolution timers requested.
OS: Unspecified → Windows 7
Whiteboard: [qf] → [qf][bhr]
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.
Attachment #8864716 - Flags: review?(jwwang) → review+
Pushed by cpearce@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/a343a3fc0558
Only call timeBeginPeriod/timeEndPeriod when media playback starts/stops. r=jwwang
https://hg.mozilla.org/mozilla-central/rev/a343a3fc0558
Status: NEW → RESOLVED
Closed: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla55
You need to log in before you can comment on or make changes to this bug.