From 1b42097f38e72574ed853a35f4e8a66e4739c421 Mon Sep 17 00:00:00 2001
From: Phil Burk <philburk@google.com>
Date: Wed, 22 Apr 2015 10:52:21 -0700
Subject: 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>
---
 services/audioflinger/Tracks.cpp | 47 ++--------------------------------------
 1 file changed, 2 insertions(+), 45 deletions(-)

(limited to 'services/audioflinger/Tracks.cpp')

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;
 }
 
-- 
cgit v1.1