Merge "AudioTrack: fix spurious retrograde messages" into mnc-dev

4 files changed, 49 insertions, 50 deletions

diff --git a/include/media/AudioTrack.h b/include/media/AudioTrack.h
index e7ee0ce..d361901 100644
--- a/include/media/AudioTrack.h
+++ b/include/media/AudioTrack.h
@@ -832,6 +832,9 @@ protected:
     int64_t                 mStartUs;               // the start time after flush or stop.
                                                     // only used for offloaded and direct tracks.
 
+    bool                    mPreviousTimestampValid;// true if mPreviousTimestamp is valid
+    AudioTimestamp          mPreviousTimestamp;     // used to detect retrograde motion
+
     audio_output_flags_t    mFlags;
         // const after set(), except for bits AUDIO_OUTPUT_FLAG_FAST and AUDIO_OUTPUT_FLAG_OFFLOAD.
         // mLock must be held to read or write those bits reliably.
diff --git a/media/libmedia/AudioTrack.cpp b/media/libmedia/AudioTrack.cpp
index 6ea09de..f0d9b96 100644
--- a/media/libmedia/AudioTrack.cpp
+++ b/media/libmedia/AudioTrack.cpp
@@ -470,6 +470,7 @@ status_t AudioTrack::set(
     mSequence = 1;
     mObservedSequence = mSequence;
     mInUnderrun = false;
+    mPreviousTimestampValid = false;
 
     return NO_ERROR;
 }
@@ -496,6 +497,8 @@ status_t AudioTrack::start()
     if (previousState == STATE_STOPPED || previousState == STATE_FLUSHED) {
         // reset current position as seen by client to 0
         mPosition = 0;
+        mPreviousTimestampValid = false;
+
         // For offloaded tracks, we don't know if the hardware counters are really zero here,
         // since the flush is asynchronous and stop may not fully drain.
         // We save the time when the track is started to later verify whether
@@ -995,6 +998,7 @@ status_t AudioTrack::reload()
     mNewPosition = mUpdatePeriod;
     (void) updateAndGetPosition_l();
     mPosition = 0;
+    mPreviousTimestampValid = false;
 #if 0
     // The documentation is not clear on the behavior of reload() and the restoration
     // of loop count. Historically we have not restored loop count, start, end,
@@ -2089,6 +2093,11 @@ status_t AudioTrack::setParameters(const String8& keyValuePairs)
 status_t AudioTrack::getTimestamp(AudioTimestamp& timestamp)
 {
     AutoMutex lock(mLock);
+
+    bool previousTimestampValid = mPreviousTimestampValid;
+    // Set false here to cover all the error return cases.
+    mPreviousTimestampValid = false;
+
     // FIXME not implemented for fast tracks; should use proxy and SSQ
     if (mFlags & AUDIO_OUTPUT_FLAG_FAST) {
         return INVALID_OPERATION;
@@ -2187,6 +2196,41 @@ status_t AudioTrack::getTimestamp(AudioTimestamp& timestamp)
         // IAudioTrack.  And timestamp.mPosition is initially in server's
         // point of view, so we need to apply the same fudge factor to it.
     }
+
+    // Prevent retrograde motion in timestamp.
+    // This is sometimes caused by erratic reports of the available space in the ALSA drivers.
+    if (status == NO_ERROR) {
+        if (previousTimestampValid) {
+#define TIME_TO_NANOS(time) ((uint64_t)time.tv_sec * 1000000000 + time.tv_nsec)
+            const uint64_t previousTimeNanos = TIME_TO_NANOS(mPreviousTimestamp.mTime);
+            const uint64_t currentTimeNanos = TIME_TO_NANOS(timestamp.mTime);
+#undef TIME_TO_NANOS
+            if (currentTimeNanos < previousTimeNanos) {
+                ALOGW("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;
+            ALOGW_IF(deltaPosition < 0,
+                    "retrograde timestamp position corrected, %d = %u - %u, (at %llu, %llu nanos)",
+                    deltaPosition,
+                    timestamp.mPosition,
+                    mPreviousTimestamp.mPosition,
+                    currentTimeNanos,
+                    previousTimeNanos);
+            if (deltaPosition < MINIMUM_POSITION_DELTA) {
+                timestamp = mPreviousTimestamp;  // Use last valid timestamp.
+            }
+        }
+        mPreviousTimestamp = timestamp;
+        mPreviousTimestampValid = true;
+    }
+
     return status;
 }
 
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;
 }