From 0560195a71ee26e8546075e56c49ff535fcf1767 Mon Sep 17 00:00:00 2001 From: Wei Jia Date: Wed, 20 Aug 2014 18:21:11 -0700 Subject: [PATCH] NuPlayerDriver: fix current position for stop and pause. When start() is called after EOS, it means restarting from the beginning of the stream. Fix racing conditon on accessing some members. Report seekTo position before any notifyPosition is called. Bug: 17031731 Bug: 17178928 Change-Id: I008b827288cf28d39e2a943373fe1e5d7d6c2595 --- .../nuplayer/NuPlayerDriver.cpp | 59 +++++++++++++++------- .../nuplayer/NuPlayerDriver.h | 2 + 2 files changed, 44 insertions(+), 17 deletions(-) diff --git a/media/libmediaplayerservice/nuplayer/NuPlayerDriver.cpp b/media/libmediaplayerservice/nuplayer/NuPlayerDriver.cpp index c0091bf300..60beb9d8ad 100644 --- a/media/libmediaplayerservice/nuplayer/NuPlayerDriver.cpp +++ b/media/libmediaplayerservice/nuplayer/NuPlayerDriver.cpp @@ -36,8 +36,8 @@ NuPlayerDriver::NuPlayerDriver() mAsyncResult(UNKNOWN_ERROR), mDurationUs(-1), mPositionUs(-1), - mNotifyTimeRealUs(0), - mPauseStartedTimeUs(0), + mNotifyTimeRealUs(-1), + mPauseStartedTimeUs(-1), mNumFramesTotal(0), mNumFramesDropped(0), mLooper(new ALooper), @@ -240,7 +240,14 @@ status_t NuPlayerDriver::start() { } case STATE_RUNNING: + { + if (mAtEOS) { + mPlayer->seekToAsync(0); + mAtEOS = false; + mPositionUs = -1; + } break; + } case STATE_PAUSED: case STATE_STOPPED_AND_PREPARED: @@ -255,6 +262,7 @@ status_t NuPlayerDriver::start() { } mState = STATE_RUNNING; + mPauseStartedTimeUs = -1; return OK; } @@ -268,7 +276,7 @@ status_t NuPlayerDriver::stop() { // fall through case STATE_PAUSED: - notifyListener(MEDIA_STOPPED); + notifyListener_l(MEDIA_STOPPED); // fall through case STATE_PREPARED: @@ -281,7 +289,7 @@ status_t NuPlayerDriver::stop() { default: return INVALID_OPERATION; } - mPauseStartedTimeUs = ALooper::GetNowUs(); + setPauseStartedTimeIfNeeded(); return OK; } @@ -295,7 +303,7 @@ status_t NuPlayerDriver::pause() { return OK; case STATE_RUNNING: - notifyListener(MEDIA_PAUSED); + notifyListener_l(MEDIA_PAUSED); mPlayer->pause(); break; @@ -303,7 +311,7 @@ status_t NuPlayerDriver::pause() { return INVALID_OPERATION; } - mPauseStartedTimeUs = ALooper::GetNowUs(); + setPauseStartedTimeIfNeeded(); mState = STATE_PAUSED; return OK; @@ -334,7 +342,7 @@ status_t NuPlayerDriver::seekTo(int msec) { { mAtEOS = false; // seeks can take a while, so we essentially paused - notifyListener(MEDIA_PAUSED); + notifyListener_l(MEDIA_PAUSED); mPlayer->seekToAsync(seekTimeUs); break; } @@ -343,6 +351,8 @@ status_t NuPlayerDriver::seekTo(int msec) { return INVALID_OPERATION; } + mPositionUs = seekTimeUs; + mNotifyTimeRealUs = -1; return OK; } @@ -351,10 +361,11 @@ status_t NuPlayerDriver::getCurrentPosition(int *msec) { if (mPositionUs < 0) { *msec = 0; + } else if (mNotifyTimeRealUs == -1) { + *msec = mPositionUs / 1000; } else { int64_t nowUs = - (mState != STATE_RUNNING ? - mPauseStartedTimeUs : ALooper::GetNowUs()); + (isPlaying() ? ALooper::GetNowUs() : mPauseStartedTimeUs); *msec = (mPositionUs + nowUs - mNotifyTimeRealUs + 500ll) / 1000; } @@ -388,7 +399,7 @@ status_t NuPlayerDriver::reset() { { CHECK(mIsAsyncPrepare); - notifyListener(MEDIA_PREPARED); + notifyListener_l(MEDIA_PREPARED); break; } @@ -397,7 +408,7 @@ status_t NuPlayerDriver::reset() { } if (mState != STATE_STOPPED) { - notifyListener(MEDIA_STOPPED); + notifyListener_l(MEDIA_STOPPED); } mState = STATE_RESET_IN_PROGRESS; @@ -552,10 +563,7 @@ void NuPlayerDriver::notifySeekComplete_l() { // no need to notify listener return; } - // note: notifyListener called with lock released - mLock.unlock(); - notifyListener(wasSeeking ? MEDIA_SEEK_COMPLETE : MEDIA_PREPARED); - mLock.lock(); + notifyListener_l(wasSeeking ? MEDIA_SEEK_COMPLETE : MEDIA_PREPARED); } void NuPlayerDriver::notifyFrameStats( @@ -587,11 +595,19 @@ status_t NuPlayerDriver::dump( void NuPlayerDriver::notifyListener( int msg, int ext1, int ext2, const Parcel *in) { + Mutex::Autolock autoLock(mLock); + notifyListener_l(msg, ext1, ext2, in); +} + +void NuPlayerDriver::notifyListener_l( + int msg, int ext1, int ext2, const Parcel *in) { switch (msg) { case MEDIA_PLAYBACK_COMPLETE: { if (mLooping) { + mLock.unlock(); mPlayer->seekToAsync(0); + mLock.lock(); break; } // fall through @@ -600,6 +616,7 @@ void NuPlayerDriver::notifyListener( case MEDIA_ERROR: { mAtEOS = true; + setPauseStartedTimeIfNeeded(); break; } @@ -607,7 +624,9 @@ void NuPlayerDriver::notifyListener( break; } + mLock.unlock(); sendEvent(msg, ext1, ext2, in); + mLock.lock(); } void NuPlayerDriver::notifySetDataSourceCompleted(status_t err) { @@ -637,12 +656,12 @@ void NuPlayerDriver::notifyPrepareCompleted(status_t err) { if (err == OK) { if (mIsAsyncPrepare) { - notifyListener(MEDIA_PREPARED); + notifyListener_l(MEDIA_PREPARED); } mState = STATE_PREPARED; } else { if (mIsAsyncPrepare) { - notifyListener(MEDIA_ERROR, MEDIA_ERROR_UNKNOWN, err); + notifyListener_l(MEDIA_ERROR, MEDIA_ERROR_UNKNOWN, err); } mState = STATE_UNPREPARED; } @@ -656,4 +675,10 @@ void NuPlayerDriver::notifyFlagsChanged(uint32_t flags) { mPlayerFlags = flags; } +void NuPlayerDriver::setPauseStartedTimeIfNeeded() { + if (mPauseStartedTimeUs == -1) { + mPauseStartedTimeUs = ALooper::GetNowUs(); + } +} + } // namespace android diff --git a/media/libmediaplayerservice/nuplayer/NuPlayerDriver.h b/media/libmediaplayerservice/nuplayer/NuPlayerDriver.h index 076493d6eb..b0a52ad060 100644 --- a/media/libmediaplayerservice/nuplayer/NuPlayerDriver.h +++ b/media/libmediaplayerservice/nuplayer/NuPlayerDriver.h @@ -120,6 +120,8 @@ private: int64_t mStartupSeekTimeUs; status_t prepare_l(); + void notifyListener_l(int msg, int ext1 = 0, int ext2 = 0, const Parcel *in = NULL); + void setPauseStartedTimeIfNeeded(); DISALLOW_EVIL_CONSTRUCTORS(NuPlayerDriver); }; -- 2.11.0