Improve timestamp jitter fix
Use last valid kernel timestamp on server side, not client side.
Bug: 28465713
Change-Id: I33590d6922980f288355f947bc56279245058429
diff --git a/include/media/AudioTimestamp.h b/include/media/AudioTimestamp.h
index 4f504a4..44d6c0b 100644
--- a/include/media/AudioTimestamp.h
+++ b/include/media/AudioTimestamp.h
@@ -37,9 +37,16 @@
struct ExtendedTimestamp {
enum Location {
LOCATION_INVALID = -1,
- LOCATION_CLIENT, // timestamp of last read frame from client-server track buffer
- LOCATION_SERVER, // timestamp of newest frame from client-server track buffer
+ // Locations in the audio playback / record pipeline.
+ LOCATION_CLIENT, // timestamp of last read frame from client-server track buffer.
+ LOCATION_SERVER, // timestamp of newest frame from client-server track buffer.
LOCATION_KERNEL, // timestamp of newest frame in the kernel (alsa) buffer.
+
+ // Historical data: info when the kernel timestamp was OK (prior to the newest frame).
+ // This may be useful when the newest frame kernel timestamp is unavailable.
+ // Available for playback timestamps.
+ LOCATION_SERVER_LASTKERNELOK, // timestamp of server the prior time kernel timestamp OK.
+ LOCATION_KERNEL_LASTKERNELOK, // timestamp of kernel the prior time kernel timestamp OK.
LOCATION_MAX // for sizing arrays only
};
@@ -101,7 +108,7 @@
// look for the closest-to-hw stage in the pipeline with a valid timestamp.
// We omit LOCATION_CLIENT as we prefer at least LOCATION_SERVER based accuracy
// when getting the best timestamp.
- for (int i = LOCATION_MAX - 1; i >= LOCATION_SERVER; --i) {
+ for (int i = LOCATION_KERNEL; i >= LOCATION_SERVER; --i) {
if (mTimeNs[i] > 0) {
*position = mPosition[i];
*time = mTimeNs[i] + mTimebaseOffset[timebase];
diff --git a/include/media/AudioTrack.h b/include/media/AudioTrack.h
index 46948e4..88c4e61 100644
--- a/include/media/AudioTrack.h
+++ b/include/media/AudioTrack.h
@@ -1047,7 +1047,6 @@
bool mRetrogradeMotionReported; // reduce log spam
AudioTimestamp mPreviousTimestamp; // used to detect retrograde motion
ExtendedTimestamp::Location mPreviousLocation; // location used for previous timestamp
- double mComputedLatencyMs; // latency between server and kernel
uint32_t mUnderrunCountOffset; // updated when restoring tracks
diff --git a/media/libmedia/AudioTrack.cpp b/media/libmedia/AudioTrack.cpp
index babe4ed..22a5acd 100644
--- a/media/libmedia/AudioTrack.cpp
+++ b/media/libmedia/AudioTrack.cpp
@@ -536,7 +536,6 @@
mTimestampStartupGlitchReported = false;
mRetrogradeMotionReported = false;
mPreviousLocation = ExtendedTimestamp::LOCATION_INVALID;
- mComputedLatencyMs = 0.;
mUnderrunCountOffset = 0;
mFramesWritten = 0;
mFramesWrittenServerOffset = 0;
@@ -570,7 +569,6 @@
mTimestampStartupGlitchReported = false;
mRetrogradeMotionReported = false;
mPreviousLocation = ExtendedTimestamp::LOCATION_INVALID;
- mComputedLatencyMs = 0.;
// read last server side position change via timestamp.
ExtendedTimestamp ets;
@@ -2375,25 +2373,22 @@
if (location == ExtendedTimestamp::LOCATION_SERVER) {
ALOGW_IF(mPreviousLocation == ExtendedTimestamp::LOCATION_KERNEL,
"getTimestamp() location moved from kernel to server");
- const double latencyMs = mComputedLatencyMs > 0.
- ? mComputedLatencyMs : mAfLatency;
const int64_t frames =
- int64_t(latencyMs * mSampleRate * mPlaybackRate.mSpeed / 1000);
- ALOGV("mComputedLatencyMs:%lf mAfLatency:%u frame adjustment:%lld",
- mComputedLatencyMs, mAfLatency, (long long)frames);
+ (ets.mTimeNs[ExtendedTimestamp::LOCATION_SERVER_LASTKERNELOK] < 0 ||
+ ets.mTimeNs[ExtendedTimestamp::LOCATION_KERNEL_LASTKERNELOK] < 0)
+ ?
+ int64_t((double)mAfLatency * mSampleRate * mPlaybackRate.mSpeed
+ / 1000)
+ :
+ (ets.mPosition[ExtendedTimestamp::LOCATION_SERVER_LASTKERNELOK]
+ - ets.mPosition[ExtendedTimestamp::LOCATION_KERNEL_LASTKERNELOK]);
+ ALOGV("frame adjustment:%lld timestamp:%s",
+ (long long)frames, ets.toString().c_str());
if (frames >= ets.mPosition[location]) {
timestamp.mPosition = 0;
} else {
timestamp.mPosition = (uint32_t)(ets.mPosition[location] - frames);
}
- } else if (location == ExtendedTimestamp::LOCATION_KERNEL) {
- const double bufferDiffMs =
- (double)(ets.mPosition[ExtendedTimestamp::LOCATION_SERVER]
- - ets.mPosition[ExtendedTimestamp::LOCATION_KERNEL])
- * 1000 / ((double)mSampleRate * mPlaybackRate.mSpeed);
- mComputedLatencyMs = bufferDiffMs > 0. ? bufferDiffMs : 0.;
- ALOGV("mComputedLatencyMs:%lf mAfLatency:%d",
- mComputedLatencyMs, mAfLatency);
}
mPreviousLocation = location;
} else {
diff --git a/services/audioflinger/Threads.cpp b/services/audioflinger/Threads.cpp
index 228426a..bee0447 100644
--- a/services/audioflinger/Threads.cpp
+++ b/services/audioflinger/Threads.cpp
@@ -2899,6 +2899,24 @@
// sink will block whie writing.
ExtendedTimestamp timestamp; // use private copy to fetch
(void) mNormalSink->getTimestamp(timestamp);
+
+ // We keep track of the last valid kernel position in case we are in underrun
+ // and the normal mixer period is the same as the fast mixer period, or there
+ // is some error from the HAL.
+ if (mTimestamp.mTimeNs[ExtendedTimestamp::LOCATION_KERNEL] >= 0) {
+ mTimestamp.mPosition[ExtendedTimestamp::LOCATION_KERNEL_LASTKERNELOK] =
+ mTimestamp.mPosition[ExtendedTimestamp::LOCATION_KERNEL];
+ mTimestamp.mTimeNs[ExtendedTimestamp::LOCATION_KERNEL_LASTKERNELOK] =
+ mTimestamp.mTimeNs[ExtendedTimestamp::LOCATION_KERNEL];
+
+ mTimestamp.mPosition[ExtendedTimestamp::LOCATION_SERVER_LASTKERNELOK] =
+ mTimestamp.mPosition[ExtendedTimestamp::LOCATION_SERVER];
+ mTimestamp.mTimeNs[ExtendedTimestamp::LOCATION_SERVER_LASTKERNELOK] =
+ mTimestamp.mTimeNs[ExtendedTimestamp::LOCATION_SERVER];
+ } else {
+ ALOGV("getTimestamp error - no valid kernel position");
+ }
+
// copy over kernel info
mTimestamp.mPosition[ExtendedTimestamp::LOCATION_KERNEL] =
timestamp.mPosition[ExtendedTimestamp::LOCATION_KERNEL];
diff --git a/services/audioflinger/Tracks.cpp b/services/audioflinger/Tracks.cpp
index 9d430aa..41cb030 100644
--- a/services/audioflinger/Tracks.cpp
+++ b/services/audioflinger/Tracks.cpp
@@ -1103,7 +1103,7 @@
if (local.mTimeNs[i] > 0) {
local.mPosition[i] = mFrameMap.findX(local.mPosition[i]);
// check drain state from the latest stage in the pipeline.
- if (!checked) {
+ if (!checked && i <= ExtendedTimestamp::LOCATION_KERNEL) {
mAudioTrackServerProxy->setDrained(
local.mPosition[i] >= mAudioTrackServerProxy->framesReleased());
checked = true;
