From 2799d743ee2ae5a25fe869a7f9c052acc029559f Mon Sep 17 00:00:00 2001 From: Glenn Kasten Date: Thu, 30 May 2013 14:33:29 -0700 Subject: Use sp instead of raw AudioTrack * This change prepares for the new implementation of AudioTrack client, which will require clients to use only sp, not raw AudioTrack *. A raw delete will cause a race condition during AudioTrack destruction. AudioTrack was made a RefBase by commit b68a91a70bc8d0d18e7404e14443d4e4020b3635 on 2011/11/15, when it was needed by OpenSL ES (for the callback protector). At that time, the only other client that was also converted from AudioTrack * to sp was android.media.AudioTrack JNI in project frameworks/base (file android_media_AudioTrack.cpp). Details: * Use .clear() instead of delete followed by = NULL. * ALOG %p need .get(). * sp<> don't need to be listed in constructor initializer, if initially 0. * Use == 0 for sp<> vs == NULL for raw pointers. * Use if (sp != 0) instead of if (raw). Change-Id: Ic7cad25795d6e862e112abdc227b6d33afdfce17 --- media/libmedia/JetPlayer.cpp | 6 ++-- media/libmedia/SoundPool.cpp | 14 ++++---- media/libmedia/ToneGenerator.cpp | 22 ++++--------- media/libmediaplayerservice/MediaPlayerService.cpp | 37 +++++++++------------- media/libmediaplayerservice/MediaPlayerService.h | 6 ++-- media/libstagefright/AudioPlayer.cpp | 11 +++---- 6 files changed, 37 insertions(+), 59 deletions(-) (limited to 'media') diff --git a/media/libmedia/JetPlayer.cpp b/media/libmedia/JetPlayer.cpp index 59e538f..8fe5bb3 100644 --- a/media/libmedia/JetPlayer.cpp +++ b/media/libmedia/JetPlayer.cpp @@ -39,7 +39,6 @@ JetPlayer::JetPlayer(void *javaJetPlayer, int maxTracks, int trackBufferSize) : mMaxTracks(maxTracks), mEasData(NULL), mEasJetFileLoc(NULL), - mAudioTrack(NULL), mTrackBufferSize(trackBufferSize) { ALOGV("JetPlayer constructor"); @@ -140,11 +139,10 @@ int JetPlayer::release() free(mEasJetFileLoc); mEasJetFileLoc = NULL; } - if (mAudioTrack) { + if (mAudioTrack != 0) { mAudioTrack->stop(); mAudioTrack->flush(); - delete mAudioTrack; - mAudioTrack = NULL; + mAudioTrack.clear(); } if (mAudioBuffer) { delete mAudioBuffer; diff --git a/media/libmedia/SoundPool.cpp b/media/libmedia/SoundPool.cpp index ee70ef7..e1e88ec 100644 --- a/media/libmedia/SoundPool.cpp +++ b/media/libmedia/SoundPool.cpp @@ -547,8 +547,8 @@ void SoundChannel::init(SoundPool* soundPool) void SoundChannel::play(const sp& sample, int nextChannelID, float leftVolume, float rightVolume, int priority, int loop, float rate) { - AudioTrack* oldTrack; - AudioTrack* newTrack; + sp oldTrack; + sp newTrack; status_t status; { // scope for the lock @@ -620,7 +620,7 @@ void SoundChannel::play(const sp& sample, int nextChannelID, float leftV ALOGE("Error creating AudioTrack"); goto exit; } - ALOGV("setVolume %p", newTrack); + ALOGV("setVolume %p", newTrack.get()); newTrack->setVolume(leftVolume, rightVolume); newTrack->setLoop(0, frameCount, loop); @@ -643,11 +643,9 @@ void SoundChannel::play(const sp& sample, int nextChannelID, float leftV } exit: - ALOGV("delete oldTrack %p", oldTrack); - delete oldTrack; + ALOGV("delete oldTrack %p", oldTrack.get()); if (status != NO_ERROR) { - delete newTrack; - mAudioTrack = NULL; + mAudioTrack.clear(); } } @@ -884,7 +882,7 @@ SoundChannel::~SoundChannel() } // do not call AudioTrack destructor with mLock held as it will wait for the AudioTrack // callback thread to exit which may need to execute process() and acquire the mLock. - delete mAudioTrack; + mAudioTrack.clear(); } void SoundChannel::dump() diff --git a/media/libmedia/ToneGenerator.cpp b/media/libmedia/ToneGenerator.cpp index f55b697..ebe1ba1 100644 --- a/media/libmedia/ToneGenerator.cpp +++ b/media/libmedia/ToneGenerator.cpp @@ -803,7 +803,6 @@ ToneGenerator::ToneGenerator(audio_stream_type_t streamType, float volume, bool ALOGV("ToneGenerator constructor: streamType=%d, volume=%f", streamType, volume); mState = TONE_IDLE; - mpAudioTrack = NULL; if (AudioSystem::getOutputSamplingRate(&mSamplingRate, streamType) != NO_ERROR) { ALOGE("Unable to marshal AudioFlinger"); @@ -855,10 +854,10 @@ ToneGenerator::ToneGenerator(audio_stream_type_t streamType, float volume, bool ToneGenerator::~ToneGenerator() { ALOGV("ToneGenerator destructor"); - if (mpAudioTrack != NULL) { + if (mpAudioTrack != 0) { stopTone(); - ALOGV("Delete Track: %p", mpAudioTrack); - delete mpAudioTrack; + ALOGV("Delete Track: %p", mpAudioTrack.get()); + mpAudioTrack.clear(); } } @@ -1047,14 +1046,9 @@ void ToneGenerator::stopTone() { //////////////////////////////////////////////////////////////////////////////// bool ToneGenerator::initAudioTrack() { - if (mpAudioTrack) { - delete mpAudioTrack; - mpAudioTrack = NULL; - } - // Open audio track in mono, PCM 16bit, default sampling rate, default buffer size mpAudioTrack = new AudioTrack(); - ALOGV("Create Track: %p", mpAudioTrack); + ALOGV("Create Track: %p", mpAudioTrack.get()); mpAudioTrack->set(mStreamType, 0, // sampleRate @@ -1081,12 +1075,10 @@ bool ToneGenerator::initAudioTrack() { initAudioTrack_exit: + ALOGV("Init failed: %p", mpAudioTrack.get()); + // Cleanup - if (mpAudioTrack != NULL) { - ALOGV("Delete Track I: %p", mpAudioTrack); - delete mpAudioTrack; - mpAudioTrack = NULL; - } + mpAudioTrack.clear(); return false; } diff --git a/media/libmediaplayerservice/MediaPlayerService.cpp b/media/libmediaplayerservice/MediaPlayerService.cpp index e600a3f..fa1ff36 100644 --- a/media/libmediaplayerservice/MediaPlayerService.cpp +++ b/media/libmediaplayerservice/MediaPlayerService.cpp @@ -1295,8 +1295,6 @@ MediaPlayerService::AudioOutput::AudioOutput(int sessionId) mSessionId(sessionId), mFlags(AUDIO_OUTPUT_FLAG_NONE) { ALOGV("AudioOutput(%d)", sessionId); - mTrack = 0; - mRecycledTrack = 0; mStreamType = AUDIO_STREAM_MUSIC; mLeftVolume = 1.0; mRightVolume = 1.0; @@ -1311,7 +1309,6 @@ MediaPlayerService::AudioOutput::AudioOutput(int sessionId) MediaPlayerService::AudioOutput::~AudioOutput() { close(); - delete mRecycledTrack; delete mCallbackData; } @@ -1422,7 +1419,7 @@ status_t MediaPlayerService::AudioOutput::open( } } - AudioTrack *t; + sp t; CallbackData *newcbd = NULL; if (mCallback != NULL) { newcbd = new CallbackData(this); @@ -1453,13 +1450,12 @@ status_t MediaPlayerService::AudioOutput::open( if ((t == 0) || (t->initCheck() != NO_ERROR)) { ALOGE("Unable to create audio track"); - delete t; delete newcbd; return NO_INIT; } - if (mRecycledTrack) { + if (mRecycledTrack != 0) { // check if the existing track can be reused as-is, or if a new track needs to be created. bool reuse = true; @@ -1484,11 +1480,10 @@ status_t MediaPlayerService::AudioOutput::open( ALOGV("chaining to next output"); close(); mTrack = mRecycledTrack; - mRecycledTrack = NULL; + mRecycledTrack.clear(); if (mCallbackData != NULL) { mCallbackData->setOutput(this); } - delete t; delete newcbd; return OK; } @@ -1499,8 +1494,7 @@ status_t MediaPlayerService::AudioOutput::open( mCallbackData->endTrackSwitch(); } mRecycledTrack->flush(); - delete mRecycledTrack; - mRecycledTrack = NULL; + mRecycledTrack.clear(); delete mCallbackData; mCallbackData = NULL; close(); @@ -1533,7 +1527,7 @@ void MediaPlayerService::AudioOutput::start() if (mCallbackData != NULL) { mCallbackData->endTrackSwitch(); } - if (mTrack) { + if (mTrack != 0) { mTrack->setVolume(mLeftVolume, mRightVolume); mTrack->setAuxEffectSendLevel(mSendLevel); mTrack->start(); @@ -1555,7 +1549,7 @@ void MediaPlayerService::AudioOutput::switchToNextOutput() { mNextOutput->mCallbackData = mCallbackData; mCallbackData = NULL; mNextOutput->mRecycledTrack = mTrack; - mTrack = NULL; + mTrack.clear(); mNextOutput->mSampleRateHz = mSampleRateHz; mNextOutput->mMsecsPerFrame = mMsecsPerFrame; mNextOutput->mBytesWritten = mBytesWritten; @@ -1568,7 +1562,7 @@ ssize_t MediaPlayerService::AudioOutput::write(const void* buffer, size_t size) LOG_FATAL_IF(mCallback != NULL, "Don't call write if supplying a callback."); //ALOGV("write(%p, %u)", buffer, size); - if (mTrack) { + if (mTrack != 0) { ssize_t ret = mTrack->write(buffer, size); mBytesWritten += ret; return ret; @@ -1579,26 +1573,25 @@ ssize_t MediaPlayerService::AudioOutput::write(const void* buffer, size_t size) void MediaPlayerService::AudioOutput::stop() { ALOGV("stop"); - if (mTrack) mTrack->stop(); + if (mTrack != 0) mTrack->stop(); } void MediaPlayerService::AudioOutput::flush() { ALOGV("flush"); - if (mTrack) mTrack->flush(); + if (mTrack != 0) mTrack->flush(); } void MediaPlayerService::AudioOutput::pause() { ALOGV("pause"); - if (mTrack) mTrack->pause(); + if (mTrack != 0) mTrack->pause(); } void MediaPlayerService::AudioOutput::close() { ALOGV("close"); - delete mTrack; - mTrack = 0; + mTrack.clear(); } void MediaPlayerService::AudioOutput::setVolume(float left, float right) @@ -1606,7 +1599,7 @@ void MediaPlayerService::AudioOutput::setVolume(float left, float right) ALOGV("setVolume(%f, %f)", left, right); mLeftVolume = left; mRightVolume = right; - if (mTrack) { + if (mTrack != 0) { mTrack->setVolume(left, right); } } @@ -1615,7 +1608,7 @@ status_t MediaPlayerService::AudioOutput::setPlaybackRatePermille(int32_t ratePe { ALOGV("setPlaybackRatePermille(%d)", ratePermille); status_t res = NO_ERROR; - if (mTrack) { + if (mTrack != 0) { res = mTrack->setSampleRate(ratePermille * mSampleRateHz / 1000); } else { res = NO_INIT; @@ -1631,7 +1624,7 @@ status_t MediaPlayerService::AudioOutput::setAuxEffectSendLevel(float level) { ALOGV("setAuxEffectSendLevel(%f)", level); mSendLevel = level; - if (mTrack) { + if (mTrack != 0) { return mTrack->setAuxEffectSendLevel(level); } return NO_ERROR; @@ -1641,7 +1634,7 @@ status_t MediaPlayerService::AudioOutput::attachAuxEffect(int effectId) { ALOGV("attachAuxEffect(%d)", effectId); mAuxEffectId = effectId; - if (mTrack) { + if (mTrack != 0) { return mTrack->attachAuxEffect(effectId); } return NO_ERROR; diff --git a/media/libmediaplayerservice/MediaPlayerService.h b/media/libmediaplayerservice/MediaPlayerService.h index b33805d..e586156 100644 --- a/media/libmediaplayerservice/MediaPlayerService.h +++ b/media/libmediaplayerservice/MediaPlayerService.h @@ -78,7 +78,7 @@ class MediaPlayerService : public BnMediaPlayerService AudioOutput(int sessionId); virtual ~AudioOutput(); - virtual bool ready() const { return mTrack != NULL; } + virtual bool ready() const { return mTrack != 0; } virtual bool realtime() const { return true; } virtual ssize_t bufferSize() const; virtual ssize_t frameCount() const; @@ -120,8 +120,8 @@ class MediaPlayerService : public BnMediaPlayerService static void CallbackWrapper( int event, void *me, void *info); - AudioTrack* mTrack; - AudioTrack* mRecycledTrack; + sp mTrack; + sp mRecycledTrack; sp mNextOutput; AudioCallback mCallback; void * mCallbackCookie; diff --git a/media/libstagefright/AudioPlayer.cpp b/media/libstagefright/AudioPlayer.cpp index 4208019..92efae8 100644 --- a/media/libstagefright/AudioPlayer.cpp +++ b/media/libstagefright/AudioPlayer.cpp @@ -36,8 +36,7 @@ AudioPlayer::AudioPlayer( const sp &audioSink, bool allowDeepBuffering, AwesomePlayer *observer) - : mAudioTrack(NULL), - mInputBuffer(NULL), + : mInputBuffer(NULL), mSampleRate(0), mLatencyUs(0), mFrameSize(0), @@ -166,8 +165,7 @@ status_t AudioPlayer::start(bool sourceAlreadyStarted) { 0, AUDIO_OUTPUT_FLAG_NONE, &AudioCallback, this, 0); if ((err = mAudioTrack->initCheck()) != OK) { - delete mAudioTrack; - mAudioTrack = NULL; + mAudioTrack.clear(); if (mFirstBuffer != NULL) { mFirstBuffer->release(); @@ -235,8 +233,7 @@ void AudioPlayer::reset() { } else { mAudioTrack->stop(); - delete mAudioTrack; - mAudioTrack = NULL; + mAudioTrack.clear(); } // Make sure to release any buffer we hold onto so that the @@ -297,7 +294,7 @@ bool AudioPlayer::reachedEOS(status_t *finalStatus) { status_t AudioPlayer::setPlaybackRatePermille(int32_t ratePermille) { if (mAudioSink.get() != NULL) { return mAudioSink->setPlaybackRatePermille(ratePermille); - } else if (mAudioTrack != NULL){ + } else if (mAudioTrack != 0){ return mAudioTrack->setSampleRate(ratePermille * mSampleRate / 1000); } else { return NO_INIT; -- cgit v1.1