From 751a9c8cbc391a91604dd1052fb95d3e4d278c2f Mon Sep 17 00:00:00 2001 From: Glenn Kasten Date: Tue, 28 Jan 2014 11:03:28 -0800 Subject: Replace size_t in shared memory by uint32_t Eventually we may want to use uint64_t, but will need to confirm atomicity. Bug: 12381724 Change-Id: Ia2c591d262d22b47b6f7dab4b9d9faa14b86d865 --- include/private/media/AudioTrackShared.h | 14 +++++++++++--- include/private/media/StaticAudioTrackState.h | 10 +++++++--- media/libmedia/AudioTrackShared.cpp | 19 +++++++++++++------ 3 files changed, 31 insertions(+), 12 deletions(-) diff --git a/include/private/media/AudioTrackShared.h b/include/private/media/AudioTrackShared.h index 7fd9379..2d033e6 100644 --- a/include/private/media/AudioTrackShared.h +++ b/include/private/media/AudioTrackShared.h @@ -65,7 +65,9 @@ typedef SingleStateQueue StaticAudioTrackSingleStateQueue struct AudioTrackSharedStatic { StaticAudioTrackSingleStateQueue::Shared mSingleStateQueue; - size_t mBufferPosition; // updated asynchronously by server, + // This field should be a size_t, but since it is located in shared memory we + // force to 32-bit. The client and server may have different typedefs for size_t. + uint32_t mBufferPosition; // updated asynchronously by server, // "for entertainment purposes only" }; @@ -108,7 +110,9 @@ struct audio_track_cblk_t private: - size_t mMinimum; // server wakes up client if available >= mMinimum + // This field should be a size_t, but since it is located in shared memory we + // force to 32-bit. The client and server may have different typedefs for size_t. + uint32_t mMinimum; // server wakes up client if available >= mMinimum // Channel volumes are fixed point U4.12, so 0x1000 means 1.0. // Left channel is in [0:15], right channel is in [16:31]. @@ -245,7 +249,11 @@ public: } void setMinimum(size_t minimum) { - mCblk->mMinimum = minimum; + // This can only happen on a 64-bit client + if (minimum > UINT32_MAX) { + minimum = UINT32_MAX; + } + mCblk->mMinimum = (uint32_t) minimum; } // Return the number of frames that would need to be obtained and released diff --git a/include/private/media/StaticAudioTrackState.h b/include/private/media/StaticAudioTrackState.h index 46a5946..d483061 100644 --- a/include/private/media/StaticAudioTrackState.h +++ b/include/private/media/StaticAudioTrackState.h @@ -25,9 +25,13 @@ namespace android { // state is wrapped by a SingleStateQueue. struct StaticAudioTrackState { // do not define constructors, destructors, or virtual methods - size_t mLoopStart; - size_t mLoopEnd; - int mLoopCount; + + // These fields should both be size_t, but since they are located in shared memory we + // force to 32-bit. The client and server may have different typedefs for size_t. + uint32_t mLoopStart; + uint32_t mLoopEnd; + + int mLoopCount; }; } // namespace android diff --git a/media/libmedia/AudioTrackShared.cpp b/media/libmedia/AudioTrackShared.cpp index caa7900..e898109 100644 --- a/media/libmedia/AudioTrackShared.cpp +++ b/media/libmedia/AudioTrackShared.cpp @@ -475,9 +475,14 @@ void StaticAudioTrackClientProxy::flush() void StaticAudioTrackClientProxy::setLoop(size_t loopStart, size_t loopEnd, int loopCount) { + // This can only happen on a 64-bit client + if (loopStart > UINT32_MAX || loopEnd > UINT32_MAX) { + // FIXME Should return an error status + return; + } StaticAudioTrackState newState; - newState.mLoopStart = loopStart; - newState.mLoopEnd = loopEnd; + newState.mLoopStart = (uint32_t) loopStart; + newState.mLoopEnd = (uint32_t) loopEnd; newState.mLoopCount = loopCount; mBufferPosition = loopStart; (void) mMutator.push(newState); @@ -487,7 +492,7 @@ size_t StaticAudioTrackClientProxy::getBufferPosition() { size_t bufferPosition; if (mMutator.ack()) { - bufferPosition = mCblk->u.mStatic.mBufferPosition; + bufferPosition = (size_t) mCblk->u.mStatic.mBufferPosition; if (bufferPosition > mFrameCount) { bufferPosition = mFrameCount; } @@ -622,7 +627,7 @@ void ServerProxy::releaseBuffer(Buffer* buffer) if (half == 0) { half = 1; } - size_t minimum = cblk->mMinimum; + size_t minimum = (size_t) cblk->mMinimum; if (minimum == 0) { minimum = mIsOut ? half : 1; } else if (minimum > half) { @@ -760,7 +765,8 @@ ssize_t StaticAudioTrackServerProxy::pollPosition() mIsShutdown = true; return (ssize_t) NO_INIT; } - mCblk->u.mStatic.mBufferPosition = position; + // This may overflow, but client is not supposed to rely on it + mCblk->u.mStatic.mBufferPosition = (uint32_t) position; } return (ssize_t) position; } @@ -836,7 +842,8 @@ void StaticAudioTrackServerProxy::releaseBuffer(Buffer* buffer) mPosition = newPosition; cblk->mServer += stepCount; - cblk->u.mStatic.mBufferPosition = newPosition; + // This may overflow, but client is not supposed to rely on it + cblk->u.mStatic.mBufferPosition = (uint32_t) newPosition; if (setFlags != 0) { (void) android_atomic_or(setFlags, &cblk->mFlags); // this would be a good place to wake a futex -- cgit v1.1