AudioTrack: Refine the wait for volume ramp on pause

This avoids pop due to a flush immediately after pause
done by MediaPlayer.

Test: Play YT Music, adjust notification volume in settings and
      see if there is a pop at the end of playback.
Bug: 202376326
Merged-In: I8bfede65bd4e8daa721d25af1ade44382937df95
Change-Id: I8bfede65bd4e8daa721d25af1ade44382937df95
(cherry picked from commit d87a53a327c7e4af6a1ec508d7f70579fe99ae63)
(cherry picked from commit 118c2715ddbb42fc872348512d37ad0c4828021c)
Merged-In:I8bfede65bd4e8daa721d25af1ade44382937df95

2 files changed, 21 insertions, 3 deletions

diff --git a/include/private/media/AudioTrackShared.h b/include/private/media/AudioTrackShared.h
index bd6db55669..a1e1702990 100644
--- a/include/private/media/AudioTrackShared.h
+++ b/include/private/media/AudioTrackShared.h
@@ -55,6 +55,7 @@ namespace android {
 
 // for audio_track_cblk_t::mState, to match TrackBase.h
 static inline constexpr int CBLK_STATE_IDLE = 0;
+static inline constexpr int CBLK_STATE_ACTIVE = 6;
 static inline constexpr int CBLK_STATE_PAUSING = 7;
 
 /**
diff --git a/media/libaudioclient/AudioTrack.cpp b/media/libaudioclient/AudioTrack.cpp
index c2bea66309..54d186e3b3 100644
--- a/media/libaudioclient/AudioTrack.cpp
+++ b/media/libaudioclient/AudioTrack.cpp
@@ -972,8 +972,16 @@ bool AudioTrack::pauseAndWait(const std::chrono::milliseconds& timeout)
 {
     using namespace std::chrono_literals;
 
+    // We use atomic access here for state variables - these are used as hints
+    // to ensure we have ramped down audio.
+    const int priorState = mProxy->getState();
+    const uint32_t priorPosition = mProxy->getPosition().unsignedValue();
+
     pause();
 
+    // Only if we were previously active, do we wait to ramp down the audio.
+    if (priorState != CBLK_STATE_ACTIVE) return true;
+
     AutoMutex lock(mLock);
     // offload and direct tracks do not wait because pause volume ramp is handled by hardware.
     if (isOffloadedOrDirect_l()) return true;
@@ -981,16 +989,25 @@ bool AudioTrack::pauseAndWait(const std::chrono::milliseconds& timeout)
     // Wait for the track state to be anything besides pausing.
     // This ensures that the volume has ramped down.
     constexpr auto SLEEP_INTERVAL_MS = 10ms;
+    constexpr auto POSITION_TIMEOUT_MS = 40ms; // don't wait longer than this for position change.
     auto begin = std::chrono::steady_clock::now();
     while (true) {
-        // wait for state to change
+        // Wait for state and position to change.
+        // After pause() the server state should be PAUSING, but that may immediately
+        // convert to PAUSED by prepareTracks before data is read into the mixer.
+        // Hence we check that the state is not PAUSING and that the server position
+        // has advanced to be a more reliable estimate that the volume ramp has completed.
         const int state = mProxy->getState();
+        const uint32_t position = mProxy->getPosition().unsignedValue();
 
         mLock.unlock(); // only local variables accessed until lock.
         auto elapsed = std::chrono::duration_cast<std::chrono::milliseconds>(
                 std::chrono::steady_clock::now() - begin);
-        if (state != CBLK_STATE_PAUSING) {
-            ALOGV("%s: success state:%d after %lld ms", __func__, state, elapsed.count());
+        if (state != CBLK_STATE_PAUSING &&
+                (elapsed >= POSITION_TIMEOUT_MS || position != priorPosition)) {
+            ALOGV("%s: success state:%d, position:%u after %lld ms"
+                    " (prior state:%d  prior position:%u)",
+                    __func__, state, position, elapsed.count(), priorState, priorPosition);
             return true;
         }
         std::chrono::milliseconds remaining = timeout - elapsed;