Bug 1443942 - Move code to toggle high res timers into VideoSink. r?jya
We have code in the MDSM to toggle on high resolution timers on Windows when we
start/stop playing because the VideoSink relies on being awoken by timers to
update the set of current frames in the compositor's queue, and on Windows 7 we
end up dropping frames due to the timer lag without this.
We assert in the MDSM's destructor that we've turned off high res timers (as
they cause needless battery drain, so we only want them on when we need them),
and the new test_mediarecorder_principals is hitting that assert on Windows. I
think we're missing turning them off when we create a new VideoSink for
outputting to the MSG. That affects the value returned by
MediaDecoderStateMachine->mVideoSink->IsPlaying(), which is what we use to
decide whether we should enable high resolution timers. We track whether we've
enabled high res timers in MDSM::mHiResTimersRequested, and that gets out of
sync with IsPlaying() when we re-create the MediaSink.
Rather than trying to handle all the permutations of places where we need to
turn off high resolution timers in the MDSM, we're better to move the code to
toggle high res timers into the VideoSink, as that's actually where we need to
be sure that we have high resolution timers enabled anyway. It's the VideoSink
after all that is relying on timers for frame update, not the MDSM.
Also remove the media.hi-res-timers.enabled pref, as we haven't needed it.
MozReview-Commit-ID: 9dNxcYxPDZH
--- a/dom/media/MediaDecoderStateMachine.cpp
+++ b/dom/media/MediaDecoderStateMachine.cpp
@@ -1,33 +1,26 @@
/* -*- Mode: C++; tab-width: 2; indent-tabs-mode: nil; c-basic-offset: 2 -*- */
/* vim:set ts=2 sw=2 sts=2 et cindent: */
/* This Source Code Form is subject to the terms of the Mozilla Public
* License, v. 2.0. If a copy of the MPL was not distributed with this
* file, You can obtain one at http://mozilla.org/MPL/2.0/. */
-#ifdef XP_WIN
-// Include Windows headers required for enabling high precision timers.
-#include "windows.h"
-#include "mmsystem.h"
-#endif
-
#include <algorithm>
#include <stdint.h>
#include "mediasink/AudioSink.h"
#include "mediasink/AudioSinkWrapper.h"
#include "mediasink/DecodedStream.h"
#include "mediasink/OutputStreamManager.h"
#include "mediasink/VideoSink.h"
#include "mozilla/IndexSequence.h"
#include "mozilla/Logging.h"
#include "mozilla/MathAlgorithms.h"
#include "mozilla/NotNull.h"
-#include "mozilla/Preferences.h"
#include "mozilla/SharedThreadPool.h"
#include "mozilla/Sprintf.h"
#include "mozilla/Telemetry.h"
#include "mozilla/TaskQueue.h"
#include "mozilla/Tuple.h"
#include "nsIMemoryReporter.h"
#include "nsPrintfCString.h"
#include "nsTArray.h"
@@ -2700,19 +2693,16 @@ MediaDecoderStateMachine::MediaDecoderSt
INIT_MIRROR(mVolume, 1.0),
INIT_MIRROR(mPreservesPitch, true),
INIT_MIRROR(mLooping, false),
INIT_MIRROR(mSameOriginMedia, false),
INIT_MIRROR(mMediaPrincipalHandle, PRINCIPAL_HANDLE_NONE),
INIT_CANONICAL(mDuration, NullableTimeUnit()),
INIT_CANONICAL(mCurrentPosition, TimeUnit::Zero()),
INIT_CANONICAL(mIsAudioDataAudible, false)
-#ifdef XP_WIN
- , mShouldUseHiResTimers(Preferences::GetBool("media.hi-res-timers.enabled", true))
-#endif
{
MOZ_COUNT_CTOR(MediaDecoderStateMachine);
NS_ASSERTION(NS_IsMainThread(), "Should be on main thread.");
InitVideoQueuePrefs();
DDLINKCHILD("reader", aReader);
}
@@ -2720,20 +2710,16 @@ MediaDecoderStateMachine::MediaDecoderSt
#undef INIT_WATCHABLE
#undef INIT_MIRROR
#undef INIT_CANONICAL
MediaDecoderStateMachine::~MediaDecoderStateMachine()
{
MOZ_ASSERT(NS_IsMainThread(), "Should be on main thread.");
MOZ_COUNT_DTOR(MediaDecoderStateMachine);
-
-#ifdef XP_WIN
- MOZ_ASSERT(!mHiResTimersRequested);
-#endif
}
void
MediaDecoderStateMachine::InitializationTask(MediaDecoder* aDecoder)
{
MOZ_ASSERT(OnTaskQueue());
// Connect mirrors.
@@ -2924,22 +2910,16 @@ MediaDecoderStateMachine::StopPlayback()
MOZ_ASSERT(OnTaskQueue());
LOG("StopPlayback()");
if (IsPlaying()) {
mOnPlaybackEvent.Notify(MediaPlaybackEvent{
MediaPlaybackEvent::PlaybackStopped, mPlaybackOffset });
mMediaSink->SetPlaying(false);
MOZ_ASSERT(!IsPlaying());
-#ifdef XP_WIN
- if (mHiResTimersRequested) {
- mHiResTimersRequested = false;
- timeEndPeriod(1);
- }
-#endif
}
}
void MediaDecoderStateMachine::MaybeStartPlayback()
{
MOZ_ASSERT(OnTaskQueue());
// Should try to start playback only after decoding first frames.
MOZ_ASSERT(mSentFirstFrameLoadedEvent);
@@ -2952,30 +2932,16 @@ void MediaDecoderStateMachine::MaybeStar
if (mPlayState != MediaDecoder::PLAY_STATE_PLAYING) {
LOG("Not starting playback [mPlayState=%d]", mPlayState.Ref());
return;
}
LOG("MaybeStartPlayback() starting playback");
StartMediaSink();
-#ifdef XP_WIN
- if (!mHiResTimersRequested && mShouldUseHiResTimers) {
- mHiResTimersRequested = true;
- // Ensure high precision timers are enabled on Windows, otherwise the state
- // machine isn't woken up at reliable intervals to set the next frame, and we
- // drop frames while painting. Note that each call must be matched by a
- // corresponding timeEndPeriod() call. Enabling high precision timers causes
- // the CPU to wake up more frequently on Windows 7 and earlier, which causes
- // more CPU load and battery use. So we only enable high precision timers
- // when we're actually playing.
- timeBeginPeriod(1);
- }
-#endif
-
if (!IsPlaying()) {
mMediaSink->SetPlaying(true);
MOZ_ASSERT(IsPlaying());
}
mOnPlaybackEvent.Notify(
MediaPlaybackEvent{ MediaPlaybackEvent::PlaybackStarted, mPlaybackOffset });
}
--- a/dom/media/MediaDecoderStateMachine.h
+++ b/dom/media/MediaDecoderStateMachine.h
@@ -746,24 +746,13 @@ public:
AbstractCanonical<media::TimeUnit>* CanonicalCurrentPosition()
{
return &mCurrentPosition;
}
AbstractCanonical<bool>* CanonicalIsAudioDataAudible()
{
return &mIsAudioDataAudible;
}
-
-#ifdef XP_WIN
- // Whether we've called timeBeginPeriod(1) to request high resolution
- // timers. We request high resolution timers when playback starts, and
- // turn them off when playback is paused. Enabling high resolution
- // timers can cause higher CPU usage and battery drain on Windows 7.
- bool mHiResTimersRequested = false;
- // Whether we should enable high resolution timers. This is initialized at
- // MDSM construction, and mirrors the value of media.hi-res-timers.enabled.
- const bool mShouldUseHiResTimers;
-#endif
};
} // namespace mozilla
#endif
--- a/dom/media/mediasink/VideoSink.cpp
+++ b/dom/media/mediasink/VideoSink.cpp
@@ -1,14 +1,20 @@
/* -*- Mode: C++; tab-width: 8; indent-tabs-mode: nil; c-basic-offset: 2 -*- */
/* vim: set ts=8 sts=2 et sw=2 tw=80: */
/* This Source Code Form is subject to the terms of the Mozilla Public
* License, v. 2.0. If a copy of the MPL was not distributed with this
* file, You can obtain one at http://mozilla.org/MPL/2.0/. */
+#ifdef XP_WIN
+// Include Windows headers required for enabling high precision timers.
+#include "windows.h"
+#include "mmsystem.h"
+#endif
+
#include "MediaQueue.h"
#include "VideoSink.h"
#include "MediaPrefs.h"
#include "VideoUtils.h"
#include "mozilla/IntegerPrintfMacros.h"
namespace mozilla {
@@ -40,22 +46,29 @@ VideoSink::VideoSink(AbstractThread* aTh
, mVideoQueue(aVideoQueue)
, mContainer(aContainer)
, mProducerID(ImageContainer::AllocateProducerID())
, mFrameStats(aFrameStats)
, mHasVideo(false)
, mUpdateScheduler(aThread)
, mVideoQueueSendToCompositorSize(aVQueueSentToCompositerSize)
, mMinVideoQueueSize(MediaPrefs::RuinAvSync() ? 1 : 0)
+#ifdef XP_WIN
+ , mHiResTimersRequested(false)
+#endif
+
{
MOZ_ASSERT(mAudioSink, "AudioSink should exist.");
}
VideoSink::~VideoSink()
{
+#ifdef XP_WIN
+ MOZ_ASSERT(!mHiResTimersRequested);
+#endif
}
const MediaSink::PlaybackParams&
VideoSink::GetPlaybackParams() const
{
AssertOwnerThread();
return mAudioSink->GetPlaybackParams();
@@ -133,16 +146,40 @@ void
VideoSink::SetPreservesPitch(bool aPreservesPitch)
{
AssertOwnerThread();
mAudioSink->SetPreservesPitch(aPreservesPitch);
}
void
+VideoSink::EnsureHighResTimersOnOnlyIfPlaying()
+{
+#ifdef XP_WIN
+ const bool needed = IsPlaying();
+ if (needed == mHiResTimersRequested) {
+ return;
+ }
+ if (needed) {
+ // Ensure high precision timers are enabled on Windows, otherwise the VideoSink
+ // isn't woken up at reliable intervals to set the next frame, and we
+ // drop frames while painting. Note that each call must be matched by a
+ // corresponding timeEndPeriod() call. Enabling high precision timers causes
+ // the CPU to wake up more frequently on Windows 7 and earlier, which causes
+ // more CPU load and battery use. So we only enable high precision timers
+ // when we're actually playing.
+ timeBeginPeriod(1);
+ } else {
+ timeEndPeriod(1);
+ }
+ mHiResTimersRequested = needed;
+#endif
+}
+
+void
VideoSink::SetPlaying(bool aPlaying)
{
AssertOwnerThread();
VSINK_LOG_V(" playing (%d) -> (%d)", mAudioSink->IsPlaying(), aPlaying);
if (!aPlaying) {
// Reset any update timer if paused.
mUpdateScheduler.Reset();
@@ -156,16 +193,18 @@ VideoSink::SetPlaying(bool aPlaying)
mAudioSink->SetPlaying(aPlaying);
if (mHasVideo && aPlaying) {
// There's no thread in VideoSink for pulling video frames, need to trigger
// rendering while becoming playing status. because the VideoQueue may be
// full already.
TryUpdateRenderedVideoFrames();
}
+
+ EnsureHighResTimersOnOnlyIfPlaying();
}
void
VideoSink::Start(const TimeUnit& aStartTime, const MediaInfo& aInfo)
{
AssertOwnerThread();
VSINK_LOG("[%s]", __func__);
@@ -219,16 +258,18 @@ VideoSink::Stop()
mUpdateScheduler.Reset();
if (mHasVideo) {
DisconnectListener();
mVideoSinkEndRequest.DisconnectIfExists();
mEndPromiseHolder.ResolveIfExists(true, __func__);
mEndPromise = nullptr;
}
mVideoFrameEndTime = TimeUnit::Zero();
+
+ EnsureHighResTimersOnOnlyIfPlaying();
}
bool
VideoSink::IsStarted() const
{
AssertOwnerThread();
return mAudioSink->IsStarted();
--- a/dom/media/mediasink/VideoSink.h
+++ b/dom/media/mediasink/VideoSink.h
@@ -74,16 +74,18 @@ private:
virtual ~VideoSink();
// VideoQueue listener related.
void OnVideoQueuePushed(RefPtr<VideoData>&& aSample);
void OnVideoQueueFinished();
void ConnectListener();
void DisconnectListener();
+ void EnsureHighResTimersOnOnlyIfPlaying();
+
// Sets VideoQueue images into the VideoFrameContainer. Called on the shared
// state machine thread. The first aMaxFrames (at most) are set.
// aClockTime and aClockTimeStamp are used as the baseline for deriving
// timestamps for the frames; when omitted, aMaxFrames must be 1 and
// a null timestamp is passed to the VideoFrameContainer.
// If the VideoQueue is empty, this does nothing.
void RenderVideoFrames(int32_t aMaxFrames, int64_t aClockTime = 0,
const TimeStamp& aClickTimeStamp = TimeStamp());
@@ -146,14 +148,23 @@ private:
// Talos tests for the compositor require at least one frame in the
// video queue so that the compositor has something to composit during
// the talos test when the decode is stressed. We have a minimum size
// on the video queue in order to facilitate this talos test.
// Note: Normal playback should not have a queue size of more than 0,
// otherwise A/V sync will be ruined! *Only* make this non-zero for
// testing purposes.
const uint32_t mMinVideoQueueSize;
+
+#ifdef XP_WIN
+ // Whether we've called timeBeginPeriod(1) to request high resolution
+ // timers. We request high resolution timers when playback starts, and
+ // turn them off when playback is paused. Enabling high resolution
+ // timers can cause higher CPU usage and battery drain on Windows 7,
+ // but reduces our frame drop rate.
+ bool mHiResTimersRequested;
+#endif
};
} // namespace media
} // namespace mozilla
#endif