From 4c053ea158b29fa2cdd4c6f39d3c8da4ee5a7a02 Mon Sep 17 00:00:00 2001 From: Glenn Kasten Date: Sun, 28 Sep 2014 14:41:07 -0700 Subject: Fix uncertainty of one normal mix buffer in AudioTrack::getTimestamp The per-thread timestamp latch was not synchronized with the per-track released frames. Now the value of each track's released frames is latched along with the timestamp. Bug: 17531839 Bug: 17669342 Change-Id: I9d50c8c6a5de55a3f4561ac40e20d497376c1257 --- services/audioflinger/Threads.cpp | 15 +++++++++++++++ services/audioflinger/Threads.h | 5 ++++- services/audioflinger/Tracks.cpp | 11 ++++++++++- 3 files changed, 29 insertions(+), 2 deletions(-) (limited to 'services') diff --git a/services/audioflinger/Threads.cpp b/services/audioflinger/Threads.cpp index 12e09ab..b1e9c07 100644 --- a/services/audioflinger/Threads.cpp +++ b/services/audioflinger/Threads.cpp @@ -2101,6 +2101,7 @@ ssize_t AudioFlinger::PlaybackThread::threadLoop_write() // If an NBAIO sink is present, use it to write the normal mixer's submix if (mNormalSink != 0) { + const size_t count = mBytesRemaining / mFrameSize; ATRACE_BEGIN("write"); @@ -2126,6 +2127,7 @@ ssize_t AudioFlinger::PlaybackThread::threadLoop_write() size_t totalFramesWritten = mNormalSink->framesWritten(); if (totalFramesWritten >= mLatchD.mTimestamp.mPosition) { mLatchD.mUnpresentedFrames = totalFramesWritten - mLatchD.mTimestamp.mPosition; + // mLatchD.mFramesReleased is set immediately before D is clocked into Q mLatchDValid = true; } } @@ -2418,6 +2420,18 @@ bool AudioFlinger::PlaybackThread::threadLoop() logString = NULL; } + // Gather the framesReleased counters for all active tracks, + // and latch them atomically with the timestamp. + // FIXME We're using raw pointers as indices. A unique track ID would be a better index. + mLatchD.mFramesReleased.clear(); + size_t size = mActiveTracks.size(); + for (size_t i = 0; i < size; i++) { + sp t = mActiveTracks[i].promote(); + if (t != 0) { + mLatchD.mFramesReleased.add(t.get(), + t->mAudioTrackServerProxy->framesReleased()); + } + } if (mLatchDValid) { mLatchQ = mLatchD; mLatchDValid = false; @@ -3093,6 +3107,7 @@ void AudioFlinger::MixerThread::threadLoop_mix() sleepTime = 0; standbyTime = systemTime() + standbyDelay; //TODO: delay standby when effects have a tail + } void AudioFlinger::MixerThread::threadLoop_sleepTime() diff --git a/services/audioflinger/Threads.h b/services/audioflinger/Threads.h index 7af5264..bb9aa18 100644 --- a/services/audioflinger/Threads.h +++ b/services/audioflinger/Threads.h @@ -819,8 +819,11 @@ private: struct { AudioTimestamp mTimestamp; uint32_t mUnpresentedFrames; + KeyedVector mFramesReleased; } mLatchD, mLatchQ; - bool mLatchDValid; // true means mLatchD is valid, and clock it into latch at next opportunity + bool mLatchDValid; // true means mLatchD is valid + // (except for mFramesReleased which is filled in later), + // and clock it into latch at next opportunity bool mLatchQValid; // true means mLatchQ is valid }; diff --git a/services/audioflinger/Tracks.cpp b/services/audioflinger/Tracks.cpp index 75190f3..b9308fa 100644 --- a/services/audioflinger/Tracks.cpp +++ b/services/audioflinger/Tracks.cpp @@ -898,7 +898,16 @@ status_t AudioFlinger::PlaybackThread::Track::getTimestamp(AudioTimestamp& times uint32_t unpresentedFrames = ((int64_t) playbackThread->mLatchQ.mUnpresentedFrames * mSampleRate) / playbackThread->mSampleRate; - uint32_t framesWritten = mAudioTrackServerProxy->framesReleased(); + // FIXME Since we're using a raw pointer as the key, it is theoretically possible + // for a brand new track to share the same address as a recently destroyed + // track, and thus for us to get the frames released of the wrong track. + // It is unlikely that we would be able to call getTimestamp() so quickly + // right after creating a new track. Nevertheless, the index here should + // be changed to something that is unique. Or use a completely different strategy. + ssize_t i = playbackThread->mLatchQ.mFramesReleased.indexOfKey(this); + uint32_t framesWritten = i >= 0 ? + playbackThread->mLatchQ.mFramesReleased[i] : + mAudioTrackServerProxy->framesReleased(); bool checkPreviousTimestamp = mPreviousValid && framesWritten >= mPreviousFramesWritten; if (framesWritten < unpresentedFrames) { mPreviousValid = false; -- cgit v1.1