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
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 @@
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 @@
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 @@
}
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 @@
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 @@
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 @@
// 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::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 @@
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 @@
}
} 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 @@
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 @@
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 @@
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 @@
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 @@
case PAUSED:
stateChar = 'P';
break;
+ case FLUSHED:
+ stateChar = 'F';
+ break;
default:
stateChar = '?';
break;
@@ -4435,6 +4445,7 @@
playbackThread->addTrack_l(this);
} else {
mState = state;
+ triggerEvents(AudioSystem::SYNC_EVENT_PRESENTATION_COMPLETE);
}
} else {
status = BAD_VALUE;
@@ -4511,11 +4522,11 @@
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 @@
android_atomic_or(CBLK_UNDERRUN_ON, &mCblk->flags);
mFillingUpStatus = FS_FILLING;
mResetDone = true;
- mPresentationCompleteFrames = 0;
+ if (mState == FLUSHED) {
+ mState = IDLE;
+ }
}
}
@@ -4577,7 +4590,6 @@
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 @@
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 @@
} 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 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 @@
mSyncStartEvent->cancel();
}
mSyncStartEvent.clear();
+ mFramestoDrop = 0;
}
void AudioFlinger::RecordThread::syncStartEventCallback(const wp<SyncEvent>& event)
@@ -6122,17 +6165,10 @@
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 @@
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 @@
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 @@
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 @@
// implement FastMixerState::VolumeProvider interface
virtual uint32_t getVolumeLR();
+ virtual status_t setSyncEvent(const sp<SyncEvent>& event);
protected:
// for numerous
@@ -758,9 +759,9 @@
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 @@
// 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;
};