Use modulo position variables in AudioTrack and AudioRecord
More type safety, plus correct treatment for sanitization.
Bug: 25569906
Bug: 25232421
Change-Id: Id852277b81a1792c5e67392cd74bc39cba7ed1ad
diff --git a/include/media/AudioRecord.h b/include/media/AudioRecord.h
index c4c7b0e..c47a4e7 100644
--- a/include/media/AudioRecord.h
+++ b/include/media/AudioRecord.h
@@ -20,6 +20,7 @@
#include <cutils/sched_policy.h>
#include <media/AudioSystem.h>
#include <media/IAudioRecord.h>
+#include <media/Modulo.h>
#include <utils/threads.h>
namespace android {
@@ -526,7 +527,7 @@
// caller must hold lock on mLock for all _l methods
- status_t openRecord_l(size_t epoch, const String16& opPackageName);
+ status_t openRecord_l(const Modulo<uint32_t> &epoch, const String16& opPackageName);
// FIXME enum is faster than strcmp() for parameter 'from'
status_t restoreRecord_l(const char *from);
@@ -556,9 +557,9 @@
bool mRetryOnPartialBuffer; // sleep and retry after partial obtainBuffer()
uint32_t mObservedSequence; // last observed value of mSequence
- uint32_t mMarkerPosition; // in wrapping (overflow) frame units
+ Modulo<uint32_t> mMarkerPosition; // in wrapping (overflow) frame units
bool mMarkerReached;
- uint32_t mNewPosition; // in frames
+ Modulo<uint32_t> mNewPosition; // in frames
uint32_t mUpdatePeriod; // in frames, zero means no EVENT_NEW_POS
status_t mStatus;
diff --git a/include/media/AudioTrack.h b/include/media/AudioTrack.h
index e02f1b7..fe4611c 100644
--- a/include/media/AudioTrack.h
+++ b/include/media/AudioTrack.h
@@ -22,6 +22,7 @@
#include <media/AudioTimestamp.h>
#include <media/IAudioTrack.h>
#include <media/AudioResamplerPublic.h>
+#include <media/Modulo.h>
#include <utils/threads.h>
namespace android {
@@ -798,7 +799,7 @@
{ return (mFlags & AUDIO_OUTPUT_FLAG_DIRECT) != 0; }
// increment mPosition by the delta of mServer, and return new value of mPosition
- uint32_t updateAndGetPosition_l();
+ Modulo<uint32_t> updateAndGetPosition_l();
// check sample rate and speed is compatible with AudioTrack
bool isSampleRateSpeedAllowed_l(uint32_t sampleRate, float speed) const;
@@ -885,19 +886,19 @@
bool mRetryOnPartialBuffer; // sleep and retry after partial obtainBuffer()
uint32_t mObservedSequence; // last observed value of mSequence
- uint32_t mMarkerPosition; // in wrapping (overflow) frame units
+ Modulo<uint32_t> mMarkerPosition; // in wrapping (overflow) frame units
bool mMarkerReached;
- uint32_t mNewPosition; // in frames
+ Modulo<uint32_t> mNewPosition; // in frames
uint32_t mUpdatePeriod; // in frames, zero means no EVENT_NEW_POS
- uint32_t mServer; // in frames, last known mProxy->getPosition()
+ Modulo<uint32_t> mServer; // in frames, last known mProxy->getPosition()
// which is count of frames consumed by server,
// reset by new IAudioTrack,
// whether it is reset by stop() is TBD
- uint32_t mPosition; // in frames, like mServer except continues
+ Modulo<uint32_t> mPosition; // in frames, like mServer except continues
// monotonically after new IAudioTrack,
// and could be easily widened to uint64_t
- uint32_t mReleased; // in frames, count of frames released to server
+ Modulo<uint32_t> mReleased; // count of frames released to server
// but not necessarily consumed by server,
// reset by stop() but continues monotonically
// after new IAudioTrack to restore mPosition,
diff --git a/include/media/Modulo.h b/include/media/Modulo.h
new file mode 100644
index 0000000..23280ac
--- /dev/null
+++ b/include/media/Modulo.h
@@ -0,0 +1,220 @@
+/*
+ * Copyright 2015 The Android Open Source Project
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+#ifndef ANDROID_MODULO_H
+#define ANDROID_MODULO_H
+
+namespace android {
+
+// Modulo class is used for intentionally wrapping variables such as
+// counters and timers.
+//
+// It may also be used for variables whose computation depends on the
+// associativity of addition or subtraction.
+//
+// Features:
+// 1) Modulo checks type sizes before performing operations to ensure
+// that the wrap points match. This is critical for safe modular arithmetic.
+// 2) Modulo returns Modulo types from arithmetic operations, thereby
+// avoiding unintentional use in a non-modular computation. A Modulo
+// type is converted to its base non-Modulo type through the value() function.
+// 3) Modulo separates out overflowable types from non-overflowable types.
+// A signed overflow is technically undefined in C and C++.
+// Modulo types do not participate in sanitization.
+// 4) Modulo comparisons are based on signed differences to account for wrap;
+// this is not the same as the direct comparison of values.
+// 5) Safe use of binary arithmetic operations relies on conversions of
+// signed operands to unsigned operands (which are modular arithmetic safe).
+// Conversions which are implementation-defined are assumed to use 2's complement
+// representation. (See A, B, C, D from the ISO/IEC FDIS 14882
+// Information technology — Programming languages — C++).
+//
+// A: ISO/IEC 14882:2011(E) p84 section 4.7 Integral conversions
+// (2) If the destination type is unsigned, the resulting value is the least unsigned
+// integer congruent to the source integer (modulo 2^n where n is the number of bits
+// used to represent the unsigned type). [ Note: In a two’s complement representation,
+// this conversion is conceptual and there is no change in the bit pattern (if there
+// is no truncation). — end note ]
+// (3) If the destination type is signed, the value is unchanged if it can be represented
+// in the destination type (and bit-field width); otherwise, the value is
+// implementation-defined.
+//
+// B: ISO/IEC 14882:2011(E) p88 section 5 Expressions
+// (9) Many binary operators that expect operands of arithmetic or enumeration type
+// cause conversions and yield result types in a similar way. The purpose is to
+// yield a common type, which is also the type of the result. This pattern is called
+// the usual arithmetic conversions, which are defined as follows:
+// [...]
+// Otherwise, if both operands have signed integer types or both have unsigned
+// integer types, the operand with the type of lesser integer conversion rank shall be
+// converted to the type of the operand with greater rank.
+// — Otherwise, if the operand that has unsigned integer type has rank greater than
+// or equal to the rank of the type of the other operand, the operand with signed
+// integer type shall be converted to the type of the operand with unsigned integer type.
+//
+// C: ISO/IEC 14882:2011(E) p86 section 4.13 Integer conversion rank
+// [...] The rank of long long int shall be greater than the rank of long int,
+// which shall be greater than the rank of int, which shall be greater than the
+// rank of short int, which shall be greater than the rank of signed char.
+// — The rank of any unsigned integer type shall equal the rank of the corresponding
+// signed integer type.
+//
+// D: ISO/IEC 14882:2011(E) p75 section 3.9.1 Fundamental types
+// [...] Unsigned integers, declared unsigned, shall obey the laws of arithmetic modulo
+// 2^n where n is the number of bits in the value representation of that particular
+// size of integer.
+//
+// Note:
+// Other libraries do exist for safe integer operations which can detect the
+// possibility of overflow (SafeInt from MS and safe-iop in android).
+// Signed safe computation is also possible from the art header safe_math.h.
+
+template <typename T> class Modulo {
+ T mValue;
+
+public:
+ typedef typename std::make_signed<T>::type signedT;
+ typedef typename std::make_unsigned<T>::type unsignedT;
+
+ Modulo() { } // intentionally uninitialized data
+ Modulo(const T &value) { mValue = value; }
+ const T & value() const { return mValue; } // not assignable
+ signedT signedValue() const { return mValue; }
+ unsignedT unsignedValue() const { return mValue; }
+ void getValue(T *value) const { *value = mValue; } // more type safe than value()
+
+ // modular operations valid only if size of T <= size of S.
+ template <typename S>
+ __attribute__((no_sanitize("integer")))
+ Modulo<T> operator +=(const Modulo<S> &other) {
+ static_assert(sizeof(T) <= sizeof(S), "argument size mismatch");
+ mValue += other.unsignedValue();
+ return *this;
+ }
+
+ template <typename S>
+ __attribute__((no_sanitize("integer")))
+ Modulo<T> operator -=(const Modulo<S> &other) {
+ static_assert(sizeof(T) <= sizeof(S), "argument size mismatch");
+ mValue -= other.unsignedValue();
+ return *this;
+ }
+
+ // modular operations resulting in a value valid only at the smaller of the two
+ // Modulo base type sizes, but we only allow equal sizes to avoid confusion.
+ template <typename S>
+ __attribute__((no_sanitize("integer")))
+ const Modulo<T> operator +(const Modulo<S> &other) const {
+ static_assert(sizeof(T) == sizeof(S), "argument size mismatch");
+ return Modulo<T>(mValue + other.unsignedValue());
+ }
+
+ template <typename S>
+ __attribute__((no_sanitize("integer")))
+ const Modulo<T> operator -(const Modulo<S> &other) const {
+ static_assert(sizeof(T) == sizeof(S), "argument size mismatch");
+ return Modulo<T>(mValue - other.unsignedValue());
+ }
+
+ // modular operations that should be checked only at the smaller of
+ // the two type sizes, but we only allow equal sizes to avoid confusion.
+ //
+ // Caution: These relational and comparison operations are not equivalent to
+ // the base type operations.
+ template <typename S>
+ __attribute__((no_sanitize("integer")))
+ bool operator >(const Modulo<S> &other) const {
+ static_assert(sizeof(T) == sizeof(S), "argument size mismatch");
+ return static_cast<signedT>(mValue - other.unsignedValue()) > 0;
+ }
+
+ template <typename S>
+ __attribute__((no_sanitize("integer")))
+ bool operator >=(const Modulo<S> &other) const {
+ static_assert(sizeof(T) == sizeof(S), "argument size mismatch");
+ return static_cast<signedT>(mValue - other.unsignedValue()) >= 0;
+ }
+
+ template <typename S>
+ __attribute__((no_sanitize("integer")))
+ bool operator ==(const Modulo<S> &other) const {
+ static_assert(sizeof(T) == sizeof(S), "argument size mismatch");
+ return static_cast<signedT>(mValue - other.unsignedValue()) == 0;
+ }
+
+ template <typename S>
+ __attribute__((no_sanitize("integer")))
+ bool operator <=(const Modulo<S> &other) const {
+ static_assert(sizeof(T) == sizeof(S), "argument size mismatch");
+ return static_cast<signedT>(mValue - other.unsignedValue()) <= 0;
+ }
+
+ template <typename S>
+ __attribute__((no_sanitize("integer")))
+ bool operator <(const Modulo<S> &other) const {
+ static_assert(sizeof(T) == sizeof(S), "argument size mismatch");
+ return static_cast<signedT>(mValue - other.unsignedValue()) < 0;
+ }
+
+
+ // modular operations with a non-Modulo type allowed with wrapping
+ // because there should be no confusion as to the meaning.
+ template <typename S>
+ __attribute__((no_sanitize("integer")))
+ Modulo<T> operator +=(const S &other) {
+ mValue += unsignedT(other);
+ return *this;
+ }
+
+ template <typename S>
+ __attribute__((no_sanitize("integer")))
+ Modulo<T> operator -=(const S &other) {
+ mValue -= unsignedT(other);
+ return *this;
+ }
+
+ // modular operations with a non-Modulo type allowed with wrapping,
+ // but we restrict this only when size of T is greater than or equal to
+ // the size of S to avoid confusion with the nature of overflow.
+ //
+ // Use of this follows left-associative style.
+ //
+ // Note: a Modulo type may be promoted by using "differences" off of
+ // a larger sized type, but we do not automate this.
+ template <typename S>
+ __attribute__((no_sanitize("integer")))
+ const Modulo<T> operator +(const S &other) const {
+ static_assert(sizeof(T) >= sizeof(S), "argument size mismatch");
+ return Modulo<T>(mValue + unsignedT(other));
+ }
+
+ template <typename S>
+ __attribute__((no_sanitize("integer")))
+ const Modulo<T> operator -(const S &other) const {
+ static_assert(sizeof(T) >= sizeof(S), "argument size mismatch");
+ return Modulo<T>(mValue - unsignedT(other));
+ }
+
+ // multiply is intentionally omitted, but it is a common operator in
+ // modular arithmetic.
+
+ // shift operations are intentionally omitted, but perhaps useful.
+ // For example, left-shifting a negative number is undefined in C++11.
+};
+
+} // namespace android
+
+#endif /* ANDROID_MODULO_H */
diff --git a/include/private/media/AudioTrackShared.h b/include/private/media/AudioTrackShared.h
index 1e5064f..1f3880f 100644
--- a/include/private/media/AudioTrackShared.h
+++ b/include/private/media/AudioTrackShared.h
@@ -26,6 +26,7 @@
#include <utils/RefBase.h>
#include <audio_utils/roundup.h>
#include <media/AudioResamplerPublic.h>
+#include <media/Modulo.h>
#include <media/SingleStateQueue.h>
namespace android {
@@ -280,11 +281,11 @@
// Call to force an obtainBuffer() to return quickly with -EINTR
void interrupt();
- size_t getPosition() {
+ Modulo<uint32_t> getPosition() {
return mEpoch + mCblk->mServer;
}
- void setEpoch(size_t epoch) {
+ void setEpoch(const Modulo<uint32_t> &epoch) {
mEpoch = epoch;
}
@@ -300,14 +301,14 @@
// in order for the client to be aligned at start of buffer
virtual size_t getMisalignment();
- size_t getEpoch() const {
+ Modulo<uint32_t> getEpoch() const {
return mEpoch;
}
size_t getFramesFilled();
private:
- size_t mEpoch;
+ Modulo<uint32_t> mEpoch;
};
// ----------------------------------------------------------------------------
diff --git a/media/libmedia/AudioRecord.cpp b/media/libmedia/AudioRecord.cpp
index 8ffcd4b..1c0d904 100644
--- a/media/libmedia/AudioRecord.cpp
+++ b/media/libmedia/AudioRecord.cpp
@@ -396,7 +396,7 @@
}
AutoMutex lock(mLock);
- *marker = mMarkerPosition;
+ mMarkerPosition.getValue(marker);
return NO_ERROR;
}
@@ -438,7 +438,7 @@
}
AutoMutex lock(mLock);
- *position = mProxy->getPosition();
+ mProxy->getPosition().getValue(position);
return NO_ERROR;
}
@@ -480,7 +480,7 @@
// -------------------------------------------------------------------------
// must be called with mLock held
-status_t AudioRecord::openRecord_l(size_t epoch, const String16& opPackageName)
+status_t AudioRecord::openRecord_l(const Modulo<uint32_t> &epoch, const String16& opPackageName)
{
const sp<IAudioFlinger>& audioFlinger = AudioSystem::get_audio_flinger();
if (audioFlinger == 0) {
@@ -890,23 +890,23 @@
}
// Get current position of server
- size_t position = mProxy->getPosition();
+ Modulo<uint32_t> position(mProxy->getPosition());
// Manage marker callback
bool markerReached = false;
- size_t markerPosition = mMarkerPosition;
+ Modulo<uint32_t> markerPosition(mMarkerPosition);
// FIXME fails for wraparound, need 64 bits
- if (!mMarkerReached && (markerPosition > 0) && (position >= markerPosition)) {
+ if (!mMarkerReached && markerPosition.value() > 0 && position >= markerPosition) {
mMarkerReached = markerReached = true;
}
// Determine the number of new position callback(s) that will be needed, while locked
size_t newPosCount = 0;
- size_t newPosition = mNewPosition;
+ Modulo<uint32_t> newPosition(mNewPosition);
uint32_t updatePeriod = mUpdatePeriod;
// FIXME fails for wraparound, need 64 bits
if (updatePeriod > 0 && position >= newPosition) {
- newPosCount = ((position - newPosition) / updatePeriod) + 1;
+ newPosCount = ((position - newPosition).value() / updatePeriod) + 1;
mNewPosition += updatePeriod * newPosCount;
}
@@ -933,7 +933,7 @@
mCbf(EVENT_MARKER, mUserData, &markerPosition);
}
while (newPosCount > 0) {
- size_t temp = newPosition;
+ size_t temp = newPosition.value(); // FIXME size_t != uint32_t
mCbf(EVENT_NEW_POS, mUserData, &temp);
newPosition += updatePeriod;
newPosCount--;
@@ -951,10 +951,10 @@
// Compute the estimated time until the next timed event (position, markers)
uint32_t minFrames = ~0;
if (!markerReached && position < markerPosition) {
- minFrames = markerPosition - position;
+ minFrames = (markerPosition - position).value();
}
if (updatePeriod > 0) {
- uint32_t remaining = newPosition - position;
+ uint32_t remaining = (newPosition - position).value();
if (remaining < minFrames) {
minFrames = remaining;
}
@@ -1087,7 +1087,7 @@
// if the new IAudioRecord is created, openRecord_l() will modify the
// following member variables: mAudioRecord, mCblkMemory, mCblk, mBufferMemory.
// It will also delete the strong references on previous IAudioRecord and IMemory
- size_t position = mProxy->getPosition();
+ Modulo<uint32_t> position(mProxy->getPosition());
mNewPosition = position + mUpdatePeriod;
status_t result = openRecord_l(position, mOpPackageName);
if (result == NO_ERROR) {
diff --git a/media/libmedia/AudioTrack.cpp b/media/libmedia/AudioTrack.cpp
index 82b6736..5e14940 100644
--- a/media/libmedia/AudioTrack.cpp
+++ b/media/libmedia/AudioTrack.cpp
@@ -920,7 +920,7 @@
}
AutoMutex lock(mLock);
- *marker = mMarkerPosition;
+ mMarkerPosition.getValue(marker);
return NO_ERROR;
}
@@ -1018,7 +1018,7 @@
// IAudioTrack::stop() isn't synchronous; we don't know when presentation completes
*position = (mState == STATE_STOPPED || mState == STATE_FLUSHED) ?
- 0 : updateAndGetPosition_l();
+ 0 : updateAndGetPosition_l().value();
}
return NO_ERROR;
}
@@ -1774,23 +1774,23 @@
}
// Get current position of server
- size_t position = updateAndGetPosition_l();
+ Modulo<uint32_t> position(updateAndGetPosition_l());
// Manage marker callback
bool markerReached = false;
- size_t markerPosition = mMarkerPosition;
- // FIXME fails for wraparound, need 64 bits
- if (!mMarkerReached && (markerPosition > 0) && (position >= markerPosition)) {
+ Modulo<uint32_t> markerPosition(mMarkerPosition);
+ // uses 32 bit wraparound for comparison with position.
+ if (!mMarkerReached && markerPosition.value() > 0 && position >= markerPosition) {
mMarkerReached = markerReached = true;
}
// Determine number of new position callback(s) that will be needed, while locked
size_t newPosCount = 0;
- size_t newPosition = mNewPosition;
- size_t updatePeriod = mUpdatePeriod;
+ Modulo<uint32_t> newPosition(mNewPosition);
+ uint32_t updatePeriod = mUpdatePeriod;
// FIXME fails for wraparound, need 64 bits
if (updatePeriod > 0 && position >= newPosition) {
- newPosCount = ((position - newPosition) / updatePeriod) + 1;
+ newPosCount = ((position - newPosition).value() / updatePeriod) + 1;
mNewPosition += updatePeriod * newPosCount;
}
@@ -1891,7 +1891,7 @@
mCbf(EVENT_MARKER, mUserData, &markerPosition);
}
while (newPosCount > 0) {
- size_t temp = newPosition;
+ size_t temp = newPosition.value(); // FIXME size_t != uint32_t
mCbf(EVENT_NEW_POS, mUserData, &temp);
newPosition += updatePeriod;
newPosCount--;
@@ -1915,14 +1915,14 @@
// FIXME only for non-compressed audio
uint32_t minFrames = ~0;
if (!markerReached && position < markerPosition) {
- minFrames = markerPosition - position;
+ minFrames = (markerPosition - position).value();
}
if (loopPeriod > 0 && loopPeriod < minFrames) {
// loopPeriod is already adjusted for actual position.
minFrames = loopPeriod;
}
if (updatePeriod > 0) {
- minFrames = min(minFrames, uint32_t(newPosition - position));
+ minFrames = min(minFrames, (newPosition - position).value());
}
// If > 0, poll periodically to recover from a stuck server. A good value is 2.
@@ -2157,11 +2157,11 @@
return result;
}
-uint32_t AudioTrack::updateAndGetPosition_l()
+Modulo<uint32_t> AudioTrack::updateAndGetPosition_l()
{
// This is the sole place to read server consumed frames
- uint32_t newServer = mProxy->getPosition();
- uint32_t delta = newServer > mServer ? newServer - mServer : 0;
+ Modulo<uint32_t> newServer(mProxy->getPosition());
+ const int32_t delta = (newServer - mServer).signedValue();
// TODO There is controversy about whether there can be "negative jitter" in server position.
// This should be investigated further, and if possible, it should be addressed.
// A more definite failure mode is infrequent polling by client.
@@ -2170,12 +2170,14 @@
// That should ensure delta never goes negative for infrequent polling
// unless the server has more than 2^31 frames in its buffer,
// in which case the use of uint32_t for these counters has bigger issues.
- if (newServer < mServer) {
- ALOGE("detected illegal retrograde motion by the server: mServer advanced by %d",
- (int32_t) newServer - mServer);
- }
+ ALOGE_IF(delta < 0,
+ "detected illegal retrograde motion by the server: mServer advanced by %d",
+ delta);
mServer = newServer;
- return mPosition += delta;
+ if (delta > 0) { // avoid retrograde
+ mPosition += delta;
+ }
+ return mPosition;
}
bool AudioTrack::isSampleRateSpeedAllowed_l(uint32_t sampleRate, float speed) const
@@ -2197,7 +2199,6 @@
return mAudioTrack->setParameters(keyValuePairs);
}
-__attribute__((no_sanitize("integer")))
status_t AudioTrack::getTimestamp(AudioTimestamp& timestamp)
{
AutoMutex lock(mLock);
@@ -2310,15 +2311,19 @@
// If this delta between these is greater than the client position, it means that
// actually presented is still stuck at the starting line (figuratively speaking),
// waiting for the first frame to go by. So we can't report a valid timestamp yet.
- if ((uint32_t) (mServer - timestamp.mPosition) > mPosition) {
+ // Note: We explicitly use non-Modulo comparison here - potential wrap issue when
+ // mPosition exceeds 32 bits.
+ // TODO Remove when timestamp is updated to contain pipeline status info.
+ const int32_t pipelineDepthInFrames = (mServer - timestamp.mPosition).signedValue();
+ if (pipelineDepthInFrames > 0 /* should be true, but we check anyways */
+ && (uint32_t)pipelineDepthInFrames > mPosition.value()) {
return INVALID_OPERATION;
}
// Convert timestamp position from server time base to client time base.
// TODO The following code should work OK now because timestamp.mPosition is 32-bit.
// But if we change it to 64-bit then this could fail.
- // Split this out instead of using += to prevent unsigned overflow
- // checks in the outer sum.
- timestamp.mPosition = timestamp.mPosition + static_cast<int32_t>(mPosition) - mServer;
+ // Use Modulo computation here.
+ timestamp.mPosition = (mPosition - mServer + timestamp.mPosition).value();
// Immediately after a call to getPosition_l(), mPosition and
// mServer both represent the same frame position. mPosition is
// in client's point of view, and mServer is in server's point of
@@ -2332,9 +2337,9 @@
// 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);
+#define TIME_TO_NANOS(time) ((int64_t)time.tv_sec * 1000000000 + time.tv_nsec)
+ const int64_t previousTimeNanos = TIME_TO_NANOS(mPreviousTimestamp.mTime);
+ const int64_t currentTimeNanos = TIME_TO_NANOS(timestamp.mTime);
#undef TIME_TO_NANOS
if (currentTimeNanos < previousTimeNanos) {
ALOGW("retrograde timestamp time");
@@ -2343,8 +2348,8 @@
// Looking at signed delta will work even when the timestamps
// are wrapping around.
- int32_t deltaPosition = static_cast<int32_t>(timestamp.mPosition
- - mPreviousTimestamp.mPosition);
+ int32_t deltaPosition = (Modulo<uint32_t>(timestamp.mPosition)
+ - mPreviousTimestamp.mPosition).signedValue();
// position can bobble slightly as an artifact; this hides the bobble
static const int32_t MINIMUM_POSITION_DELTA = 8;
if (deltaPosition < 0) {