Merge "Use the timestamp reported by the HAL for aaudio."
diff --git a/services/oboeservice/AAudioServiceEndpointMMAP.cpp b/services/oboeservice/AAudioServiceEndpointMMAP.cpp
index b9c1260..046b84b 100644
--- a/services/oboeservice/AAudioServiceEndpointMMAP.cpp
+++ b/services/oboeservice/AAudioServiceEndpointMMAP.cpp
@@ -242,6 +242,14 @@
     setFormat(config.format);
     setSampleRate(config.sample_rate);
 
+    // If the position is not updated while the timestamp is updated for more than a certain amount,
+    // the timestamp reported from the HAL may not be accurate. Here, a timestamp grace period is
+    // set as 5 burst size. We may want to update this value if there is any report from OEMs saying
+    // that is too short.
+    static constexpr int kTimestampGraceBurstCount = 5;
+    mTimestampGracePeriodMs = ((int64_t) kTimestampGraceBurstCount * mFramesPerBurst
+            * AAUDIO_MILLIS_PER_SECOND) / getSampleRate();
+
     ALOGD("%s() actual rate = %d, channels = %d channelMask = %#x, deviceId = %d, capacity = %d\n",
           __func__, getSampleRate(), getSamplesPerFrame(), getChannelMask(),
           deviceId, getBufferCapacity());
@@ -418,14 +426,79 @@
 aaudio_result_t AAudioServiceEndpointMMAP::getExternalPosition(uint64_t *positionFrames,
                                                                int64_t *timeNanos)
 {
-    if (!mExternalPositionSupported) {
-        return AAUDIO_ERROR_INVALID_STATE;
+    if (mHalExternalPositionStatus != AAUDIO_OK) {
+        return mHalExternalPositionStatus;
     }
-    status_t status = mMmapStream->getExternalPosition(positionFrames, timeNanos);
-    if (status == INVALID_OPERATION) {
-        // getExternalPosition is not supported. Set mExternalPositionSupported as false
+    uint64_t tempPositionFrames;
+    int64_t tempTimeNanos;
+    status_t status = mMmapStream->getExternalPosition(&tempPositionFrames, &tempTimeNanos);
+    if (status != OK) {
+        // getExternalPosition reports error. The HAL may not support the API. Cache the result
         // so that the call will not go to the HAL next time.
-        mExternalPositionSupported = false;
+        mHalExternalPositionStatus = AAudioConvert_androidToAAudioResult(status);
+        return mHalExternalPositionStatus;
     }
-    return AAudioConvert_androidToAAudioResult(status);
+
+    // If the HAL keeps reporting the same position or timestamp, the HAL may be having some issues
+    // to report correct external position. In that case, we will not trust the values reported from
+    // the HAL. Ideally, we may want to stop querying external position if the HAL cannot report
+    // correct position within a period. But it may not be a good idea to get system time too often.
+    // In that case, a maximum number of frozen external position is defined so that if the
+    // count of the same timestamp or position is reported by the HAL continuously, the values from
+    // the HAL will no longer be trusted.
+    static constexpr int kMaxFrozenCount = 20;
+    // If the HAL version is less than 7.0, the getPresentationPosition is an optional API.
+    // If the HAL version is 7.0 or later, the getPresentationPosition is a mandatory API.
+    // In that case, even the returned status is NO_ERROR, it doesn't indicate the returned
+    // position is a valid one. Do a simple validation, which is checking if the position is
+    // forward within half a second or not, here so that this function can return error if
+    // the validation fails. Note that we don't only apply this validation logic to HAL API
+    // less than 7.0. The reason is that there is a chance the HAL is not reporting the
+    // timestamp and position correctly.
+    if (mLastPositionFrames > tempPositionFrames) {
+        // If the position is going backwards, there must be something wrong with the HAL.
+        // In that case, we do not trust the values reported by the HAL.
+        ALOGW("%s position is going backwards, last position(%jd) current position(%jd)",
+              __func__, mLastPositionFrames, tempPositionFrames);
+        mHalExternalPositionStatus = AAUDIO_ERROR_INTERNAL;
+        return mHalExternalPositionStatus;
+    } else if (mLastPositionFrames == tempPositionFrames) {
+        if (tempTimeNanos - mTimestampNanosForLastPosition >
+                AAUDIO_NANOS_PER_MILLISECOND * mTimestampGracePeriodMs) {
+            ALOGW("%s, the reported position is not changed within %d msec. "
+                  "Set the external position as not supported", __func__, mTimestampGracePeriodMs);
+            mHalExternalPositionStatus = AAUDIO_ERROR_INTERNAL;
+            return mHalExternalPositionStatus;
+        }
+        mFrozenPositionCount++;
+    } else {
+        mFrozenPositionCount = 0;
+    }
+
+    if (mTimestampNanosForLastPosition > tempTimeNanos) {
+        // If the timestamp is going backwards, there must be something wrong with the HAL.
+        // In that case, we do not trust the values reported by the HAL.
+        ALOGW("%s timestamp is going backwards, last timestamp(%jd), current timestamp(%jd)",
+              __func__, mTimestampNanosForLastPosition, tempTimeNanos);
+        mHalExternalPositionStatus = AAUDIO_ERROR_INTERNAL;
+        return mHalExternalPositionStatus;
+    } else if (mTimestampNanosForLastPosition == tempTimeNanos) {
+        mFrozenTimestampCount++;
+    } else {
+        mFrozenTimestampCount = 0;
+    }
+
+    if (mFrozenTimestampCount + mFrozenPositionCount > kMaxFrozenCount) {
+        ALOGW("%s too many frozen external position from HAL.", __func__);
+        mHalExternalPositionStatus = AAUDIO_ERROR_INTERNAL;
+        return mHalExternalPositionStatus;
+    }
+
+    mLastPositionFrames = tempPositionFrames;
+    mTimestampNanosForLastPosition = tempTimeNanos;
+
+    // Only update the timestamp and position when they looks valid.
+    *positionFrames = tempPositionFrames;
+    *timeNanos = tempTimeNanos;
+    return mHalExternalPositionStatus;
 }
diff --git a/services/oboeservice/AAudioServiceEndpointMMAP.h b/services/oboeservice/AAudioServiceEndpointMMAP.h
index ddfac63..6314e5e 100644
--- a/services/oboeservice/AAudioServiceEndpointMMAP.h
+++ b/services/oboeservice/AAudioServiceEndpointMMAP.h
@@ -106,7 +106,12 @@
 
     int64_t                                   mHardwareTimeOffsetNanos = 0; // TODO get from HAL
 
-    bool                                      mExternalPositionSupported = true;
+    aaudio_result_t                           mHalExternalPositionStatus = AAUDIO_OK;
+    uint64_t                                  mLastPositionFrames = 0;
+    int64_t                                   mTimestampNanosForLastPosition = 0;
+    int32_t                                   mTimestampGracePeriodMs;
+    int32_t                                   mFrozenPositionCount = 0;
+    int32_t                                   mFrozenTimestampCount = 0;
 
 };
 
diff --git a/services/oboeservice/AAudioServiceStreamMMAP.cpp b/services/oboeservice/AAudioServiceStreamMMAP.cpp
index 05b7f7d..ffc16ac 100644
--- a/services/oboeservice/AAudioServiceStreamMMAP.cpp
+++ b/services/oboeservice/AAudioServiceStreamMMAP.cpp
@@ -167,7 +167,6 @@
 // If it fails, get timestamp that was written by getFreeRunningPosition()
 aaudio_result_t AAudioServiceStreamMMAP::getHardwareTimestamp_l(int64_t *positionFrames,
                                                                 int64_t *timeNanos) {
-
     sp<AAudioServiceEndpoint> endpoint = mServiceEndpointWeak.promote();
     if (endpoint == nullptr) {
         ALOGE("%s() has no endpoint", __func__);
@@ -176,17 +175,17 @@
     sp<AAudioServiceEndpointMMAP> serviceEndpointMMAP =
             static_cast<AAudioServiceEndpointMMAP *>(endpoint.get());
 
-    // Disable this code temporarily because the HAL is not returning
-    // a useful result.
-#if 0
     uint64_t position;
-    if (serviceEndpointMMAP->getExternalPosition(&position, timeNanos) == AAUDIO_OK) {
-        ALOGD("%s() getExternalPosition() says pos = %" PRIi64 ", time = %" PRIi64,
+    aaudio_result_t result = serviceEndpointMMAP->getExternalPosition(&position, timeNanos);
+    if (result == AAUDIO_OK) {
+        ALOGV("%s() getExternalPosition() says pos = %" PRIi64 ", time = %" PRIi64,
                 __func__, position, *timeNanos);
         *positionFrames = (int64_t) position;
         return AAUDIO_OK;
-    } else
-#endif
+    } else {
+        ALOGV("%s() getExternalPosition() returns error %d", __func__, result);
+    }
+
     if (mAtomicStreamTimestamp.isValid()) {
         Timestamp timestamp = mAtomicStreamTimestamp.read();
         *positionFrames = timestamp.getPosition();