AudioTrack: fix spurious retrograde messages

The retrograde motion was confused by some positions coming from
the DSP on offloaded tracks. So the retrograde check was moved up
into AudioTrack.cpp. This allows us to take advantage of the checks
for invalid positions based on timing.

Bug: 2047891
Change-Id: Ifcad2349201443a7f1711347c203297100449536
Signed-off-by: Phil Burk <philburk@google.com>

2 files changed, 2 insertions, 50 deletions

diff --git a/services/audioflinger/PlaybackTracks.h b/services/audioflinger/PlaybackTracks.h
index c51021b..7bc6f0c 100644
--- a/services/audioflinger/PlaybackTracks.h
+++ b/services/audioflinger/PlaybackTracks.h
@@ -156,11 +156,6 @@ private:
     bool                mResumeToStopping; // track was paused in stopping state.
     bool                mFlushHwPending; // track requests for thread flush
 
-    // for last call to getTimestamp
-    bool                mPreviousTimestampValid;
-    // This is either the first timestamp or one that has passed
-    // the check to prevent retrograde motion.
-    AudioTimestamp      mPreviousTimestamp;
 };  // end of Track
 
 class TimedTrack : public Track {
diff --git a/services/audioflinger/Tracks.cpp b/services/audioflinger/Tracks.cpp
index c6e9745..1b03060 100644
--- a/services/audioflinger/Tracks.cpp
+++ b/services/audioflinger/Tracks.cpp
@@ -404,8 +404,7 @@ AudioFlinger::PlaybackThread::Track::Track(
     mIsInvalid(false),
     mAudioTrackServerProxy(NULL),
     mResumeToStopping(false),
-    mFlushHwPending(false),
-    mPreviousTimestampValid(false)
+    mFlushHwPending(false)
 {
     // client == 0 implies sharedBuffer == 0
     ALOG_ASSERT(!(client == 0 && sharedBuffer != 0));
@@ -863,7 +862,6 @@ void AudioFlinger::PlaybackThread::Track::reset()
         if (mState == FLUSHED) {
             mState = IDLE;
         }
-        mPreviousTimestampValid = false;
     }
 }
 
@@ -885,12 +883,10 @@ status_t AudioFlinger::PlaybackThread::Track::getTimestamp(AudioTimestamp& times
 {
     // Client should implement this using SSQ; the unpresented frame count in latch is irrelevant
     if (isFastTrack()) {
-        // FIXME no lock held to set mPreviousTimestampValid = false
         return INVALID_OPERATION;
     }
     sp<ThreadBase> thread = mThread.promote();
     if (thread == 0) {
-        // FIXME no lock held to set mPreviousTimestampValid = false
         return INVALID_OPERATION;
     }
 
@@ -900,7 +896,6 @@ status_t AudioFlinger::PlaybackThread::Track::getTimestamp(AudioTimestamp& times
     status_t result = INVALID_OPERATION;
     if (!isOffloaded() && !isDirect()) {
         if (!playbackThread->mLatchQValid) {
-            mPreviousTimestampValid = false;
             return INVALID_OPERATION;
         }
         // FIXME Not accurate under dynamic changes of sample rate and speed.
@@ -919,10 +914,7 @@ status_t AudioFlinger::PlaybackThread::Track::getTimestamp(AudioTimestamp& times
         uint32_t framesWritten = i >= 0 ?
                 playbackThread->mLatchQ.mFramesReleased[i] :
                 mAudioTrackServerProxy->framesReleased();
-        if (framesWritten < unpresentedFrames) {
-            mPreviousTimestampValid = false;
-            // return invalid result
-        } else {
+        if (framesWritten >= unpresentedFrames) {
             timestamp.mPosition = framesWritten - unpresentedFrames;
             timestamp.mTime = playbackThread->mLatchQ.mTimestamp.mTime;
             result = NO_ERROR;
@@ -931,41 +923,6 @@ status_t AudioFlinger::PlaybackThread::Track::getTimestamp(AudioTimestamp& times
         result = playbackThread->getTimestamp_l(timestamp);
     }
 
-    // Prevent retrograde motion in timestamp.
-    if (result == NO_ERROR) {
-        if (mPreviousTimestampValid) {
-            if (timestamp.mTime.tv_sec < mPreviousTimestamp.mTime.tv_sec ||
-                    (timestamp.mTime.tv_sec == mPreviousTimestamp.mTime.tv_sec &&
-                    timestamp.mTime.tv_nsec < mPreviousTimestamp.mTime.tv_nsec)) {
-                ALOGW("WARNING - retrograde timestamp time");
-                // FIXME Consider blocking this from propagating upwards.
-            }
-
-            // Looking at signed delta will work even when the timestamps
-            // are wrapping around.
-            int32_t deltaPosition = static_cast<int32_t>(timestamp.mPosition
-                    - mPreviousTimestamp.mPosition);
-            // position can bobble slightly as an artifact; this hides the bobble
-            static const int32_t MINIMUM_POSITION_DELTA = 8;
-            if (deltaPosition < 0) {
-#define TIME_TO_NANOS(time) ((uint64_t)time.tv_sec * 1000000000 + time.tv_nsec)
-                ALOGW("WARNING - retrograde timestamp position corrected,"
-                        " %d = %u - %u, (at %llu, %llu nanos)",
-                        deltaPosition,
-                        timestamp.mPosition,
-                        mPreviousTimestamp.mPosition,
-                        TIME_TO_NANOS(timestamp.mTime),
-                        TIME_TO_NANOS(mPreviousTimestamp.mTime));
-#undef TIME_TO_NANOS
-            }
-            if (deltaPosition < MINIMUM_POSITION_DELTA) {
-                // Current timestamp is bad. Use last valid timestamp.
-                timestamp = mPreviousTimestamp;
-            }
-        }
-        mPreviousTimestamp = timestamp;
-        mPreviousTimestampValid = true;
-    }
     return result;
 }