diff options
author | Eric Laurent <elaurent@google.com> | 2012-05-08 18:57:51 -0700 |
---|---|---|
committer | Eric Laurent <elaurent@google.com> | 2012-05-14 17:20:12 -0700 |
commit | 2986460984580833161bdaabc7f17da1005a8961 (patch) | |
tree | bd8aa210c0faee61dc45e23479e29cfc85f313c2 | |
parent | dfa29ab13647f22b30b2de34d4830c9e815bf120 (diff) | |
download | frameworks_av-2986460984580833161bdaabc7f17da1005a8961.zip frameworks_av-2986460984580833161bdaabc7f17da1005a8961.tar.gz frameworks_av-2986460984580833161bdaabc7f17da1005a8961.tar.bz2 |
Fix issues with synchronous record start.
- Added a timeout in case the trigger event is never fired.
- Extend AudioRecord obtainBuffer() timeout in case of
synchronous start to avoid spurious warning.
- Make sure that the event is triggered if the track is
destroyed.
- Reject event if the triggering track is in an incompatible state.
Also fix a problem when restoring a static AudioTrack after
a mediaserver crash.
Bug 6449468.
Change-Id: Ib36e11111fb88f73caa31dcb0622792737d57a4b
-rw-r--r-- | include/media/AudioSystem.h | 4 | ||||
-rw-r--r-- | media/libmedia/AudioRecord.cpp | 5 | ||||
-rw-r--r-- | media/libmedia/AudioTrack.cpp | 3 | ||||
-rw-r--r-- | services/audioflinger/AudioFlinger.cpp | 92 | ||||
-rw-r--r-- | services/audioflinger/AudioFlinger.h | 11 |
5 files changed, 82 insertions, 33 deletions
diff --git a/include/media/AudioSystem.h b/include/media/AudioSystem.h index f73a317..e2662f2 100644 --- a/include/media/AudioSystem.h +++ b/include/media/AudioSystem.h @@ -173,6 +173,10 @@ public: SYNC_EVENT_CNT, }; + // Timeout for synchronous record start. Prevents from blocking the record thread forever + // if the trigger event is not fired. + static const uint32_t kSyncRecordStartTimeOutMs = 30000; + // // IAudioPolicyService interface (see AudioPolicyInterface for method descriptions) // diff --git a/media/libmedia/AudioRecord.cpp b/media/libmedia/AudioRecord.cpp index c21979b..0562f8e 100644 --- a/media/libmedia/AudioRecord.cpp +++ b/media/libmedia/AudioRecord.cpp @@ -334,7 +334,8 @@ status_t AudioRecord::start(AudioSystem::sync_event_t event, int triggerSession) cblk->lock.unlock(); if (ret == NO_ERROR) { mNewPosition = cblk->user + mUpdatePeriod; - cblk->bufferTimeoutMs = MAX_RUN_TIMEOUT_MS; + cblk->bufferTimeoutMs = (event == AudioSystem::SYNC_EVENT_NONE) ? MAX_RUN_TIMEOUT_MS : + AudioSystem::kSyncRecordStartTimeOutMs; cblk->waitTimeMs = 0; if (t != 0) { // thread unblocks in readyToRun() and returns NO_ERROR @@ -569,6 +570,8 @@ create_new_record: } cblk->waitTimeMs = 0; + // reset time out to running value after obtaining a buffer + cblk->bufferTimeoutMs = MAX_RUN_TIMEOUT_MS; if (framesReq > framesReady) { framesReq = framesReady; diff --git a/media/libmedia/AudioTrack.cpp b/media/libmedia/AudioTrack.cpp index 6189be5..5e6cd51 100644 --- a/media/libmedia/AudioTrack.cpp +++ b/media/libmedia/AudioTrack.cpp @@ -1365,6 +1365,9 @@ status_t AudioTrack::restoreTrack_l(audio_track_cblk_t*& cblk, bool fromStart) mCblk->stepUser(frames); } } + if (mSharedBuffer != 0) { + mCblk->stepUser(mCblk->frameCount); + } if (mActive) { result = mAudioTrack->start(); ALOGW_IF(result != NO_ERROR, "restoreTrack_l() start() failed status %d", result); diff --git a/services/audioflinger/AudioFlinger.cpp b/services/audioflinger/AudioFlinger.cpp index e1fae82..5acf29a 100644 --- a/services/audioflinger/AudioFlinger.cpp +++ b/services/audioflinger/AudioFlinger.cpp @@ -515,7 +515,11 @@ sp<IAudioTrack> AudioFlinger::createTrack( for (int i = 0; i < (int)mPendingSyncEvents.size(); i++) { if (mPendingSyncEvents[i]->triggerSession() == lSessionId) { if (thread->isValidSyncEvent(mPendingSyncEvents[i])) { - track->setSyncEvent(mPendingSyncEvents[i]); + if (lStatus == NO_ERROR) { + track->setSyncEvent(mPendingSyncEvents[i]); + } else { + mPendingSyncEvents[i]->cancel(); + } mPendingSyncEvents.removeAt(i); i--; } @@ -1847,6 +1851,7 @@ status_t AudioFlinger::PlaybackThread::addTrack_l(const sp<Track>& track) // effectively get the latency it requested. track->mFillingUpStatus = Track::FS_FILLING; track->mResetDone = false; + track->mPresentationCompleteFrames = 0; mActiveTracks.add(track); if (track->mainBuffer() != mMixBuffer) { sp<EffectChain> chain = getEffectChain_l(track->sessionId()); @@ -1877,13 +1882,14 @@ void AudioFlinger::PlaybackThread::destroyTrack_l(const sp<Track>& track) void AudioFlinger::PlaybackThread::removeTrack_l(const sp<Track>& track) { + track->triggerEvents(AudioSystem::SYNC_EVENT_PRESENTATION_COMPLETE); mTracks.remove(track); deleteTrackName_l(track->name()); // redundant as track is about to be destroyed, for dumpsys only track->mName = -1; if (track->isFastTrack()) { int index = track->mFastIndex; - ALOG_ASSERT(0 < index && index < FastMixerState::kMaxFastTracks); + ALOG_ASSERT(0 < index && index < (int)FastMixerState::kMaxFastTracks); ALOG_ASSERT(!(mFastTrackAvailMask & (1 << index))); mFastTrackAvailMask |= 1 << index; // redundant as track is about to be destroyed, for dumpsys only @@ -2850,7 +2856,8 @@ AudioFlinger::PlaybackThread::mixer_state AudioFlinger::MixerThread::prepareTrac case TrackBase::STOPPING_2: case TrackBase::PAUSED: case TrackBase::TERMINATED: - case TrackBase::STOPPED: // flush() while active + case TrackBase::STOPPED: + case TrackBase::FLUSHED: // flush() while active // Check for presentation complete if track is inactive // We have consumed all the buffers of this track. // This would be incomplete if we auto-paused on underrun @@ -3088,9 +3095,6 @@ AudioFlinger::PlaybackThread::mixer_state AudioFlinger::MixerThread::prepareTrac } } else { //ALOGV("track %d u=%08x, s=%08x [NOT READY] on thread %p", name, cblk->user, cblk->server, this); - if (track->isStopped()) { - track->reset(); - } if ((track->sharedBuffer() != 0) || track->isTerminated() || track->isStopped() || track->isPaused()) { // We have consumed all the buffers of this track. @@ -3102,6 +3106,9 @@ AudioFlinger::PlaybackThread::mixer_state AudioFlinger::MixerThread::prepareTrac size_t framesWritten = mBytesWritten / audio_stream_frame_size(&mOutput->stream->common); if (track->presentationComplete(framesWritten, audioHALFrames)) { + if (track->isStopped()) { + track->reset(); + } tracksToRemove->add(track); } } else { @@ -3546,9 +3553,6 @@ AudioFlinger::PlaybackThread::mixer_state AudioFlinger::DirectOutputThread::prep mixerStatus = MIXER_TRACKS_READY; } else { //ALOGV("track %d u=%08x, s=%08x [NOT READY]", track->name(), cblk->user, cblk->server); - if (track->isStopped()) { - track->reset(); - } if (track->isTerminated() || track->isStopped() || track->isPaused()) { // We have consumed all the buffers of this track. // Remove it from the list of active tracks. @@ -3558,6 +3562,9 @@ AudioFlinger::PlaybackThread::mixer_state AudioFlinger::DirectOutputThread::prep size_t framesWritten = mBytesWritten / audio_stream_frame_size(&mOutput->stream->common); if (track->presentationComplete(framesWritten, audioHALFrames)) { + if (track->isStopped()) { + track->reset(); + } trackToRemove = track; } } else { @@ -4170,7 +4177,7 @@ AudioFlinger::PlaybackThread::Track::Track( mCblk->flags |= CBLK_FAST; // atomic op not needed yet ALOG_ASSERT(thread->mFastTrackAvailMask != 0); int i = __builtin_ctz(thread->mFastTrackAvailMask); - ALOG_ASSERT(0 < i && i < FastMixerState::kMaxFastTracks); + ALOG_ASSERT(0 < i && i < (int)FastMixerState::kMaxFastTracks); // FIXME This is too eager. We allocate a fast track index before the // fast track becomes active. Since fast tracks are a scarce resource, // this means we are potentially denying other more important fast tracks from @@ -4278,6 +4285,9 @@ void AudioFlinger::PlaybackThread::Track::dump(char* buffer, size_t size) case PAUSED: stateChar = 'P'; break; + case FLUSHED: + stateChar = 'F'; + break; default: stateChar = '?'; break; @@ -4435,6 +4445,7 @@ status_t AudioFlinger::PlaybackThread::Track::start(AudioSystem::sync_event_t ev playbackThread->addTrack_l(this); } else { mState = state; + triggerEvents(AudioSystem::SYNC_EVENT_PRESENTATION_COMPLETE); } } else { status = BAD_VALUE; @@ -4511,11 +4522,11 @@ void AudioFlinger::PlaybackThread::Track::flush() return; } // No point remaining in PAUSED state after a flush => go to - // STOPPED state - mState = STOPPED; + // FLUSHED state + mState = FLUSHED; // do not reset the track if it is still in the process of being stopped or paused. // this will be done by prepareTracks_l() when the track is stopped. - // prepareTracks_l() will see mState == STOPPED, then + // prepareTracks_l() will see mState == FLUSHED, then // remove from active track list, reset(), and trigger presentation complete PlaybackThread *playbackThread = (PlaybackThread *)thread.get(); if (playbackThread->mActiveTracks.indexOf(this) < 0) { @@ -4536,7 +4547,9 @@ void AudioFlinger::PlaybackThread::Track::reset() android_atomic_or(CBLK_UNDERRUN_ON, &mCblk->flags); mFillingUpStatus = FS_FILLING; mResetDone = true; - mPresentationCompleteFrames = 0; + if (mState == FLUSHED) { + mState = IDLE; + } } } @@ -4577,7 +4590,6 @@ bool AudioFlinger::PlaybackThread::Track::presentationComplete(size_t framesWrit ALOGV("presentationComplete() session %d complete: framesWritten %d", mSessionId, framesWritten); triggerEvents(AudioSystem::SYNC_EVENT_PRESENTATION_COMPLETE); - mPresentationCompleteFrames = 0; return true; } return false; @@ -4621,6 +4633,20 @@ uint32_t AudioFlinger::PlaybackThread::Track::getVolumeLR() return vlr; } +status_t AudioFlinger::PlaybackThread::Track::setSyncEvent(const sp<SyncEvent>& event) +{ + if (mState == TERMINATED || mState == PAUSED || + ((framesReady() == 0) && ((mSharedBuffer != 0) || + (mState == STOPPED)))) { + ALOGW("Track::setSyncEvent() in invalid state %d on session %d %s mode, framesReady %d ", + mState, mSessionId, (mSharedBuffer != 0) ? "static" : "stream", framesReady()); + event->cancel(); + return INVALID_OPERATION; + } + TrackBase::setSyncEvent(event); + return NO_ERROR; +} + // timed audio tracks sp<AudioFlinger::PlaybackThread::TimedTrack> @@ -5945,8 +5971,18 @@ bool AudioFlinger::RecordThread::threadLoop() } else { if (mFramestoDrop > 0) { mFramestoDrop -= buffer.frameCount; - if (mFramestoDrop < 0) { - mFramestoDrop = 0; + if (mFramestoDrop <= 0) { + clearSyncStartEvent(); + } + } else { + mFramestoDrop += buffer.frameCount; + if (mFramestoDrop >= 0 || mSyncStartEvent == 0 || + mSyncStartEvent->isCancelled()) { + ALOGW("Synced record %s, session %d, trigger session %d", + (mFramestoDrop >= 0) ? "timed out" : "cancelled", + mActiveTrack->sessionId(), + (mSyncStartEvent != 0) ? mSyncStartEvent->triggerSession() : 0); + clearSyncStartEvent(); } } } @@ -6040,15 +6076,21 @@ status_t AudioFlinger::RecordThread::start(RecordThread::RecordTrack* recordTrac status_t status = NO_ERROR; if (event == AudioSystem::SYNC_EVENT_NONE) { - mSyncStartEvent.clear(); - mFramestoDrop = 0; + clearSyncStartEvent(); } else if (event != AudioSystem::SYNC_EVENT_SAME) { mSyncStartEvent = mAudioFlinger->createSyncEvent(event, triggerSession, recordTrack->sessionId(), syncStartEventCallback, this); - mFramestoDrop = -1; + // Sync event can be cancelled by the trigger session if the track is not in a + // compatible state in which case we start record immediately + if (mSyncStartEvent->isCancelled()) { + clearSyncStartEvent(); + } else { + // do not wait for the event for more than AudioSystem::kSyncRecordStartTimeOutMs + mFramestoDrop = - ((AudioSystem::kSyncRecordStartTimeOutMs * mReqSampleRate) / 1000); + } } { @@ -6108,6 +6150,7 @@ void AudioFlinger::RecordThread::clearSyncStartEvent() mSyncStartEvent->cancel(); } mSyncStartEvent.clear(); + mFramestoDrop = 0; } void AudioFlinger::RecordThread::syncStartEventCallback(const wp<SyncEvent>& event) @@ -6122,17 +6165,10 @@ void AudioFlinger::RecordThread::syncStartEventCallback(const wp<SyncEvent>& eve void AudioFlinger::RecordThread::handleSyncStartEvent(const sp<SyncEvent>& event) { - ALOGV("handleSyncStartEvent() mActiveTrack %p session %d event->listenerSession() %d", - mActiveTrack.get(), - mActiveTrack.get() ? mActiveTrack->sessionId() : 0, - event->listenerSession()); - - if (mActiveTrack != 0 && - event == mSyncStartEvent) { + if (event == mSyncStartEvent) { // TODO: use actual buffer filling status instead of 2 buffers when info is available // from audio HAL mFramestoDrop = mFrameCount * 2; - mSyncStartEvent.clear(); } } diff --git a/services/audioflinger/AudioFlinger.h b/services/audioflinger/AudioFlinger.h index 88056b8..d69cec4 100644 --- a/services/audioflinger/AudioFlinger.h +++ b/services/audioflinger/AudioFlinger.h @@ -231,6 +231,7 @@ public: virtual ~SyncEvent() {} void trigger() { Mutex::Autolock _l(mLock); if (mCallback) mCallback(this); } + bool isCancelled() { Mutex::Autolock _l(mLock); return (mCallback == NULL); } void cancel() {Mutex::Autolock _l(mLock); mCallback = NULL; } AudioSystem::sync_event_t type() const { return mType; } int triggerSession() const { return mTriggerSession; } @@ -362,8 +363,7 @@ private: enum track_state { IDLE, TERMINATED, - // These are order-sensitive; do not change order without reviewing the impact. - // In particular there are assumptions about > STOPPED. + FLUSHED, STOPPED, // next 2 states are currently used for fast tracks only STOPPING_1, // waiting for first underrun @@ -417,7 +417,7 @@ private: void* getBuffer(uint32_t offset, uint32_t frames) const; bool isStopped() const { - return mState == STOPPED; + return (mState == STOPPED || mState == FLUSHED); } // for fast tracks only @@ -721,6 +721,7 @@ private: // implement FastMixerState::VolumeProvider interface virtual uint32_t getVolumeLR(); + virtual status_t setSyncEvent(const sp<SyncEvent>& event); protected: // for numerous @@ -758,9 +759,9 @@ private: sp<IMemory> sharedBuffer() const { return mSharedBuffer; } bool presentationComplete(size_t framesWritten, size_t audioHalFrames); - void triggerEvents(AudioSystem::sync_event_t type); public: + void triggerEvents(AudioSystem::sync_event_t type); virtual bool isTimedTrack() const { return false; } bool isFastTrack() const { return (mFlags & IAudioFlinger::TRACK_FAST) != 0; } protected: @@ -1422,6 +1423,8 @@ private: // be dropped and therefore not read by the application. sp<SyncEvent> mSyncStartEvent; // number of captured frames to drop after the start sync event has been received. + // when < 0, maximum frames to drop before starting capture even if sync event is + // not received ssize_t mFramestoDrop; }; |