commit 2799d743ee2ae5a25fe869a7f9c052acc029559f
author Glenn Kasten <gkasten@google.com> Thu May 30 14:33:29 2013 -0700
committer Glenn Kasten <gkasten@google.com> Mon Jun 03 11:28:36 2013 -0700
tree 5cab5ff06f7cae0025b93fdcd549cfb2ec72b755
parent 7c684c622110af460025eff85082030947278430

Use sp<AudioTrack> instead of raw AudioTrack *

This change prepares for the new implementation of AudioTrack client, which
will require clients to use only sp<AudioTrack>, 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<AudioTrack> 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

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 @@
         mMaxTracks(maxTracks),
         mEasData(NULL),
         mEasJetFileLoc(NULL),
-        mAudioTrack(NULL),
         mTrackBufferSize(trackBufferSize)
 {
     ALOGV("JetPlayer constructor");
@@ -140,11 +139,10 @@
         free(mEasJetFileLoc);
         mEasJetFileLoc = NULL;
     }
-    if (mAudioTrack) {
+    if (mAudioTrack != 0) {
         mAudioTrack->stop();
         mAudioTrack->flush();
-        delete mAudioTrack;
-        mAudioTrack = NULL;
+        mAudioTrack.clear();
     }
     if (mAudioBuffer) {
         delete mAudioBuffer;

media/libmedia/SoundPool.cpp

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::play(const sp<Sample>& sample, int nextChannelID, float leftVolume,
         float rightVolume, int priority, int loop, float rate)
 {
-    AudioTrack* oldTrack;
-    AudioTrack* newTrack;
+    sp<AudioTrack> oldTrack;
+    sp<AudioTrack> newTrack;
     status_t status;
 
     { // scope for the lock
@@ -620,7 +620,7 @@
             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 @@
     }
 
 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 @@
     }
     // 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()

media/libmedia/ToneGenerator.cpp

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 @@
     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() {
     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 @@
 ////////////////////////////////////////////////////////////////////////////////
 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 @@
 
 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;
 }