From 0bbdb71a121566eecb8d7125403be0bdca9c5e93 Mon Sep 17 00:00:00 2001 From: Jan Sebechlebsky Date: Mon, 9 Jan 2023 14:29:19 +0100 Subject: [PATCH] Fix MediaPlayer construction with AUDIO_SESSION_ALLOCATE. When AUDIO_SESSION_ALLOCATE is used during construction, the media player should allocate new unique session id. However, the previous call to native_set_session id tried to acquire session_id with value 0, which correspond to restricted OUTPUT_MIX session and caused failure during initialization. The topic also modifies the native initizalization code, so it's possible to pass explicit session id during construction of native MediaPlayer instance (prior to this change, the native constructor would always allocate new session id from audio server, even though in some cases it might have been immediately changed to exliplicitly requested session id). Bug: 263373078 Bug: 263362598 Test: atest MediaPlayerUnitTest Test: atest CtsMediaAudioTestCases CtsMediaPlayerTestCases Change-Id: I92b085e23c72e0b9d21e12792bf67c2497aa0e25 --- media/java/android/media/MediaPlayer.java | 12 ++++-------- media/jni/android_media_MediaPlayer.cpp | 10 +++++++--- .../mediaframeworktest/unit/MediaPlayerUnitTest.java | 2 ++ 3 files changed, 13 insertions(+), 11 deletions(-) diff --git a/media/java/android/media/MediaPlayer.java b/media/java/android/media/MediaPlayer.java index 273c7af7d244b..4323c738f910e 100644 --- a/media/java/android/media/MediaPlayer.java +++ b/media/java/android/media/MediaPlayer.java @@ -708,12 +708,10 @@ public class MediaPlayer extends PlayerBase * It's easier to create it here than in C++. */ try (ScopedParcelState attributionSourceState = attributionSource.asScopedParcelState()) { - native_setup(new WeakReference(this), attributionSourceState.getParcel()); + native_setup(new WeakReference<>(this), attributionSourceState.getParcel(), + resolvePlaybackSessionId(context, sessionId)); } - - int effectiveSessionId = resolvePlaybackSessionId(context, sessionId); - baseRegisterPlayer(effectiveSessionId); - native_setAudioSessionId(effectiveSessionId); + baseRegisterPlayer(getAudioSessionId()); } private Parcel createPlayerIIdParcel() { @@ -1022,8 +1020,6 @@ public class MediaPlayer extends PlayerBase final AudioAttributes aa = audioAttributes != null ? audioAttributes : new AudioAttributes.Builder().build(); mp.setAudioAttributes(aa); - mp.native_setAudioSessionId(audioSessionId); - mp.setDataSource(afd.getFileDescriptor(), afd.getStartOffset(), afd.getLength()); afd.close(); mp.prepare(); @@ -2521,7 +2517,7 @@ public class MediaPlayer extends PlayerBase private static native final void native_init(); private native void native_setup(Object mediaplayerThis, - @NonNull Parcel attributionSource); + @NonNull Parcel attributionSource, int audioSessionId); private native final void native_finalize(); /** diff --git a/media/jni/android_media_MediaPlayer.cpp b/media/jni/android_media_MediaPlayer.cpp index da920bb63178d..95522001d3420 100644 --- a/media/jni/android_media_MediaPlayer.cpp +++ b/media/jni/android_media_MediaPlayer.cpp @@ -956,14 +956,16 @@ android_media_MediaPlayer_native_init(JNIEnv *env) static void android_media_MediaPlayer_native_setup(JNIEnv *env, jobject thiz, jobject weak_this, - jobject jAttributionSource) + jobject jAttributionSource, + jint jAudioSessionId) { ALOGV("native_setup"); Parcel* parcel = parcelForJavaObject(env, jAttributionSource); android::content::AttributionSourceState attributionSource; attributionSource.readFromParcel(parcel); - sp mp = sp::make(attributionSource); + sp mp = sp::make( + attributionSource, static_cast(jAudioSessionId)); if (mp == NULL) { jniThrowException(env, "java/lang/RuntimeException", "Out of memory"); return; @@ -1419,7 +1421,9 @@ static const JNINativeMethod gMethods[] = { {"native_setMetadataFilter", "(Landroid/os/Parcel;)I", (void *)android_media_MediaPlayer_setMetadataFilter}, {"native_getMetadata", "(ZZLandroid/os/Parcel;)Z", (void *)android_media_MediaPlayer_getMetadata}, {"native_init", "()V", (void *)android_media_MediaPlayer_native_init}, - {"native_setup", "(Ljava/lang/Object;Landroid/os/Parcel;)V",(void *)android_media_MediaPlayer_native_setup}, + {"native_setup", + "(Ljava/lang/Object;Landroid/os/Parcel;I)V", + (void *)android_media_MediaPlayer_native_setup}, {"native_finalize", "()V", (void *)android_media_MediaPlayer_native_finalize}, {"getAudioSessionId", "()I", (void *)android_media_MediaPlayer_get_audio_session_id}, {"native_setAudioSessionId", "(I)V", (void *)android_media_MediaPlayer_set_audio_session_id}, diff --git a/media/tests/MediaFrameworkTest/src/com/android/mediaframeworktest/unit/MediaPlayerUnitTest.java b/media/tests/MediaFrameworkTest/src/com/android/mediaframeworktest/unit/MediaPlayerUnitTest.java index f48056633e4eb..f812d5fa1c208 100644 --- a/media/tests/MediaFrameworkTest/src/com/android/mediaframeworktest/unit/MediaPlayerUnitTest.java +++ b/media/tests/MediaFrameworkTest/src/com/android/mediaframeworktest/unit/MediaPlayerUnitTest.java @@ -23,6 +23,7 @@ import static android.media.AudioManager.AUDIO_SESSION_ID_GENERATE; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertNotEquals; +import static org.junit.Assert.assertTrue; import static org.mockito.ArgumentMatchers.anyInt; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.when; @@ -56,6 +57,7 @@ public class MediaPlayerUnitTest { MediaPlayer mediaPlayer = new MediaPlayer(virtualDeviceContext); assertNotEquals(vdmPlaybackSessionId, mediaPlayer.getAudioSessionId()); + assertTrue(mediaPlayer.getAudioSessionId() > 0); } @Test