Improve AudioTrack offload timestamp startup glitch detector
New or existing glitch behavior for Nexus 5 offload audio:
we receive several 0 timestamps,
then we get a stale timestamp (very large),
then a few ms later we get a correct nonzero timestamp.
We attempt to hide the glitch because the retrograde timestamp
correction makes the glitch "sticky".
Bug: 21633313
Change-Id: I39153af718c151f9435e7d315651a811f72743da
diff --git a/include/media/AudioTrack.h b/include/media/AudioTrack.h
index ec16933..c0bc516 100644
--- a/include/media/AudioTrack.h
+++ b/include/media/AudioTrack.h
@@ -894,6 +894,7 @@
// only used for offloaded and direct tracks.
bool mPreviousTimestampValid;// true if mPreviousTimestamp is valid
+ bool mTimestampStartupGlitchReported; // reduce log spam
bool mRetrogradeMotionReported; // reduce log spam
AudioTimestamp mPreviousTimestamp; // used to detect retrograde motion
diff --git a/media/libmedia/AudioTrack.cpp b/media/libmedia/AudioTrack.cpp
index e2889b1..81ae6d7 100644
--- a/media/libmedia/AudioTrack.cpp
+++ b/media/libmedia/AudioTrack.cpp
@@ -492,6 +492,8 @@
mObservedSequence = mSequence;
mInUnderrun = false;
mPreviousTimestampValid = false;
+ mTimestampStartupGlitchReported = false;
+ mRetrogradeMotionReported = false;
return NO_ERROR;
}
@@ -519,6 +521,8 @@
// reset current position as seen by client to 0
mPosition = 0;
mPreviousTimestampValid = false;
+ mTimestampStartupGlitchReported = false;
+ mRetrogradeMotionReported = 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.
@@ -2218,7 +2222,12 @@
}
// Check whether a pending flush or stop has completed, as those commands may
- // be asynchronous or return near finish.
+ // be asynchronous or return near finish or exhibit glitchy behavior.
+ //
+ // Originally this showed up as the first timestamp being a continuation of
+ // the previous song under gapless playback.
+ // However, we sometimes see zero timestamps, then a glitch of
+ // the previous song's position, and then correct timestamps afterwards.
if (mStartUs != 0 && mSampleRate != 0) {
static const int kTimeJitterUs = 100000; // 100 ms
static const int k1SecUs = 1000000;
@@ -2236,16 +2245,29 @@
if (deltaPositionByUs > deltaTimeUs + kTimeJitterUs) {
// Verify that the counter can't count faster than the sample rate
- // since the start time. If greater, then that means we have failed
+ // since the start time. If greater, then that means we may have failed
// to completely flush or stop the previous playing track.
- ALOGW("incomplete flush or stop:"
+ ALOGW_IF(!mTimestampStartupGlitchReported,
+ "getTimestamp startup glitch detected"
" deltaTimeUs(%lld) deltaPositionUs(%lld) tsmPosition(%u)",
(long long)deltaTimeUs, (long long)deltaPositionByUs,
timestamp.mPosition);
+ mTimestampStartupGlitchReported = true;
+ if (previousTimestampValid
+ && mPreviousTimestamp.mPosition == 0 /* should be true if valid */) {
+ timestamp = mPreviousTimestamp;
+ mPreviousTimestampValid = true;
+ return NO_ERROR;
+ }
return WOULD_BLOCK;
}
+ if (deltaPositionByUs != 0) {
+ mStartUs = 0; // don't check again, we got valid nonzero position.
+ }
+ } else {
+ mStartUs = 0; // don't check again, start time expired.
}
- mStartUs = 0; // no need to check again, start timestamp has either expired or unneeded.
+ mTimestampStartupGlitchReported = false;
}
} else {
// Update the mapping between local consumed (mPosition) and server consumed (mServer)