From 3ac54e4d6e699467b608a4f0eda6023d50d3cd61 Mon Sep 17 00:00:00 2001
From: Glenn Kasten <gkasten@google.com>
Date: Wed, 22 Sep 2010 18:55:12 -0700
Subject: [PATCH] Fix bug found by BufferQueue automated test

Fix bug for destroying partially constructed audio player.
Remove obsolete logging.

Change-Id: Ib186153bf50ed1311c021d53711ec90d000bdee2
---
 opensles/libopensles/CAudioPlayer.c           |  9 +++++++++
 opensles/libopensles/IOutputMixExt.c          |  1 +
 opensles/tests/automated/BufferQueue_test.cpp | 10 +++++-----
 3 files changed, 15 insertions(+), 5 deletions(-)

diff --git a/opensles/libopensles/CAudioPlayer.c b/opensles/libopensles/CAudioPlayer.c
index 956a946e..d27f15b5 100644
--- a/opensles/libopensles/CAudioPlayer.c
+++ b/opensles/libopensles/CAudioPlayer.c
@@ -72,6 +72,15 @@ bool CAudioPlayer_PreDestroy(void *self)
 {
 #ifdef USE_OUTPUTMIXEXT
     CAudioPlayer *this = (CAudioPlayer *) self;
+    // Safe to proceed immediately if a track has not yet been assigned
+    Track *track = this->mTrack;
+    if (NULL == track)
+        return true;
+    // FIXME We're reading from track without taking even a share lock on it
+    CAudioPlayer *audioPlayer = track->mAudioPlayer;
+    if (NULL == audioPlayer)
+        return true;
+    assert(audioPlayer == this);
     // Request the mixer thread to unlink this audio player's track
     this->mDestroyRequested = true;
     while (this->mDestroyRequested) {
diff --git a/opensles/libopensles/IOutputMixExt.c b/opensles/libopensles/IOutputMixExt.c
index 2025e544..b64bc636 100644
--- a/opensles/libopensles/IOutputMixExt.c
+++ b/opensles/libopensles/IOutputMixExt.c
@@ -392,6 +392,7 @@ SLresult IOutputMixExt_checkAudioPlayerSourceSink(CAudioPlayer *this)
     // allocation, initialize rest of track, and doubly-link track to player (currently single).
     assert(NULL != track);
     track->mBufferQueue = &this->mBufferQueue;
+    // FIXME should be last thing to do? and is the track locked? probably not
     track->mAudioPlayer = this;
     track->mReader = NULL;
     track->mAvail = 0;
diff --git a/opensles/tests/automated/BufferQueue_test.cpp b/opensles/tests/automated/BufferQueue_test.cpp
index 31b35739..34cb850b 100644
--- a/opensles/tests/automated/BufferQueue_test.cpp
+++ b/opensles/tests/automated/BufferQueue_test.cpp
@@ -275,13 +275,13 @@ protected:
         CheckErr(res);
         ASSERT_EQ((SLuint32) 1, bufferqueueState.count);
         ASSERT_EQ((SLuint32) 0, bufferqueueState.playIndex);
-        LOGV("Before 1.5 sec");
+        //LOGV("Before 1.5 sec");
         // wait 1.5 seconds
         usleep(1500000);
-        LOGV("After 1.5 sec");
+        //LOGV("After 1.5 sec");
         // state should still be playing
         res = (*playerPlay)->GetPlayState(playerPlay, &playerState);
-        LOGV("GetPlayState");
+        //LOGV("GetPlayState");
         CheckErr(res);
         ASSERT_EQ(SL_PLAYSTATE_PLAYING, playerState);
         // buffer should be removed from the queue
@@ -289,12 +289,12 @@ protected:
         CheckErr(res);
         ASSERT_EQ((SLuint32) 0, bufferqueueState.count);
         ASSERT_EQ((SLuint32) 1, bufferqueueState.playIndex);
-        LOGV("TestEnd");
+        //LOGV("TestEnd");
     }
 };
 
 TEST_F(TestBufferQueue, testInvalidBuffer){
-    LOGV("Test Fixture: InvalidBuffer");
+    //LOGV("Test Fixture: InvalidBuffer");
     InvalidBuffer();
 }
 
-- 
2.11.0