From 7db7df0e8d9d7cee8ba374468cdbfa0108e3337c Mon Sep 17 00:00:00 2001 From: Glenn Kasten Date: Tue, 25 Jun 2013 16:13:23 -0700 Subject: AudioTrackShared cleanup Maintain unreleased frame count on client side also (was already there on server side). Assertion failure instead of BAD_VALUE status for incorrect usage of APIs. Clean up error handling code. Change-Id: I23ca2f6f8a7c18645309ee5d64fbc844429bcba8 --- media/libmedia/AudioTrackShared.cpp | 63 +++++++++++++++++++------------------ 1 file changed, 32 insertions(+), 31 deletions(-) (limited to 'media/libmedia/AudioTrackShared.cpp') diff --git a/media/libmedia/AudioTrackShared.cpp b/media/libmedia/AudioTrackShared.cpp index 5f8f292..554802d 100644 --- a/media/libmedia/AudioTrackShared.cpp +++ b/media/libmedia/AudioTrackShared.cpp @@ -38,7 +38,7 @@ Proxy::Proxy(audio_track_cblk_t* cblk, void *buffers, size_t frameCount, size_t bool isOut, bool clientInServer) : mCblk(cblk), mBuffers(buffers), mFrameCount(frameCount), mFrameSize(frameSize), mFrameCountP2(roundup(frameCount)), mIsOut(isOut), mClientInServer(clientInServer), - mIsShutdown(false) + mIsShutdown(false), mUnreleased(0) { } @@ -64,10 +64,7 @@ const struct timespec ClientProxy::kNonBlocking = {0 /*tv_sec*/, 0 /*tv_nsec*/}; status_t ClientProxy::obtainBuffer(Buffer* buffer, const struct timespec *requested, struct timespec *elapsed) { - if (buffer == NULL || buffer->mFrameCount == 0) { - ALOGE("%s BAD_VALUE", __func__); - return BAD_VALUE; - } + LOG_ALWAYS_FATAL_IF(buffer == NULL || buffer->mFrameCount == 0); struct timespec total; // total elapsed time spent waiting total.tv_sec = 0; total.tv_nsec = 0; @@ -164,7 +161,7 @@ status_t ClientProxy::obtainBuffer(Buffer* buffer, const struct timespec *reques buffer->mRaw = part1 > 0 ? &((char *) mBuffers)[(mIsOut ? rear : front) * mFrameSize] : NULL; buffer->mNonContig = avail - part1; - // mUnreleased = part1; + mUnreleased = part1; status = NO_ERROR; break; } @@ -238,6 +235,7 @@ status_t ClientProxy::obtainBuffer(Buffer* buffer, const struct timespec *reques case -EWOULDBLOCK: // benign race condition with server case -EINTR: // wait was interrupted by signal or other spurious wakeup case -ETIMEDOUT: // time-out expired + // FIXME these error/non-0 status are being dropped break; default: ALOGE("%s unexpected error %d", __func__, ret); @@ -252,6 +250,7 @@ end: buffer->mFrameCount = 0; buffer->mRaw = NULL; buffer->mNonContig = 0; + mUnreleased = 0; } if (elapsed != NULL) { *elapsed = total; @@ -268,14 +267,17 @@ end: void ClientProxy::releaseBuffer(Buffer* buffer) { + LOG_ALWAYS_FATAL_IF(buffer == NULL); size_t stepCount = buffer->mFrameCount; - // FIXME - // check mUnreleased - // verify that stepCount <= frameCount returned by the last obtainBuffer() - // verify stepCount not > total frame count of pipe - if (stepCount == 0) { + if (stepCount == 0 || mIsShutdown) { + // prevent accidental re-use of buffer + buffer->mFrameCount = 0; + buffer->mRaw = NULL; + buffer->mNonContig = 0; return; } + LOG_ALWAYS_FATAL_IF(!(stepCount <= mUnreleased && mUnreleased <= mFrameCount)); + mUnreleased -= stepCount; audio_track_cblk_t* cblk = mCblk; // Both of these barriers are required if (mIsOut) { @@ -362,20 +364,18 @@ size_t StaticAudioTrackClientProxy::getBufferPosition() ServerProxy::ServerProxy(audio_track_cblk_t* cblk, void *buffers, size_t frameCount, size_t frameSize, bool isOut, bool clientInServer) - : Proxy(cblk, buffers, frameCount, frameSize, isOut, clientInServer), mUnreleased(0), + : Proxy(cblk, buffers, frameCount, frameSize, isOut, clientInServer), mAvailToClient(0), mFlush(0), mDeferWake(false) { } status_t ServerProxy::obtainBuffer(Buffer* buffer) { + LOG_ALWAYS_FATAL_IF(buffer == NULL || buffer->mFrameCount == 0); if (mIsShutdown) { - buffer->mFrameCount = 0; - buffer->mRaw = NULL; - buffer->mNonContig = 0; - mUnreleased = 0; - return NO_INIT; + goto no_init; } + { audio_track_cblk_t* cblk = mCblk; // compute number of frames available to write (AudioTrack) or read (AudioRecord), // or use previous cached value from framesReady(), with added barrier if it omits. @@ -402,11 +402,7 @@ status_t ServerProxy::obtainBuffer(Buffer* buffer) mIsShutdown = true; } if (mIsShutdown) { - buffer->mFrameCount = 0; - buffer->mRaw = NULL; - buffer->mNonContig = 0; - mUnreleased = 0; - return NO_INIT; + goto no_init; } // don't allow filling pipe beyond the nominal size size_t availToServer; @@ -443,23 +439,27 @@ status_t ServerProxy::obtainBuffer(Buffer* buffer) // FIXME need to test for recording mDeferWake = part1 < ask && availToServer >= ask; return part1 > 0 ? NO_ERROR : WOULD_BLOCK; + } +no_init: + buffer->mFrameCount = 0; + buffer->mRaw = NULL; + buffer->mNonContig = 0; + mUnreleased = 0; + return NO_INIT; } void ServerProxy::releaseBuffer(Buffer* buffer) { - if (mIsShutdown) { - buffer->mFrameCount = 0; - buffer->mRaw = NULL; - buffer->mNonContig = 0; - return; - } + LOG_ALWAYS_FATAL_IF(buffer == NULL); size_t stepCount = buffer->mFrameCount; - LOG_ALWAYS_FATAL_IF(stepCount > mUnreleased); - if (stepCount == 0) { + if (stepCount == 0 || mIsShutdown) { + // prevent accidental re-use of buffer + buffer->mFrameCount = 0; buffer->mRaw = NULL; buffer->mNonContig = 0; return; } + LOG_ALWAYS_FATAL_IF(!(stepCount <= mUnreleased && mUnreleased <= mFrameCount)); mUnreleased -= stepCount; audio_track_cblk_t* cblk = mCblk; if (mIsOut) { @@ -637,8 +637,9 @@ status_t StaticAudioTrackServerProxy::obtainBuffer(Buffer* buffer) void StaticAudioTrackServerProxy::releaseBuffer(Buffer* buffer) { size_t stepCount = buffer->mFrameCount; - LOG_ALWAYS_FATAL_IF(stepCount > mUnreleased); + LOG_ALWAYS_FATAL_IF(!(stepCount <= mUnreleased)); if (stepCount == 0) { + // prevent accidental re-use of buffer buffer->mRaw = NULL; buffer->mNonContig = 0; return; -- cgit v1.1