Simplify AudioTrack::restoreTrack_l()

Remove CBLK_RESTORING and CBLK_RESTORED from control block flags,
for AudioTrack only.  They are still used by AudioRecord.

This is part of a series to clean up the control block.

Change-Id: Iae4798f5b527c492bdaf789987ff3a1dadd0cb37

3 files changed, 72 insertions, 93 deletions

diff --git a/include/media/AudioTrack.h b/include/media/AudioTrack.h
index 5f235cb..6e88032 100644
--- a/include/media/AudioTrack.h
+++ b/include/media/AudioTrack.h
@@ -509,7 +509,7 @@ protected:
     float                   mSendLevel;
     uint32_t                mFrameCount;
 
-    audio_track_cblk_t*     mCblk;
+    audio_track_cblk_t*     mCblk;                  // re-load after mLock.unlock()
     audio_format_t          mFormat;
     audio_stream_type_t     mStreamType;
     uint8_t                 mChannelCount;
@@ -548,7 +548,6 @@ protected:
     // It is OK to lock only mCblk->lock.
     mutable Mutex           mLock;
 
-    status_t                mRestoreStatus;
     bool                    mIsTimed;
     int                     mPreviousPriority;          // before start()
     SchedPolicy             mPreviousSchedulingGroup;
diff --git a/include/private/media/AudioTrackShared.h b/include/private/media/AudioTrackShared.h
index 46788c4..90301cd 100644
--- a/include/private/media/AudioTrackShared.h
+++ b/include/private/media/AudioTrackShared.h
@@ -31,6 +31,7 @@ namespace android {
                                         // init time
 #define MAX_RUN_TIMEOUT_MS      1000
 #define WAIT_PERIOD_MS          10
+// AudioTrack no longer uses this, it is for AudioRecord only:
 #define RESTORE_TIMEOUT_MS      5000    // Maximum waiting time for a track to be restored
 
 #define CBLK_UNDERRUN   0x01 // set: underrun (out) or overrrun (in), clear: no underrun or overrun
@@ -38,6 +39,7 @@ namespace android {
                              // clear: track is ready when buffer full
 #define CBLK_INVALID    0x04 // track buffer invalidated by AudioFlinger, need to re-create
 #define CBLK_DISABLED   0x08 // track disabled by AudioFlinger due to underrun, need to re-start
+// AudioTrack no longer uses these, they are for AudioRecord only:
 #define CBLK_RESTORING  0x10 // track is being restored after invalidation by AudioFlinger
 #define CBLK_RESTORED   0x20 // track has been restored after invalidation by AudioFlinger
 
diff --git a/media/libmedia/AudioTrack.cpp b/media/libmedia/AudioTrack.cpp
index 324fd6d..7ce9879 100644
--- a/media/libmedia/AudioTrack.cpp
+++ b/media/libmedia/AudioTrack.cpp
@@ -297,7 +297,6 @@ status_t AudioTrack::set(
     mUpdatePeriod = 0;
     mFlushed = false;
     AudioSystem::acquireAudioSessionId(mSessionId);
-    mRestoreStatus = NO_ERROR;
     return NO_ERROR;
 }
 
@@ -956,12 +955,17 @@ status_t AudioTrack::obtainBuffer(Buffer* audioBuffer, int32_t waitCount)
             }
             if (!(cblk->flags & CBLK_INVALID)) {
                 mLock.unlock();
+                // this condition is in shared memory, so if IAudioTrack and control block
+                // are replaced due to mediaserver death or IAudioTrack invalidation then
+                // cv won't be signalled, but fortunately the timeout will limit the wait
                 result = cblk->cv.waitRelative(cblk->lock, milliseconds(waitTimeMs));
                 cblk->lock.unlock();
                 mLock.lock();
                 if (!mActive) {
                     return status_t(STOPPED);
                 }
+                // IAudioTrack may have been re-created while mLock was unlocked
+                cblk = mCblk;
                 cblk->lock.lock();
             }
 
@@ -1072,6 +1076,9 @@ ssize_t AudioTrack::write(const void* buffer, size_t userSize)
     sp<IMemory> iMem = mCblkMemory;
     mLock.unlock();
 
+    // since mLock is unlocked the IAudioTrack and shared memory may be re-created,
+    // so all cblk references might still refer to old shared memory, but that should be benign
+
     ssize_t written = 0;
     const int8_t *src = (const int8_t *)buffer;
     Buffer audioBuffer;
@@ -1192,6 +1199,9 @@ bool AudioTrack::processAudioBuffer(const sp<AudioTrackThread>& thread)
     bool active = mActive;
     mLock.unlock();
 
+    // since mLock is unlocked the IAudioTrack and shared memory may be re-created,
+    // so all cblk references might still refer to old shared memory, but that should be benign
+
     // Manage underrun callback
     if (active && (cblk->framesAvailableOut() == cblk->frameCount)) {
         ALOGV("Underrun user: %x, server: %x, flags %04x", cblk->user, cblk->server, cblk->flags);
@@ -1318,104 +1328,72 @@ status_t AudioTrack::restoreTrack_l(audio_track_cblk_t*& refCblk, bool fromStart
 
     audio_track_cblk_t* cblk = refCblk;
     audio_track_cblk_t* newCblk = cblk;
-    if (!(android_atomic_or(CBLK_RESTORING, &cblk->flags) & CBLK_RESTORING)) {
-        ALOGW("dead IAudioTrack, creating a new one from %s TID %d",
-            fromStart ? "start()" : "obtainBuffer()", gettid());
+    ALOGW("dead IAudioTrack, creating a new one from %s TID %d",
+        fromStart ? "start()" : "obtainBuffer()", gettid());
 
-        // signal old cblk condition so that other threads waiting for available buffers stop
-        // waiting now
-        cblk->cv.broadcast();
-        cblk->lock.unlock();
+    // signal old cblk condition so that other threads waiting for available buffers stop
+    // waiting now
+    cblk->cv.broadcast();
+    cblk->lock.unlock();
 
-        // refresh the audio configuration cache in this process to make sure we get new
-        // output parameters in getOutput_l() and createTrack_l()
-        AudioSystem::clearAudioConfigCache();
-
-        // if the new IAudioTrack is created, createTrack_l() will modify the
-        // following member variables: mAudioTrack, mCblkMemory and mCblk.
-        // It will also delete the strong references on previous IAudioTrack and IMemory
-        result = createTrack_l(mStreamType,
-                               cblk->sampleRate,
-                               mFormat,
-                               mChannelMask,
-                               mFrameCount,
-                               mFlags,
-                               mSharedBuffer,
-                               getOutput_l());
-
-        if (result == NO_ERROR) {
-            uint32_t user = cblk->user;
-            uint32_t server = cblk->server;
-            // restore write index and set other indexes to reflect empty buffer status
-            newCblk = mCblk;
-            newCblk->user = user;
-            newCblk->server = user;
-            newCblk->userBase = user;
-            newCblk->serverBase = user;
-            // restore loop: this is not guaranteed to succeed if new frame count is not
-            // compatible with loop length
-            setLoop_l(cblk->loopStart, cblk->loopEnd, cblk->loopCount);
-            if (!fromStart) {
-                newCblk->bufferTimeoutMs = MAX_RUN_TIMEOUT_MS;
-                // Make sure that a client relying on callback events indicating underrun or
-                // the actual amount of audio frames played (e.g SoundPool) receives them.
-                if (mSharedBuffer == 0) {
-                    uint32_t frames = 0;
-                    if (user > server) {
-                        frames = ((user - server) > newCblk->frameCount) ?
-                                newCblk->frameCount : (user - server);
-                        memset(newCblk->buffers, 0, frames * newCblk->frameSize);
-                    }
-                    // restart playback even if buffer is not completely filled.
-                    android_atomic_or(CBLK_FORCEREADY, &newCblk->flags);
-                    // stepUser() clears CBLK_UNDERRUN flag enabling underrun callbacks to
-                    // the client
-                    newCblk->stepUserOut(frames);
+    // refresh the audio configuration cache in this process to make sure we get new
+    // output parameters in getOutput_l() and createTrack_l()
+    AudioSystem::clearAudioConfigCache();
+
+    // if the new IAudioTrack is created, createTrack_l() will modify the
+    // following member variables: mAudioTrack, mCblkMemory and mCblk.
+    // It will also delete the strong references on previous IAudioTrack and IMemory
+    result = createTrack_l(mStreamType,
+                           cblk->sampleRate,
+                           mFormat,
+                           mChannelMask,
+                           mFrameCount,
+                           mFlags,
+                           mSharedBuffer,
+                           getOutput_l());
+
+    if (result == NO_ERROR) {
+        uint32_t user = cblk->user;
+        uint32_t server = cblk->server;
+        // restore write index and set other indexes to reflect empty buffer status
+        newCblk = mCblk;
+        newCblk->user = user;
+        newCblk->server = user;
+        newCblk->userBase = user;
+        newCblk->serverBase = user;
+        // restore loop: this is not guaranteed to succeed if new frame count is not
+        // compatible with loop length
+        setLoop_l(cblk->loopStart, cblk->loopEnd, cblk->loopCount);
+        if (!fromStart) {
+            newCblk->bufferTimeoutMs = MAX_RUN_TIMEOUT_MS;
+            // Make sure that a client relying on callback events indicating underrun or
+            // the actual amount of audio frames played (e.g SoundPool) receives them.
+            if (mSharedBuffer == 0) {
+                uint32_t frames = 0;
+                if (user > server) {
+                    frames = ((user - server) > newCblk->frameCount) ?
+                            newCblk->frameCount : (user - server);
+                    memset(newCblk->buffers, 0, frames * newCblk->frameSize);
                 }
-            }
-            if (mSharedBuffer != 0) {
-                newCblk->stepUserOut(newCblk->frameCount);
-            }
-            if (mActive) {
-                result = mAudioTrack->start();
-                ALOGW_IF(result != NO_ERROR, "restoreTrack_l() start() failed status %d", result);
-            }
-            if (fromStart && result == NO_ERROR) {
-                mNewPosition = newCblk->server + mUpdatePeriod;
+                // restart playback even if buffer is not completely filled.
+                android_atomic_or(CBLK_FORCEREADY, &newCblk->flags);
+                // stepUser() clears CBLK_UNDERRUN flag enabling underrun callbacks to
+                // the client
+                newCblk->stepUserOut(frames);
             }
         }
-        if (result != NO_ERROR) {
-            android_atomic_and(~CBLK_RESTORING, &cblk->flags);
-            ALOGW_IF(result != NO_ERROR, "restoreTrack_l() failed status %d", result);
+        if (mSharedBuffer != 0) {
+            newCblk->stepUserOut(newCblk->frameCount);
         }
-        mRestoreStatus = result;
-        // signal old cblk condition for other threads waiting for restore completion
-        android_atomic_or(CBLK_RESTORED, &cblk->flags);
-        cblk->cv.broadcast();
-    } else {
-        bool haveLogged = false;
-        for (;;) {
-            if (cblk->flags & CBLK_RESTORED) {
-                ALOGW("dead IAudioTrack restored");
-                result = mRestoreStatus;
-                cblk->lock.unlock();
-                break;
-            }
-            if (!haveLogged) {
-                ALOGW("dead IAudioTrack, waiting for a new one");
-                haveLogged = true;
-            }
-            mLock.unlock();
-            result = cblk->cv.waitRelative(cblk->lock, milliseconds(RESTORE_TIMEOUT_MS));
-            cblk->lock.unlock();
-            mLock.lock();
-            if (result != NO_ERROR) {
-                ALOGW("timed out");
-                break;
-            }
-            cblk->lock.lock();
+        if (mActive) {
+            result = mAudioTrack->start();
+            ALOGW_IF(result != NO_ERROR, "restoreTrack_l() start() failed status %d", result);
+        }
+        if (fromStart && result == NO_ERROR) {
+            mNewPosition = newCblk->server + mUpdatePeriod;
         }
     }
+    ALOGW_IF(result != NO_ERROR, "restoreTrack_l() failed status %d", result);
     ALOGV("restoreTrack_l() status %d mActive %d cblk %p, old cblk %p flags %08x old flags %08x",
         result, mActive, newCblk, cblk, newCblk->flags, cblk->flags);