Merge "Cleanup CreateAudioPlayer and CreateMediaPlayer"

4 files changed, 49 insertions, 55 deletions

diff --git a/wilhelm/src/android/AudioPlayer_to_android.cpp b/wilhelm/src/android/AudioPlayer_to_android.cpp
index 571388c8..2b47b70e 100644
--- a/wilhelm/src/android/AudioPlayer_to_android.cpp
+++ b/wilhelm/src/android/AudioPlayer_to_android.cpp
@@ -544,12 +544,6 @@ void audioPlayer_auxEffectUpdate(CAudioPlayer* ap) {
 
 
 //-----------------------------------------------------------------------------
-void audioPlayer_setInvalid(CAudioPlayer* ap) {
-    ap->mAndroidObjType = INVALID_TYPE;
-}
-
-
-//-----------------------------------------------------------------------------
 /*
  * returns true if the given data sink is supported by AudioPlayer that doesn't
  *   play to an OutputMix object, false otherwise
@@ -1251,53 +1245,48 @@ static void audioTrack_callBack_pullFromBuffQueue(int event, void* user, void *i
 
 
 //-----------------------------------------------------------------------------
-SLresult android_audioPlayer_create(CAudioPlayer *pAudioPlayer) {
+void android_audioPlayer_create(CAudioPlayer *pAudioPlayer) {
 
-    SLresult result = SL_RESULT_SUCCESS;
-    // pAudioPlayer->mAndroidObjType has been set in audioPlayer_getAndroidObjectTypeForSourceSink()
-    if (INVALID_TYPE == pAudioPlayer->mAndroidObjType) {
-        audioPlayer_setInvalid(pAudioPlayer);
-        result = SL_RESULT_PARAMETER_INVALID;
-    } else {
+    // pAudioPlayer->mAndroidObjType has been set in android_audioPlayer_checkSourceSink()
+    // and if it was == INVALID_TYPE, then IEngine_CreateAudioPlayer would never call us
+    assert(INVALID_TYPE != pAudioPlayer->mAndroidObjType);
 
-        // These initializations are in the same order as the field declarations in classes.h
+    // These initializations are in the same order as the field declarations in classes.h
 
-        // FIXME Consolidate initializations (many of these already in IEngine_CreateAudioPlayer)
-        // mAndroidObjType: see above comment
-        pAudioPlayer->mAndroidObjState = ANDROID_UNINITIALIZED;
-        pAudioPlayer->mSessionId = android::AudioSystem::newAudioSessionId();
-        pAudioPlayer->mStreamType = ANDROID_DEFAULT_OUTPUT_STREAM_TYPE;
+    // FIXME Consolidate initializations (many of these already in IEngine_CreateAudioPlayer)
+    // mAndroidObjType: see above comment
+    pAudioPlayer->mAndroidObjState = ANDROID_UNINITIALIZED;
+    pAudioPlayer->mSessionId = android::AudioSystem::newAudioSessionId();
 
-        // mAudioTrack
-        pAudioPlayer->mCallbackProtector = new android::CallbackProtector();
-        // mAPLayer
-        // mAuxEffect
+    // placeholder: not necessary yet as session ID lifetime doesn't extend beyond player
+    // android::AudioSystem::acquireAudioSessionId(pAudioPlayer->mSessionId);
 
-        pAudioPlayer->mAuxSendLevel = 0;
-        pAudioPlayer->mAmplFromDirectLevel = 1.0f; // matches initial mDirectLevel value
-        pAudioPlayer->mDeferredStart = false;
+    pAudioPlayer->mStreamType = ANDROID_DEFAULT_OUTPUT_STREAM_TYPE;
 
-        // Already initialized in IEngine_CreateAudioPlayer, to be consolidated
-        pAudioPlayer->mDirectLevel = 0; // no attenuation
+    // mAudioTrack
+    pAudioPlayer->mCallbackProtector = new android::CallbackProtector();
+    // mAPLayer
+    // mAuxEffect
 
-        // This section re-initializes interface-specific fields that
-        // can be set or used regardless of whether the interface is
-        // exposed on the AudioPlayer or not
+    pAudioPlayer->mAuxSendLevel = 0;
+    pAudioPlayer->mAmplFromDirectLevel = 1.0f; // matches initial mDirectLevel value
+    pAudioPlayer->mDeferredStart = false;
 
-        // Only AudioTrack supports a non-trivial playback rate
-        switch (pAudioPlayer->mAndroidObjType) {
-        case AUDIOPLAYER_FROM_PCM_BUFFERQUEUE:
-            pAudioPlayer->mPlaybackRate.mMinRate = AUDIOTRACK_MIN_PLAYBACKRATE_PERMILLE;
-            pAudioPlayer->mPlaybackRate.mMaxRate = AUDIOTRACK_MAX_PLAYBACKRATE_PERMILLE;
-            break;
-        default:
-            // use the default range
-            break;
-        }
+    // This section re-initializes interface-specific fields that
+    // can be set or used regardless of whether the interface is
+    // exposed on the AudioPlayer or not
 
+    // Only AudioTrack supports a non-trivial playback rate
+    switch (pAudioPlayer->mAndroidObjType) {
+    case AUDIOPLAYER_FROM_PCM_BUFFERQUEUE:
+        pAudioPlayer->mPlaybackRate.mMinRate = AUDIOTRACK_MIN_PLAYBACKRATE_PERMILLE;
+        pAudioPlayer->mPlaybackRate.mMaxRate = AUDIOTRACK_MAX_PLAYBACKRATE_PERMILLE;
+        break;
+    default:
+        // use the default range
+        break;
     }
 
-    return result;
 }
 
 
@@ -1639,10 +1628,10 @@ SLresult android_audioPlayer_destroy(CAudioPlayer *pAudioPlayer) {
         break;
     }
 
-    pAudioPlayer->mCallbackProtector.clear();
+    // placeholder: not necessary yet as session ID lifetime doesn't extend beyond player
+    // android::AudioSystem::releaseAudioSessionId(pAudioPlayer->mSessionId);
 
-    // FIXME might not be needed
-    pAudioPlayer->mAndroidObjType = INVALID_TYPE;
+    pAudioPlayer->mCallbackProtector.clear();
 
     // explicit destructor
     pAudioPlayer->mAudioTrack.~sp();
diff --git a/wilhelm/src/android/AudioPlayer_to_android.h b/wilhelm/src/android/AudioPlayer_to_android.h
index 430b6227..12113b58 100644
--- a/wilhelm/src/android/AudioPlayer_to_android.h
+++ b/wilhelm/src/android/AudioPlayer_to_android.h
@@ -28,14 +28,9 @@
 extern SLresult android_audioPlayer_checkSourceSink(CAudioPlayer *pAudioPlayer);
 
 /*
- * Determines the Android media framework object that maps to the given audio source and sink.
- * Return
- *     SL_RESULT_SUCCESS if the Android resources were successfully created
- *     SL_PARAMETER_INVALID if the Android resources couldn't be created due to an invalid or
- *         unsupported parameter or value
- *     SL_RESULT_CONTENT_UNSUPPORTED if a format is not supported (e.g. sample rate too high)
+ * Finish the Android-specific pre-Realize initialization of a CAudioPlayer.
  */
-extern SLresult android_audioPlayer_create(CAudioPlayer *pAudioPlayer);
+extern void android_audioPlayer_create(CAudioPlayer *pAudioPlayer);
 
 /*
  * Allocates and initializes the Android media framework objects intended to be used with the
diff --git a/wilhelm/src/android/MediaPlayer_to_android.cpp b/wilhelm/src/android/MediaPlayer_to_android.cpp
index 692aa5c5..dedb8541 100644
--- a/wilhelm/src/android/MediaPlayer_to_android.cpp
+++ b/wilhelm/src/android/MediaPlayer_to_android.cpp
@@ -382,6 +382,7 @@ XAresult android_Player_create(CMediaPlayer *mp) {
         break;
     case XA_DATALOCATOR_ADDRESS: // intended fall-through
     default:
+        mp->mAndroidObjType = INVALID_TYPE;
         SL_LOGE("Unable to create MediaPlayer for data source locator 0x%x", sourceLocator);
         result = XA_RESULT_PARAMETER_INVALID;
         break;
@@ -392,6 +393,9 @@ XAresult android_Player_create(CMediaPlayer *mp) {
     mp->mStreamType = ANDROID_DEFAULT_OUTPUT_STREAM_TYPE;
     mp->mSessionId = android::AudioSystem::newAudioSessionId();
 
+    // placeholder: not necessary yet as session ID lifetime doesn't extend beyond player
+    // android::AudioSystem::acquireAudioSessionId(mp->mSessionId);
+
     mp->mCallbackProtector = new android::CallbackProtector();
 
     return result;
@@ -490,6 +494,10 @@ XAresult android_Player_destroy(CMediaPlayer *mp) {
     SL_LOGV("android_Player_destroy(%p)", mp);
 
     mp->mAVPlayer.clear();
+
+    // placeholder: not necessary yet as session ID lifetime doesn't extend beyond player
+    // android::AudioSystem::releaseAudioSessionId(mp->mSessionId);
+
     mp->mCallbackProtector.clear();
 
     // explicit destructor
diff --git a/wilhelm/src/itf/IEngine.c b/wilhelm/src/itf/IEngine.c
index b024fca7..4b6d2b68 100644
--- a/wilhelm/src/itf/IEngine.c
+++ b/wilhelm/src/itf/IEngine.c
@@ -212,7 +212,7 @@ static SLresult IEngine_CreateAudioPlayer(SLEngineItf self, SLObjectItf *pPlayer
                     thiz->mSampleRateMilliHz = UNKNOWN_SAMPLERATE;
 
                     // More default values, in case destructor needs to be called early
-                    thiz->mDirectLevel = 0;
+                    thiz->mDirectLevel = 0; // no attenuation
 #ifdef USE_OUTPUTMIXEXT
                     thiz->mTrack = NULL;
                     thiz->mGains[0] = 1.0f;
@@ -237,6 +237,8 @@ static SLresult IEngine_CreateAudioPlayer(SLEngineItf self, SLObjectItf *pPlayer
                             android::sp<android::CallbackProtector>();
                     (void) new (&thiz->mAuxEffect) android::sp<android::AudioEffect>();
                     (void) new (&thiz->mAPlayer) android::sp<android::GenericPlayer>();
+                    // Android-specific POD fields are initialized in android_audioPlayer_create,
+                    // and assume calloc or memset 0 during allocation
 #endif
 
                     // Check the source and sink parameters against generic constraints,
@@ -1087,8 +1089,6 @@ static XAresult IEngine_CreateMediaPlayer(XAEngineItf self, XAObjectItf *pPlayer
                     // More default values, in case destructor needs to be called early
                     thiz->mNumChannels = UNKNOWN_NUMCHANNELS;
 
-                    // (assume calloc or memset 0 during allocation)
-                    // placement new
 #ifdef ANDROID
                     // placement new (explicit constructor)
                     // FIXME unnecessary once those fields are encapsulated in one class, rather
@@ -1096,6 +1096,8 @@ static XAresult IEngine_CreateMediaPlayer(XAEngineItf self, XAObjectItf *pPlayer
                     (void) new (&thiz->mAVPlayer) android::sp<android::GenericPlayer>();
                     (void) new (&thiz->mCallbackProtector)
                             android::sp<android::CallbackProtector>();
+                    // Android-specific POD fields are initialized in android_Player_create,
+                    // and assume calloc or memset 0 during allocation
 #endif
 
                     // Check the source and sink parameters against generic constraints