From 7cb5d740eadb3697361147cd28170c81d2f3a9bc Mon Sep 17 00:00:00 2001 From: JW Wang Date: Wed, 2 Apr 2014 08:51:47 -0400 Subject: [PATCH] Bug 990356 - Part 2: Only one way to dispatch state machine (using a timer) tasks and remove unused variables. r=cpearce --- content/media/MediaDecoderStateMachine.cpp | 82 ++++------------------ content/media/MediaDecoderStateMachine.h | 27 ++----- 2 files changed, 22 insertions(+), 87 deletions(-) diff --git a/content/media/MediaDecoderStateMachine.cpp b/content/media/MediaDecoderStateMachine.cpp index ec1b63f4f844..1a633a12fcf3 100644 --- a/content/media/MediaDecoderStateMachine.cpp +++ b/content/media/MediaDecoderStateMachine.cpp @@ -160,6 +160,7 @@ MediaDecoderStateMachine::MediaDecoderStateMachine(MediaDecoder* aDecoder, bool aRealTime) : mDecoder(aDecoder), mState(DECODER_STATE_DECODING_METADATA), + mInRunningStateMachine(false), mSyncPointInMediaStream(-1), mSyncPointInDecodedStream(-1), mResetPlayStartTime(false), @@ -191,10 +192,7 @@ MediaDecoderStateMachine::MediaDecoderStateMachine(MediaDecoder* aDecoder, mDispatchedEventToDecode(false), mStopAudioThread(true), mQuickBuffering(false), - mIsRunning(false), - mRunAgain(false), mMinimizePreroll(false), - mDispatchedRunEvent(false), mDecodeThreadWaiting(false), mRealTime(aRealTime), mEventManager(aDecoder), @@ -2708,40 +2706,26 @@ nsresult MediaDecoderStateMachine::GetBuffered(dom::TimeRanges* aBuffered) { nsresult MediaDecoderStateMachine::Run() { - ReentrantMonitorAutoEnter mon(mDecoder->GetReentrantMonitor()); - NS_ASSERTION(OnStateMachineThread(), "Should be on state machine thread."); - - return CallRunStateMachine(); + // only allowing scheduling with timer task + MOZ_ASSERT(false); + return NS_ERROR_FAILURE; } nsresult MediaDecoderStateMachine::CallRunStateMachine() { AssertCurrentThreadInMonitor(); NS_ASSERTION(OnStateMachineThread(), "Should be on state machine thread."); - // This will be set to true by ScheduleStateMachine() if it's called - // while we're in RunStateMachine(). - mRunAgain = false; - - // Set to true whenever we dispatch an event to run this state machine. - // This flag prevents us from dispatching - mDispatchedRunEvent = false; // If audio is being captured, stop the audio thread if it's running if (mAudioCaptured) { StopAudioThread(); } + MOZ_ASSERT(!mInRunningStateMachine, "State machine cycles must run in sequence!"); mTimeout = TimeStamp(); - - mIsRunning = true; + mInRunningStateMachine = true; nsresult res = RunStateMachine(); - mIsRunning = false; - - if (mRunAgain && !mDispatchedRunEvent) { - mDispatchedRunEvent = true; - return GetStateMachineThread()->Dispatch(this, NS_DISPATCH_NORMAL); - } - + mInRunningStateMachine = false; return res; } @@ -2756,16 +2740,7 @@ void MediaDecoderStateMachine::TimeoutExpired() { ReentrantMonitorAutoEnter mon(mDecoder->GetReentrantMonitor()); NS_ASSERTION(OnStateMachineThread(), "Must be on state machine thread"); - if (mIsRunning) { - mRunAgain = true; - } else if (!mDispatchedRunEvent) { - // We don't have an event dispatched to run the state machine, so we - // can just run it from here. - CallRunStateMachine(); - } - // Otherwise, an event has already been dispatched to run the state machine - // as soon as possible. Nothing else needed to do, the state machine is - // going to run anyway. + CallRunStateMachine(); } void MediaDecoderStateMachine::ScheduleStateMachineWithLockAndWakeDecoder() { @@ -2785,46 +2760,19 @@ nsresult MediaDecoderStateMachine::ScheduleStateMachine(int64_t aUsecs) { aUsecs = std::max(aUsecs, 0); TimeStamp timeout = TimeStamp::Now() + UsecsToDuration(aUsecs); - if (!mTimeout.IsNull()) { - if (timeout >= mTimeout) { - // We've already scheduled a timer set to expire at or before this time, - // or have an event dispatched to run the state machine. - return NS_OK; - } - // We've been asked to schedule a timer to run before an existing timer. - // Cancel the existing timer. - mTimer->Cancel(); + if (!mTimeout.IsNull() && timeout >= mTimeout) { + // We've already scheduled a timer set to expire at or before this time, + // or have an event dispatched to run the state machine. + return NS_OK; } uint32_t ms = static_cast((aUsecs / USECS_PER_MS) & 0xFFFFFFFF); - if (mRealTime && ms > 40) + if (mRealTime && ms > 40) { ms = 40; - if (ms == 0) { - if (mIsRunning) { - // We're currently running this state machine on the state machine - // thread. Signal it to run again once it finishes its current cycle. - mRunAgain = true; - return NS_OK; - } else if (!mDispatchedRunEvent) { - // We're not currently running this state machine on the state machine - // thread. Dispatch an event to run one cycle of the state machine. - mDispatchedRunEvent = true; - return GetStateMachineThread()->Dispatch(this, NS_DISPATCH_NORMAL); - } - // We're not currently running this state machine on the state machine - // thread, but something has already dispatched an event to run it again, - // so just exit; it's going to run real soon. - return NS_OK; } - - // Since there is already a pending task that will run immediately, - // we don't need to schedule a timer task. - if (mRunAgain) { - return NS_OK; - } - mTimeout = timeout; - + // Cancel existing timer if any since we are going to schedule a new one. + mTimer->Cancel(); nsresult rv = mTimer->InitWithFuncCallback(mozilla::TimeoutExpired, this, ms, diff --git a/content/media/MediaDecoderStateMachine.h b/content/media/MediaDecoderStateMachine.h index ccfdc3538800..c578f298dcdb 100644 --- a/content/media/MediaDecoderStateMachine.h +++ b/content/media/MediaDecoderStateMachine.h @@ -646,7 +646,7 @@ private: bool IsStateMachineScheduled() const { AssertCurrentThreadInMonitor(); - return !mTimeout.IsNull() || mRunAgain; + return !mTimeout.IsNull(); } // Returns true if we're not playing and the decode thread has filled its @@ -682,15 +682,17 @@ private: RefPtr mStateMachineThreadPool; - // Timer to call the state machine Run() method. Used by + // Timer to run the state machine cycles. Used by // ScheduleStateMachine(). Access protected by decoder monitor. nsCOMPtr mTimer; - // Timestamp at which the next state machine Run() method will be called. - // If this is non-null, a call to Run() is scheduled, either by a timer, - // or via an event. Access protected by decoder monitor. + // Timestamp at which the next state machine cycle will run. + // Access protected by decoder monitor. TimeStamp mTimeout; + // Used to check if there are state machine cycles are running in sequence. + DebugOnly mInRunningStateMachine; + // The time that playback started from the system clock. This is used for // timing the presentation of video frames when there's no audio. // Accessed only via the state machine thread. @@ -920,14 +922,6 @@ private: // Synchronised via decoder monitor. bool mQuickBuffering; - // True if the shared state machine thread is currently running this - // state machine. - bool mIsRunning; - - // True if we should run the state machine again once the current - // state machine run has finished. - bool mRunAgain; - // True if we should not decode/preroll unnecessary samples, unless we're // played. "Prerolling" in this context refers to when we decode and // buffer decoded samples in advance of when they're needed for playback. @@ -940,13 +934,6 @@ private: // memory and CPU overhead. bool mMinimizePreroll; - // True if we've dispatched an event to run the state machine. It's - // imperative that we don't dispatch multiple events to run the state - // machine at the same time, as our code assume all events are synchronous. - // If we dispatch multiple events, the second event can run while the - // first is shutting down a thread, causing inconsistent state. - bool mDispatchedRunEvent; - // True if the decode thread has gone filled its buffers and is now // waiting to be awakened before it continues decoding. Synchronized // by the decoder monitor.