From 6ec546dad994d3cc7ffbdf9be42d69811805085a Mon Sep 17 00:00:00 2001 From: Eric Laurent Date: Wed, 10 Oct 2018 16:52:14 -0700 Subject: [PATCH] audio: fix AudioTrack and AudioRecord restore Allow retry on failure to reconnected to audioserver when restoring an AudioTrack or AudioRecord and do not change state to inactive when reconnection fails. In case of failure due to temporary race conditions this will increase the chances of success and avoid ending up in an inconsistent active state if a later reconnection succeeds. Bug: 113109529 Test: AudioTrack CTS tests and manual playback smoke tests. Test: AudioRecord CTS tests and manual capture smoke tests. Change-Id: I570724eb46f3a0f830267396aba06297bd37df7a --- media/libaudioclient/AudioRecord.cpp | 17 ++++++++--------- media/libaudioclient/AudioTrack.cpp | 15 ++++++++------- 2 files changed, 16 insertions(+), 16 deletions(-) diff --git a/media/libaudioclient/AudioRecord.cpp b/media/libaudioclient/AudioRecord.cpp index 34c542854e..038c854be8 100644 --- a/media/libaudioclient/AudioRecord.cpp +++ b/media/libaudioclient/AudioRecord.cpp @@ -1300,10 +1300,7 @@ retry: mNewPosition = position + mUpdatePeriod; status_t result = createRecord_l(position, mOpPackageName); - if (result != NO_ERROR) { - ALOGW("%s(%d): createRecord_l failed, do not retry", __func__, mId); - retries = 0; - } else { + if (result == NO_ERROR) { if (mActive) { // callback thread or sync event hasn't changed // FIXME this fails if we have a new AudioFlinger instance @@ -1316,13 +1313,15 @@ retry: if (result != NO_ERROR) { ALOGW("%s(%d): failed status %d, retries %d", __func__, mId, result, retries); if (--retries > 0) { + // leave time for an eventual race condition to clear before retrying + usleep(500000); goto retry; } - } - - if (result != NO_ERROR) { - ALOGW("%s(%d): failed status %d", __func__, mId, result); - mActive = false; + // if no retries left, set invalid bit to force restoring at next occasion + // and avoid inconsistent active state on client and server sides + if (mCblk != nullptr) { + android_atomic_or(CBLK_INVALID, &mCblk->mFlags); + } } return result; diff --git a/media/libaudioclient/AudioTrack.cpp b/media/libaudioclient/AudioTrack.cpp index e77abc657c..c86d4ce261 100644 --- a/media/libaudioclient/AudioTrack.cpp +++ b/media/libaudioclient/AudioTrack.cpp @@ -2308,10 +2308,7 @@ retry: // If a new IAudioTrack cannot be created, the previous (dead) instance will be left intact. status_t result = createTrack_l(); - if (result != NO_ERROR) { - ALOGW("%s(%d): createTrack_l failed, do not retry", __func__, mId); - retries = 0; - } else { + if (result == NO_ERROR) { // take the frames that will be lost by track recreation into account in saved position // For streaming tracks, this is the amount we obtained from the user/client // (not the number actually consumed at the server - those are already lost). @@ -2358,12 +2355,16 @@ retry: if (result != NO_ERROR) { ALOGW("%s(%d): failed status %d, retries %d", __func__, mId, result, retries); if (--retries > 0) { + // leave time for an eventual race condition to clear before retrying + usleep(500000); goto retry; } - mState = STATE_STOPPED; - mReleased = 0; + // if no retries left, set invalid bit to force restoring at next occasion + // and avoid inconsistent active state on client and server sides + if (mCblk != nullptr) { + android_atomic_or(CBLK_INVALID, &mCblk->mFlags); + } } - return result; }