Fix race condition for non-started fast tracks
This required re-implementing how fast tracks are considered active.
Now, they use the same logic as normal tracks, except underrun is ignored.
Other changes:
- add framesReady() to AudioBufferProvider interface
- rebased
- add track underrun counter state to fast mixer dump state
- move dumpsys header to Track::appendDumpHeader()
so it closer to where tracks are dumped
- display track state in dumpsys as a character code
- measure and display warmup time and cycles in dumpsys
- copy in the presentation complete code
- add ExtendedAudioBufferProvider for framesReady() which returns size_t
- simplify underrun tracking
- deferred reset track after stop()
- add comments
Change-Id: I7db8821bc565230ec76da1f9380fe3fb09735e5b
diff --git a/services/audioflinger/AudioFlinger.cpp b/services/audioflinger/AudioFlinger.cpp
index d123cbb..21137f2 100644
--- a/services/audioflinger/AudioFlinger.cpp
+++ b/services/audioflinger/AudioFlinger.cpp
@@ -1493,12 +1493,9 @@
mLastWriteTime(0), mNumWrites(0), mNumDelayedWrites(0), mInWrite(false),
mMixerStatus(MIXER_IDLE),
standbyDelay(AudioFlinger::mStandbyTimeInNsecs),
- mFastTrackAvailMask(((1 << FastMixerState::kMaxFastTracks) - 1) & ~1),
- mFastTrackNewMask(0)
+ // index 0 is reserved for normal mixer's submix
+ mFastTrackAvailMask(((1 << FastMixerState::kMaxFastTracks) - 1) & ~1)
{
-#if !LOG_NDEBUG
- memset(mFastTrackNewArray, 0, sizeof(mFastTrackNewArray));
-#endif
snprintf(mName, kNameLength, "AudioOut_%X", id);
readOutputParameters();
@@ -1550,8 +1547,7 @@
snprintf(buffer, SIZE, "Output thread %p tracks\n", this);
result.append(buffer);
- result.append(" Name Client Type Fmt Chn mask Session Frames S M F SRate L dB R dB "
- "Server User Main buf Aux Buf\n");
+ Track::appendDumpHeader(result);
for (size_t i = 0; i < mTracks.size(); ++i) {
sp<Track> track = mTracks[i];
if (track != 0) {
@@ -1562,8 +1558,7 @@
snprintf(buffer, SIZE, "Output thread %p active tracks\n", this);
result.append(buffer);
- result.append(" Name Client Type Fmt Chn mask Session Frames S M F SRate L dB R dB "
- "Server User Main buf Aux Buf\n");
+ Track::appendDumpHeader(result);
for (size_t i = 0; i < mActiveTracks.size(); ++i) {
sp<Track> track = mActiveTracks[i].promote();
if (track != 0) {
@@ -1866,6 +1861,7 @@
void AudioFlinger::PlaybackThread::destroyTrack_l(const sp<Track>& track)
{
track->mState = TrackBase::TERMINATED;
+ // active tracks are removed by threadLoop()
if (mActiveTracks.indexOf(track) < 0) {
removeTrack_l(track);
}
@@ -1875,6 +1871,16 @@
{
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(!(mFastTrackAvailMask & (1 << index)));
+ mFastTrackAvailMask |= 1 << index;
+ // redundant as track is about to be destroyed, for dumpsys only
+ track->mFastIndex = -1;
+ }
sp<EffectChain> chain = getEffectChain_l(track->sessionId());
if (chain != 0) {
chain->decTrackCnt();
@@ -2541,99 +2547,9 @@
return false;
}
-// FIXME This method needs a better name.
-// It pushes a new fast mixer state and returns (via tracksToRemove) a set of tracks to remove.
+// returns (via tracksToRemove) a set of tracks to remove.
void AudioFlinger::MixerThread::threadLoop_removeTracks(const Vector< sp<Track> >& tracksToRemove)
{
- // were any of the removed tracks also fast tracks?
- unsigned removedMask = 0;
- for (size_t i = 0; i < tracksToRemove.size(); ++i) {
- if (tracksToRemove[i]->isFastTrack()) {
- int j = tracksToRemove[i]->mFastIndex;
- ALOG_ASSERT(0 < j && j < FastMixerState::kMaxFastTracks);
- removedMask |= 1 << j;
- }
- }
- Track* newArray[FastMixerState::kMaxFastTracks];
- unsigned newMask;
- {
- AutoMutex _l(mLock);
- mFastTrackAvailMask |= removedMask;
- newMask = mFastTrackNewMask;
- if (newMask) {
- mFastTrackNewMask = 0;
- memcpy(newArray, mFastTrackNewArray, sizeof(mFastTrackNewArray));
-#if !LOG_NDEBUG
- memset(mFastTrackNewArray, 0, sizeof(mFastTrackNewArray));
-#endif
- }
- }
- unsigned changedMask = newMask | removedMask;
- // are there any newly added or removed fast tracks?
- if (changedMask) {
-
- // This assert would be incorrect because it's theoretically possible (though unlikely)
- // for a track to be created and then removed within the same normal mix cycle:
- // ALOG_ASSERT(!(newMask & removedMask));
- // The converse, of removing a track and then creating a new track at the identical slot
- // within the same normal mix cycle, is impossible because the slot isn't marked available.
-
- // prepare a new state to push
- FastMixerStateQueue *sq = mFastMixer->sq();
- FastMixerState *state = sq->begin();
- FastMixerStateQueue::block_t block = FastMixerStateQueue::BLOCK_UNTIL_PUSHED;
- while (changedMask) {
- int j = __builtin_ctz(changedMask);
- ALOG_ASSERT(0 < j && j < FastMixerState::kMaxFastTracks);
- changedMask &= ~(1 << j);
- FastTrack *fastTrack = &state->mFastTracks[j];
- // must first do new tracks, then removed tracks, in case same track in both
- if (newMask & (1 << j)) {
- ALOG_ASSERT(!(state->mTrackMask & (1 << j)));
- ALOG_ASSERT(fastTrack->mBufferProvider == NULL &&
- fastTrack->mVolumeProvider == NULL);
- Track *track = newArray[j];
- AudioBufferProvider *abp = track;
- VolumeProvider *vp = track;
- fastTrack->mBufferProvider = abp;
- fastTrack->mVolumeProvider = vp;
- fastTrack->mSampleRate = track->mSampleRate;
- fastTrack->mChannelMask = track->mChannelMask;
- state->mTrackMask |= 1 << j;
- }
- if (removedMask & (1 << j)) {
- ALOG_ASSERT(state->mTrackMask & (1 << j));
- ALOG_ASSERT(fastTrack->mBufferProvider != NULL &&
- fastTrack->mVolumeProvider != NULL);
- fastTrack->mBufferProvider = NULL;
- fastTrack->mVolumeProvider = NULL;
- fastTrack->mSampleRate = mSampleRate;
- fastTrack->mChannelMask = AUDIO_CHANNEL_OUT_STEREO;
- state->mTrackMask &= ~(1 << j);
- }
- fastTrack->mGeneration++;
- }
- state->mFastTracksGen++;
- // if the fast mixer was active, but now there are no fast tracks, then put it in cold idle
- if (kUseFastMixer == FastMixer_Dynamic &&
- state->mCommand == FastMixerState::MIX_WRITE && state->mTrackMask <= 1) {
- state->mCommand = FastMixerState::COLD_IDLE;
- state->mColdFutexAddr = &mFastMixerFutex;
- state->mColdGen++;
- mFastMixerFutex = 0;
- mNormalSink = mOutputSink;
- block = FastMixerStateQueue::BLOCK_UNTIL_ACKED;
- }
- sq->end();
- // If any fast tracks were removed, we must wait for acknowledgement
- // because we're about to decrement the last sp<> on those tracks.
- // Similarly if we put it into cold idle, need to wait for acknowledgement
- // so that it stops doing I/O.
- if (removedMask) {
- block = FastMixerStateQueue::BLOCK_UNTIL_ACKED;
- }
- sq->push(block);
- }
PlaybackThread::threadLoop_removeTracks(tracksToRemove);
}
@@ -2783,7 +2699,9 @@
size_t count = mActiveTracks.size();
size_t mixedTracks = 0;
size_t tracksWithEffect = 0;
+ // counts only _active_ fast tracks
size_t fastTracks = 0;
+ uint32_t resetMask = 0; // bit mask of fast tracks that need to be reset
float masterVolume = mMasterVolume;
bool masterMute = mMasterMute;
@@ -2800,6 +2718,16 @@
chain.clear();
}
+ // prepare a new state to push
+ FastMixerStateQueue *sq = NULL;
+ FastMixerState *state = NULL;
+ bool didModify = false;
+ FastMixerStateQueue::block_t block = FastMixerStateQueue::BLOCK_UNTIL_PUSHED;
+ if (mFastMixer != NULL) {
+ sq = mFastMixer->sq();
+ state = sq->begin();
+ }
+
for (size_t i=0 ; i<count ; i++) {
sp<Track> t = mActiveTracks[i].promote();
if (t == 0) continue;
@@ -2807,13 +2735,98 @@
// this const just means the local variable doesn't change
Track* const track = t.get();
+ // process fast tracks
if (track->isFastTrack()) {
- // cache the combined master volume and stream type volume for fast mixer;
- // this lacks any synchronization or barrier so VolumeProvider may read a stale value
- track->mCachedVolume = masterVolume * mStreamTypes[track->streamType()].volume;
- ++fastTracks;
- if (track->isTerminated()) {
- tracksToRemove->add(track);
+
+ // It's theoretically possible (though unlikely) for a fast track to be created
+ // and then removed within the same normal mix cycle. This is not a problem, as
+ // the track never becomes active so it's fast mixer slot is never touched.
+ // The converse, of removing an (active) track and then creating a new track
+ // at the identical fast mixer slot within the same normal mix cycle,
+ // is impossible because the slot isn't marked available until the end of each cycle.
+ int j = track->mFastIndex;
+ FastTrack *fastTrack = &state->mFastTracks[j];
+
+ // Determine whether the track is currently in underrun condition,
+ // and whether it had a recent underrun.
+ uint32_t underruns = mFastMixerDumpState.mTracks[j].mUnderruns;
+ uint32_t recentUnderruns = (underruns - (track->mObservedUnderruns & ~1)) >> 1;
+ // don't count underruns that occur while stopping or pausing
+ if (!(track->isStopped() || track->isPausing())) {
+ track->mUnderrunCount += recentUnderruns;
+ }
+ track->mObservedUnderruns = underruns;
+
+ // This is similar to the formula for normal tracks,
+ // with a few modifications for fast tracks.
+ bool isActive;
+ if (track->isStopped()) {
+ // track stays active after stop() until first underrun
+ isActive = recentUnderruns == 0;
+ } else if (track->isPaused() || track->isTerminated()) {
+ isActive = false;
+ } else if (track->isPausing()) {
+ // ramp down is not yet implemented
+ isActive = true;
+ track->setPaused();
+ } else if (track->isResuming()) {
+ // ramp up is not yet implemented
+ isActive = true;
+ track->mState = TrackBase::ACTIVE;
+ } else {
+ // no minimum frame count for fast tracks; continual underrun is allowed,
+ // but later could implement automatic pause after several consecutive underruns,
+ // or auto-mute yet still consider the track active and continue to service it
+ isActive = true;
+ }
+
+ if (isActive) {
+ // was it previously inactive?
+ if (!(state->mTrackMask & (1 << j))) {
+ ExtendedAudioBufferProvider *eabp = track;
+ VolumeProvider *vp = track;
+ fastTrack->mBufferProvider = eabp;
+ fastTrack->mVolumeProvider = vp;
+ fastTrack->mSampleRate = track->mSampleRate;
+ fastTrack->mChannelMask = track->mChannelMask;
+ fastTrack->mGeneration++;
+ state->mTrackMask |= 1 << j;
+ didModify = true;
+ // no acknowledgement required for newly active tracks
+ }
+ // cache the combined master volume and stream type volume for fast mixer; this
+ // lacks any synchronization or barrier so VolumeProvider may read a stale value
+ track->mCachedVolume = track->isMuted() ?
+ 0 : masterVolume * mStreamTypes[track->streamType()].volume;
+ ++fastTracks;
+ } else {
+ // was it previously active?
+ if (state->mTrackMask & (1 << j)) {
+ fastTrack->mBufferProvider = NULL;
+ fastTrack->mGeneration++;
+ state->mTrackMask &= ~(1 << j);
+ didModify = true;
+ // If any fast tracks were removed, we must wait for acknowledgement
+ // because we're about to decrement the last sp<> on those tracks.
+ block = FastMixerStateQueue::BLOCK_UNTIL_ACKED;
+ }
+ // Remainder of this block is copied from similar code for normal tracks
+ if (track->isStopped()) {
+ // Can't reset directly, as fast mixer is still polling this track
+ // track->reset();
+ // So instead mark this track as needing to be reset after push with ack
+ resetMask |= 1 << i;
+ }
+ // This would be incomplete if we auto-paused on underrun
+ size_t audioHALFrames =
+ (mOutput->stream->get_latency(mOutput->stream)*mSampleRate) / 1000;
+ size_t framesWritten =
+ mBytesWritten / audio_stream_frame_size(&mOutput->stream->common);
+ if (track->presentationComplete(framesWritten, audioHALFrames)) {
+ tracksToRemove->add(track);
+ }
+ // Avoids a misleading display in dumpsys
+ track->mObservedUnderruns &= ~1;
}
continue;
}
@@ -3009,7 +3022,8 @@
if (--(track->mRetryCount) <= 0) {
ALOGV("BUFFER TIMEOUT: remove(%d) from active list on thread %p", name, this);
tracksToRemove->add(track);
- // indicate to client process that the track was disabled because of underrun
+ // indicate to client process that the track was disabled because of underrun;
+ // it will then automatically call start() when data is available
android_atomic_or(CBLK_DISABLED_ON, &cblk->flags);
// If one track is not ready, mark the mixer also not ready if:
// - the mixer was ready during previous round OR
@@ -3027,7 +3041,41 @@
}
- // FIXME Here is where we would push the new FastMixer state if necessary
+ // Push the new FastMixer state if necessary
+ if (didModify) {
+ state->mFastTracksGen++;
+ // if the fast mixer was active, but now there are no fast tracks, then put it in cold idle
+ if (kUseFastMixer == FastMixer_Dynamic &&
+ state->mCommand == FastMixerState::MIX_WRITE && state->mTrackMask <= 1) {
+ state->mCommand = FastMixerState::COLD_IDLE;
+ state->mColdFutexAddr = &mFastMixerFutex;
+ state->mColdGen++;
+ mFastMixerFutex = 0;
+ if (kUseFastMixer == FastMixer_Dynamic) {
+ mNormalSink = mOutputSink;
+ }
+ // If we go into cold idle, need to wait for acknowledgement
+ // so that fast mixer stops doing I/O.
+ block = FastMixerStateQueue::BLOCK_UNTIL_ACKED;
+ }
+ sq->end();
+ }
+ if (sq != NULL) {
+ sq->end(didModify);
+ sq->push(block);
+ }
+
+ // Now perform the deferred reset on fast tracks that have stopped
+ while (resetMask != 0) {
+ size_t i = __builtin_ctz(resetMask);
+ ALOG_ASSERT(i < count);
+ resetMask &= ~(1 << i);
+ sp<Track> t = mActiveTracks[i].promote();
+ if (t == 0) continue;
+ Track* track = t.get();
+ ALOG_ASSERT(track->isFastTrack() && track->isStopped());
+ track->reset();
+ }
// remove all the tracks that need to be...
count = tracksToRemove->size();
@@ -3940,6 +3988,7 @@
{
buffer->raw = NULL;
mFrameCount = buffer->frameCount;
+ // FIXME See note at getNextBuffer()
(void) step(); // ignore return value of step()
buffer->frameCount = 0;
}
@@ -4021,6 +4070,8 @@
mPresentationCompleteFrames(0),
mFlags(flags),
mFastIndex(-1),
+ mObservedUnderruns(0),
+ mUnderrunCount(0),
mCachedVolume(1.0)
{
if (mCblk != NULL) {
@@ -4032,18 +4083,22 @@
ALOG_ASSERT(thread->mFastTrackAvailMask != 0);
int i = __builtin_ctz(thread->mFastTrackAvailMask);
ALOG_ASSERT(0 < i && i < 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
+ // being created. It would be better to allocate the index dynamically.
mFastIndex = i;
+ // Read the initial underruns because this field is never cleared by the fast mixer
+ mObservedUnderruns = thread->getFastTrackUnderruns(i) & ~1;
thread->mFastTrackAvailMask &= ~(1 << i);
- // Although we've allocated an index, we can't mutate or push a new fast track state
- // here, because that data structure can only be changed within the normal mixer
- // threadLoop(). So instead, make a note to mutate and push later.
- thread->mFastTrackNewArray[i] = this;
- thread->mFastTrackNewMask |= 1 << i;
}
// to avoid leaking a track name, do not allocate one unless there is an mCblk
mName = thread->getTrackName_l((audio_channel_mask_t)channelMask);
if (mName < 0) {
ALOGE("no more track names available");
+ // FIXME bug - if sufficient fast track indices, but insufficient normal mixer names,
+ // then we leak a fast track index. Should swap these two sections, or better yet
+ // only allocate a normal mixer name for normal tracks.
}
}
ALOGV("Track constructor name %d, calling pid %d", mName, IPCThreadState::self()->getCallingPid());
@@ -4091,22 +4146,59 @@
}
}
+/*static*/ void AudioFlinger::PlaybackThread::Track::appendDumpHeader(String8& result)
+{
+ result.append(" Name Client Type Fmt Chn mask Session Frames S M F SRate L dB R dB "
+ " Server User Main buf Aux Buf FastUnder\n");
+
+}
+
void AudioFlinger::PlaybackThread::Track::dump(char* buffer, size_t size)
{
uint32_t vlr = mCblk->getVolumeLR();
if (isFastTrack()) {
- strcpy(buffer, " fast");
+ sprintf(buffer, " F %2d", mFastIndex);
} else {
sprintf(buffer, " %4d", mName - AudioMixer::TRACK0);
}
- snprintf(&buffer[7], size-7, " %6d %4u %3u 0x%08x %7u %6u %1d %1d %1d %5u %5.2g %5.2g 0x%08x 0x%08x 0x%08x 0x%08x\n",
+ track_state state = mState;
+ char stateChar;
+ switch (state) {
+ case IDLE:
+ stateChar = 'I';
+ break;
+ case TERMINATED:
+ stateChar = 'T';
+ break;
+ case STOPPED:
+ stateChar = 'S';
+ break;
+ case RESUMING:
+ stateChar = 'R';
+ break;
+ case ACTIVE:
+ stateChar = 'A';
+ break;
+ case PAUSING:
+ stateChar = 'p';
+ break;
+ case PAUSED:
+ stateChar = 'P';
+ break;
+ default:
+ stateChar = '?';
+ break;
+ }
+ bool nowInUnderrun = mObservedUnderruns & 1;
+ snprintf(&buffer[7], size-7, " %6d %4u %3u 0x%08x %7u %6u %1c %1d %1d %5u %5.2g %5.2g "
+ "0x%08x 0x%08x 0x%08x 0x%08x %9u%c\n",
(mClient == 0) ? getpid_cached : mClient->pid(),
mStreamType,
mFormat,
mChannelMask,
mSessionId,
mFrameCount,
- mState,
+ stateChar,
mMute,
mFillingUpStatus,
mCblk->sampleRate,
@@ -4115,7 +4207,9 @@
mCblk->server,
mCblk->user,
(int)mMainBuffer,
- (int)mAuxBuffer);
+ (int)mAuxBuffer,
+ mUnderrunCount,
+ nowInUnderrun ? '*' : ' ');
}
// AudioBufferProvider interface
@@ -4128,11 +4222,19 @@
// Check if last stepServer failed, try to step now
if (mStepServerFailed) {
+ // FIXME When called by fast mixer, this takes a mutex with tryLock().
+ // Since the fast mixer is higher priority than client callback thread,
+ // it does not result in priority inversion for client.
+ // But a non-blocking solution would be preferable to avoid
+ // fast mixer being unable to tryLock(), and
+ // to avoid the extra context switches if the client wakes up,
+ // discovers the mutex is locked, then has to wait for fast mixer to unlock.
if (!step()) goto getNextBuffer_exit;
ALOGV("stepServer recovered");
mStepServerFailed = false;
}
+ // FIXME Same as above
framesReady = cblk->framesReady();
if (CC_LIKELY(framesReady)) {
@@ -4161,10 +4263,19 @@
return NOT_ENOUGH_DATA;
}
-uint32_t AudioFlinger::PlaybackThread::Track::framesReady() const {
+// Note that framesReady() takes a mutex on the control block using tryLock().
+// This could result in priority inversion if framesReady() is called by the normal mixer,
+// as the normal mixer thread runs at lower
+// priority than the client's callback thread: there is a short window within framesReady()
+// during which the normal mixer could be preempted, and the client callback would block.
+// Another problem can occur if framesReady() is called by the fast mixer:
+// the tryLock() could block for up to 1 ms, and a sequence of these could delay fast mixer.
+// FIXME Replace AudioTrackShared control block implementation by a non-blocking FIFO queue.
+size_t AudioFlinger::PlaybackThread::Track::framesReady() const {
return mCblk->framesReady();
}
+// Don't call for fast tracks; the framesReady() could result in priority inversion
bool AudioFlinger::PlaybackThread::Track::isReady() const {
if (mFillingUpStatus != FS_FILLING || isStopped() || isPausing()) return true;
@@ -4895,7 +5006,7 @@
buffer->frameCount = 0;
}
-uint32_t AudioFlinger::PlaybackThread::TimedTrack::framesReady() const {
+size_t AudioFlinger::PlaybackThread::TimedTrack::framesReady() const {
Mutex::Autolock _l(mTimedBufferQueueLock);
return mFramesPendingInQueue;
}
diff --git a/services/audioflinger/AudioFlinger.h b/services/audioflinger/AudioFlinger.h
index 9a0bbcd..0b73a30 100644
--- a/services/audioflinger/AudioFlinger.h
+++ b/services/audioflinger/AudioFlinger.h
@@ -46,6 +46,7 @@
#include <hardware/audio_policy.h>
#include "AudioBufferProvider.h"
+#include "ExtendedAudioBufferProvider.h"
#include "FastMixer.h"
#include "NBAIO.h"
@@ -355,7 +356,7 @@
void clearPowerManager();
// base for record and playback
- class TrackBase : public AudioBufferProvider, public RefBase {
+ class TrackBase : public ExtendedAudioBufferProvider, public RefBase {
public:
enum track_state {
@@ -396,6 +397,10 @@
virtual status_t getNextBuffer(AudioBufferProvider::Buffer* buffer, int64_t pts) = 0;
virtual void releaseBuffer(AudioBufferProvider::Buffer* buffer);
+ // ExtendedAudioBufferProvider interface is only needed for Track,
+ // but putting it in TrackBase avoids the complexity of virtual inheritance
+ virtual size_t framesReady() const { return SIZE_MAX; }
+
audio_format_t format() const {
return mFormat;
}
@@ -676,6 +681,7 @@
IAudioFlinger::track_flags_t flags);
virtual ~Track();
+ static void appendDumpHeader(String8& result);
void dump(char* buffer, size_t size);
virtual status_t start(AudioSystem::sync_event_t event = AudioSystem::SYNC_EVENT_NONE,
int triggerSession = 0);
@@ -699,11 +705,6 @@
int16_t *mainBuffer() const { return mMainBuffer; }
int auxEffectId() const { return mAuxEffectId; }
-#if 0
- bool isFastTrack() const
- { return (mFlags & IAudioFlinger::TRACK_FAST) != 0; }
-#endif
-
// implement FastMixerState::VolumeProvider interface
virtual uint32_t getVolumeLR();
@@ -720,7 +721,7 @@
virtual status_t getNextBuffer(AudioBufferProvider::Buffer* buffer, int64_t pts = kInvalidPTS);
// releaseBuffer() not overridden
- virtual uint32_t framesReady() const;
+ virtual size_t framesReady() const;
bool isMuted() const { return mMute; }
bool isPausing() const {
@@ -729,6 +730,9 @@
bool isPaused() const {
return mState == PAUSED;
}
+ bool isResuming() const {
+ return mState == RESUMING;
+ }
bool isReady() const;
void setPaused() { mState = PAUSED; }
void reset();
@@ -756,7 +760,9 @@
const sp<IMemory> mSharedBuffer;
bool mResetDone;
const audio_stream_type_t mStreamType;
- int mName; // track name on the normal mixer
+ int mName; // track name on the normal mixer,
+ // allocated statically at track creation time,
+ // and is even allocated (though unused) for fast tracks
int16_t *mMainBuffer;
int32_t *mAuxBuffer;
int mAuxEffectId;
@@ -765,7 +771,17 @@
// when this track will be fully rendered
private:
IAudioFlinger::track_flags_t mFlags;
- int mFastIndex; // index within FastMixerState::mFastTracks[] or -1
+
+ // The following fields are only for fast tracks, and should be in a subclass
+ int mFastIndex; // index within FastMixerState::mFastTracks[];
+ // either mFastIndex == -1
+ // or 0 < mFastIndex < FastMixerState::kMaxFast because
+ // index 0 is reserved for normal mixer's submix;
+ // index is allocated statically at track creation time
+ // but the slot is only used if track is active
+ uint32_t mObservedUnderruns; // Most recently observed value of
+ // mFastMixerDumpState.mTracks[mFastIndex].mUnderruns
+ uint32_t mUnderrunCount; // Counter of total number of underruns, never reset
volatile float mCachedVolume; // combined master volume and stream type volume;
// 'volatile' means accessed without lock or
// barrier, but is read/written atomically
@@ -800,7 +816,7 @@
// Mixer facing methods.
virtual bool isTimedTrack() const { return true; }
- virtual uint32_t framesReady() const;
+ virtual size_t framesReady() const;
// AudioBufferProvider interface
virtual status_t getNextBuffer(AudioBufferProvider::Buffer* buffer,
@@ -921,9 +937,9 @@
virtual void threadLoop_standby();
virtual void threadLoop_removeTracks(const Vector< sp<Track> >& tracksToRemove) { }
- // prepareTracks_l reads and writes mActiveTracks, and also returns the
- // pending set of tracks to remove via Vector 'tracksToRemove'. The caller is
- // responsible for clearing or destroying this Vector later on, when it
+ // prepareTracks_l reads and writes mActiveTracks, and returns
+ // the pending set of tracks to remove via Vector 'tracksToRemove'. The caller
+ // is responsible for clearing or destroying this Vector later on, when it
// is safe to do so. That will drop the final ref count and destroy the tracks.
virtual mixer_state prepareTracks_l(Vector< sp<Track> > *tracksToRemove) = 0;
@@ -993,7 +1009,7 @@
bool mMasterMute;
void setMasterMute_l(bool muted) { mMasterMute = muted; }
protected:
- SortedVector< wp<Track> > mActiveTracks;
+ SortedVector< wp<Track> > mActiveTracks; // FIXME check if this could be sp<>
// Allocate a track name for a given channel mask.
// Returns name >= 0 if successful, -1 on failure.
@@ -1080,12 +1096,11 @@
sp<NBAIO_Sink> mNormalSink;
public:
virtual bool hasFastMixer() const = 0;
+ virtual uint32_t getFastTrackUnderruns(size_t fastIndex) const { return 0; }
protected:
// accessed by both binder threads and within threadLoop(), lock on mutex needed
unsigned mFastTrackAvailMask; // bit i set if fast track [i] is available
- unsigned mFastTrackNewMask; // bit i set if fast track [i] just created
- Track* mFastTrackNewArray[FastMixerState::kMaxFastTracks];
};
@@ -1136,6 +1151,11 @@
public:
virtual bool hasFastMixer() const { return mFastMixer != NULL; }
+ virtual uint32_t getFastTrackUnderruns(size_t fastIndex) const {
+ ALOG_ASSERT(0 < fastIndex &&
+ fastIndex < FastMixerState::kMaxFastTracks);
+ return mFastMixerDumpState.mTracks[fastIndex].mUnderruns;
+ }
};
class DirectOutputThread : public PlaybackThread {
diff --git a/services/audioflinger/ExtendedAudioBufferProvider.h b/services/audioflinger/ExtendedAudioBufferProvider.h
new file mode 100644
index 0000000..88279b4
--- /dev/null
+++ b/services/audioflinger/ExtendedAudioBufferProvider.h
@@ -0,0 +1,31 @@
+/*
+ * Copyright (C) 2012 The Android Open Source Project
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+#ifndef ANDROID_EXTENDED_AUDIO_BUFFER_PROVIDER_H
+#define ANDROID_EXTENDED_AUDIO_BUFFER_PROVIDER_H
+
+#include "AudioBufferProvider.h"
+
+namespace android {
+
+class ExtendedAudioBufferProvider : public AudioBufferProvider {
+public:
+ virtual size_t framesReady() const = 0; // see description at AudioFlinger.h
+};
+
+} // namespace android
+
+#endif // ANDROID_EXTENDED_AUDIO_BUFFER_PROVIDER_H
diff --git a/services/audioflinger/FastMixer.cpp b/services/audioflinger/FastMixer.cpp
index 841b06a..bf264be 100644
--- a/services/audioflinger/FastMixer.cpp
+++ b/services/audioflinger/FastMixer.cpp
@@ -29,6 +29,7 @@
#define FAST_HOT_IDLE_NS 1000000L // 1 ms: time to sleep while hot idling
#define FAST_DEFAULT_NS 999999999L // ~1 sec: default time to sleep
+#define MAX_WARMUP_CYCLES 10 // maximum number of loop cycles to wait for warmup
namespace android {
@@ -58,8 +59,9 @@
unsigned sampleRate = 0;
int fastTracksGen = 0;
long periodNs = 0; // expected period; the time required to render one mix buffer
- long underrunNs = 0; // an underrun is likely if an actual cycle is greater than this value
- long overrunNs = 0; // an overrun is likely if an actual cycle if less than this value
+ long underrunNs = 0; // underrun likely when write cycle is greater than this value
+ long overrunNs = 0; // overrun likely when write cycle is less than this value
+ long warmupNs = 0; // warmup complete when write cycle is greater than to this value
FastMixerDumpState dummyDumpState, *dumpState = &dummyDumpState;
bool ignoreNextOverrun = true; // used to ignore initial overrun and first after an underrun
#ifdef FAST_MIXER_STATISTICS
@@ -67,6 +69,9 @@
static const unsigned kMaxSamples = 1000;
#endif
unsigned coldGen = 0; // last observed mColdGen
+ bool isWarm = false; // true means ready to mix, false means wait for warmup before mixing
+ struct timespec measuredWarmupTs = {0, 0}; // how long did it take for warmup to complete
+ uint32_t warmupCycles = 0; // counter of number of loop cycles required to warmup
for (;;) {
@@ -138,6 +143,12 @@
if (old <= 0) {
__futex_syscall4(coldFutexAddr, FUTEX_WAIT_PRIVATE, old - 1, NULL);
}
+ // This may be overly conservative; there could be times that the normal mixer
+ // requests such a brief cold idle that it doesn't require resetting this flag.
+ isWarm = false;
+ measuredWarmupTs.tv_sec = 0;
+ measuredWarmupTs.tv_nsec = 0;
+ warmupCycles = 0;
sleepNs = -1;
coldGen = current->mColdGen;
} else {
@@ -195,6 +206,7 @@
periodNs = (frameCount * 1000000000LL) / sampleRate; // 1.00
underrunNs = (frameCount * 1750000000LL) / sampleRate; // 1.75
overrunNs = (frameCount * 250000000LL) / sampleRate; // 0.25
+ warmupNs = (frameCount * 500000000LL) / sampleRate; // 0.50
} else {
periodNs = 0;
underrunNs = 0;
@@ -226,6 +238,7 @@
i = __builtin_ctz(removedTracks);
removedTracks &= ~(1 << i);
const FastTrack* fastTrack = ¤t->mFastTracks[i];
+ ALOG_ASSERT(fastTrack->mBufferProvider == NULL);
if (mixer != NULL) {
name = fastTrackNames[i];
ALOG_ASSERT(name >= 0);
@@ -234,6 +247,7 @@
#if !LOG_NDEBUG
fastTrackNames[i] = -1;
#endif
+ // don't reset track dump state, since other side is ignoring it
generations[i] = fastTrack->mGeneration;
}
@@ -313,13 +327,13 @@
}
// do work using current state here
- if ((command & FastMixerState::MIX) && (mixer != NULL)) {
+ if ((command & FastMixerState::MIX) && (mixer != NULL) && isWarm) {
ALOG_ASSERT(mixBuffer != NULL);
- // update volumes
- unsigned volumeTracks = current->mTrackMask;
- while (volumeTracks != 0) {
- i = __builtin_ctz(volumeTracks);
- volumeTracks &= ~(1 << i);
+ // for each track, update volume and check for underrun
+ unsigned currentTrackMask = current->mTrackMask;
+ while (currentTrackMask != 0) {
+ i = __builtin_ctz(currentTrackMask);
+ currentTrackMask &= ~(1 << i);
const FastTrack* fastTrack = ¤t->mFastTracks[i];
int name = fastTrackNames[i];
ALOG_ASSERT(name >= 0);
@@ -330,6 +344,25 @@
mixer->setParameter(name, AudioMixer::VOLUME, AudioMixer::VOLUME1,
(void *)(vlr >> 16));
}
+ // FIXME The current implementation of framesReady() for fast tracks
+ // takes a tryLock, which can block
+ // up to 1 ms. If enough active tracks all blocked in sequence, this would result
+ // in the overall fast mix cycle being delayed. Should use a non-blocking FIFO.
+ size_t framesReady = fastTrack->mBufferProvider->framesReady();
+ FastTrackDump *ftDump = &dumpState->mTracks[i];
+ uint32_t underruns = ftDump->mUnderruns;
+ if (framesReady < frameCount) {
+ ftDump->mUnderruns = (underruns + 2) | 1;
+ if (framesReady == 0) {
+ mixer->disable(name);
+ } else {
+ // allow mixing partial buffer
+ mixer->enable(name);
+ }
+ } else if (underruns & 1) {
+ ftDump->mUnderruns = underruns & ~1;
+ mixer->enable(name);
+ }
}
// process() is CPU-bound
mixer->process(AudioBufferProvider::kInvalidPTS);
@@ -337,6 +370,8 @@
} else if (mixBufferState == MIXED) {
mixBufferState = UNDEFINED;
}
+ bool attemptedWrite = false;
+ //bool didFullWrite = false; // dumpsys could display a count of partial writes
if ((command & FastMixerState::WRITE) && (outputSink != NULL) && (mixBuffer != NULL)) {
if (mixBufferState == UNDEFINED) {
memset(mixBuffer, 0, frameCount * 2 * sizeof(short));
@@ -348,10 +383,15 @@
ssize_t framesWritten = outputSink->write(mixBuffer, frameCount);
dumpState->mWriteSequence++;
if (framesWritten >= 0) {
+ ALOG_ASSERT(framesWritten <= frameCount);
dumpState->mFramesWritten += framesWritten;
+ //if ((size_t) framesWritten == frameCount) {
+ // didFullWrite = true;
+ //}
} else {
dumpState->mWriteErrors++;
}
+ attemptedWrite = true;
// FIXME count # of writes blocked excessively, CPU usage, etc. for dump
}
@@ -368,6 +408,27 @@
--sec;
nsec += 1000000000;
}
+ // To avoid an initial underrun on fast tracks after exiting standby,
+ // do not start pulling data from tracks and mixing until warmup is complete.
+ // Warmup is considered complete after the earlier of:
+ // first successful single write() that blocks for more than warmupNs
+ // MAX_WARMUP_CYCLES write() attempts.
+ // This is overly conservative, but to get better accuracy requires a new HAL API.
+ if (!isWarm && attemptedWrite) {
+ measuredWarmupTs.tv_sec += sec;
+ measuredWarmupTs.tv_nsec += nsec;
+ if (measuredWarmupTs.tv_nsec >= 1000000000) {
+ measuredWarmupTs.tv_sec++;
+ measuredWarmupTs.tv_nsec -= 1000000000;
+ }
+ ++warmupCycles;
+ if ((attemptedWrite && nsec > warmupNs) ||
+ (warmupCycles >= MAX_WARMUP_CYCLES)) {
+ isWarm = true;
+ dumpState->mMeasuredWarmupTs = measuredWarmupTs;
+ dumpState->mWarmupCycles = warmupCycles;
+ }
+ }
if (sec > 0 || nsec > underrunNs) {
// FIXME only log occasionally
ALOGV("underrun: time since last cycle %d.%03ld sec",
@@ -421,11 +482,13 @@
FastMixerDumpState::FastMixerDumpState() :
mCommand(FastMixerState::INITIAL), mWriteSequence(0), mFramesWritten(0),
mNumTracks(0), mWriteErrors(0), mUnderruns(0), mOverruns(0),
- mSampleRate(0), mFrameCount(0)
+ mSampleRate(0), mFrameCount(0), /* mMeasuredWarmupTs({0, 0}), */ mWarmupCycles(0)
#ifdef FAST_MIXER_STATISTICS
, mMean(0.0), mMinimum(0.0), mMaximum(0.0), mStddev(0.0)
#endif
{
+ mMeasuredWarmupTs.tv_sec = 0;
+ mMeasuredWarmupTs.tv_nsec = 0;
}
FastMixerDumpState::~FastMixerDumpState()
@@ -462,12 +525,14 @@
snprintf(string, COMMAND_MAX, "%d", mCommand);
break;
}
+ double mMeasuredWarmupMs = (mMeasuredWarmupTs.tv_sec * 1000.0) +
+ (mMeasuredWarmupTs.tv_nsec / 1000000.0);
fdprintf(fd, "FastMixer command=%s writeSequence=%u framesWritten=%u\n"
" numTracks=%u writeErrors=%u underruns=%u overruns=%u\n"
- " sampleRate=%u frameCount=%u\n",
+ " sampleRate=%u frameCount=%u measuredWarmup=%.3g ms, warmupCycles=%u\n",
string, mWriteSequence, mFramesWritten,
mNumTracks, mWriteErrors, mUnderruns, mOverruns,
- mSampleRate, mFrameCount);
+ mSampleRate, mFrameCount, mMeasuredWarmupMs, mWarmupCycles);
#ifdef FAST_MIXER_STATISTICS
fdprintf(fd, " cycle time in ms: mean=%.1f min=%.1f max=%.1f stddev=%.1f\n",
mMean*1e3, mMinimum*1e3, mMaximum*1e3, mStddev*1e3);
diff --git a/services/audioflinger/FastMixer.h b/services/audioflinger/FastMixer.h
index 8a8fcb8..a6dd310 100644
--- a/services/audioflinger/FastMixer.h
+++ b/services/audioflinger/FastMixer.h
@@ -42,8 +42,23 @@
}; // class FastMixer
+// Represents the dump state of a fast track
+struct FastTrackDump {
+ FastTrackDump() : mUnderruns(0) { }
+ /*virtual*/ ~FastTrackDump() { }
+ uint32_t mUnderruns; // Underrun status, represented as follows:
+ // bit 0 == 0 means not currently in underrun
+ // bit 0 == 1 means currently in underrun
+ // bits 1 to 31 == total number of underruns
+ // Not reset to zero for new tracks or if track generation changes.
+ // This representation is used to keep the information atomic.
+};
+
// The FastMixerDumpState keeps a cache of FastMixer statistics that can be logged by dumpsys.
-// Since used non-atomically, only POD types are permitted, and the contents can't be trusted.
+// Each individual native word-sized field is accessed atomically. But the
+// overall structure is non-atomic, that is there may be an inconsistency between fields.
+// No barriers or locks are used for either writing or reading.
+// Only POD types are permitted, and the contents shouldn't be trusted (i.e. do range checks).
// It has a different lifetime than the FastMixer, and so it can't be a member of FastMixer.
struct FastMixerDumpState {
FastMixerDumpState();
@@ -60,6 +75,9 @@
uint32_t mOverruns; // total number of overruns
uint32_t mSampleRate;
size_t mFrameCount;
+ struct timespec mMeasuredWarmupTs; // measured warmup time
+ uint32_t mWarmupCycles; // number of loop cycles required to warmup
+ FastTrackDump mTracks[FastMixerState::kMaxFastTracks];
#ifdef FAST_MIXER_STATISTICS
// cycle times in seconds
float mMean;
diff --git a/services/audioflinger/FastMixerState.h b/services/audioflinger/FastMixerState.h
index 83094c8..ce0cdb5 100644
--- a/services/audioflinger/FastMixerState.h
+++ b/services/audioflinger/FastMixerState.h
@@ -18,7 +18,7 @@
#define ANDROID_AUDIO_FAST_MIXER_STATE_H
#include <system/audio.h>
-#include "AudioBufferProvider.h"
+#include "ExtendedAudioBufferProvider.h"
#include "NBAIO.h"
namespace android {
@@ -40,7 +40,7 @@
FastTrack();
/*virtual*/ ~FastTrack();
- AudioBufferProvider* mBufferProvider; // must not be NULL
+ ExtendedAudioBufferProvider* mBufferProvider; // must be NULL if inactive, or non-NULL if active
VolumeProvider* mVolumeProvider; // optional; if NULL then full-scale
unsigned mSampleRate; // optional; if zero then use mixer sample rate
audio_channel_mask_t mChannelMask; // AUDIO_CHANNEL_OUT_MONO or AUDIO_CHANNEL_OUT_STEREO
@@ -57,7 +57,7 @@
// all pointer fields use raw pointers; objects are owned and ref-counted by the normal mixer
FastTrack mFastTracks[kMaxFastTracks];
int mFastTracksGen; // increment when any mFastTracks[i].mGeneration is incremented
- unsigned mTrackMask; // bit i is set if and only if mFastTracks[i] != NULL
+ unsigned mTrackMask; // bit i is set if and only if mFastTracks[i] is active
NBAIO_Sink* mOutputSink; // HAL output device, must already be negotiated
int mOutputSinkGen; // increment when mOutputSink is assigned
size_t mFrameCount; // number of frames per fast mix buffer
diff --git a/services/audioflinger/SourceAudioBufferProvider.cpp b/services/audioflinger/SourceAudioBufferProvider.cpp
index e9e8c16..e9d6d2c 100644
--- a/services/audioflinger/SourceAudioBufferProvider.cpp
+++ b/services/audioflinger/SourceAudioBufferProvider.cpp
@@ -95,4 +95,10 @@
mGetCount = 0;
}
+size_t SourceAudioBufferProvider::framesReady() const
+{
+ ssize_t avail = mSource->availableToRead();
+ return avail < 0 ? 0 : (size_t) avail;
+}
+
} // namespace android
diff --git a/services/audioflinger/SourceAudioBufferProvider.h b/services/audioflinger/SourceAudioBufferProvider.h
index 3219d78..85ccbb2 100644
--- a/services/audioflinger/SourceAudioBufferProvider.h
+++ b/services/audioflinger/SourceAudioBufferProvider.h
@@ -20,11 +20,11 @@
#define ANDROID_SOURCE_AUDIO_BUFFER_PROVIDER_H
#include "NBAIO.h"
-#include "AudioBufferProvider.h"
+#include "ExtendedAudioBufferProvider.h"
namespace android {
-class SourceAudioBufferProvider : public AudioBufferProvider {
+class SourceAudioBufferProvider : public ExtendedAudioBufferProvider {
public:
SourceAudioBufferProvider(const sp<NBAIO_Source>& source);
@@ -34,6 +34,9 @@
virtual status_t getNextBuffer(Buffer *buffer, int64_t pts);
virtual void releaseBuffer(Buffer *buffer);
+ // ExtendedAudioBufferProvider interface
+ virtual size_t framesReady() const;
+
private:
const sp<NBAIO_Source> mSource; // the wrapped source
/*const*/ size_t mFrameBitShift; // log2(frame size in bytes)