Clear surface from previous player when assigning a new player

The surface must only be used by one player at a time. To ensure
that, we can keep a reference to the previously used player
and clear its surface reference before assigning to a new one.

Note that we do not need to clear the surface in onDispose
of a DisposableEffect because the lifecycle management of the
surface is moved to the Player and the Player takes care of
unregistering its surface reference as soon as the surface is
destroyed (which happens when the AndroidView element is no longer
is the Composable tree).

PiperOrigin-RevId: 745558414
(cherry picked from commit f9617e1)

Author: tonihei

RELEASENOTES.md

@@ -65,6 +65,7 @@
 *   UI:
     *   Enable `PlayerSurface` to work with `ExoPlayer.setVideoEffects` and
         `CompositionPlayer`.
+    *   Fix bug where `PlayerSurface` can't be recomposed with a new `Player`.
 *   Downloads:
     *   Add partial download support for progressive streams. Apps can prepare a
         progressive stream with `DownloadHelper`, and request a

libraries/ui_compose/src/main/java/androidx/media3/ui/compose/PlayerSurface.kt

@@ -16,12 +16,17 @@
 
 package androidx.media3.ui.compose
 
+import android.content.Context
 import android.view.SurfaceView
 import android.view.TextureView
+import android.view.View
 import androidx.annotation.IntDef
 import androidx.compose.runtime.Composable
+import androidx.compose.runtime.LaunchedEffect
 import androidx.compose.runtime.getValue
-import androidx.compose.runtime.rememberUpdatedState
+import androidx.compose.runtime.mutableStateOf
+import androidx.compose.runtime.remember
+import androidx.compose.runtime.setValue
 import androidx.compose.ui.Modifier
 import androidx.compose.ui.viewinterop.AndroidView
 import androidx.media3.common.Player
@@ -47,37 +52,53 @@ fun PlayerSurface(
   modifier: Modifier = Modifier,
   surfaceType: @SurfaceType Int = SURFACE_TYPE_SURFACE_VIEW,
 ) {
-  // Player might change between compositions,
-  // we need long-lived surface-related lambdas to always use the latest value
-  val currentPlayer by rememberUpdatedState(player)
-
   when (surfaceType) {
     SURFACE_TYPE_SURFACE_VIEW ->
-      AndroidView(
-        factory = {
-          SurfaceView(it).apply {
-            if (currentPlayer.isCommandAvailable(Player.COMMAND_SET_VIDEO_SURFACE))
-              currentPlayer.setVideoSurfaceView(this)
-          }
-        },
-        onReset = {},
-        modifier = modifier,
+      PlayerSurfaceInternal(
+        player,
+        modifier,
+        createView = { SurfaceView(it) },
+        setViewOnPlayer = { player, view -> player.setVideoSurfaceView(view) },
+        clearViewFromPlayer = { player, view -> player.clearVideoSurfaceView(view) },
       )
     SURFACE_TYPE_TEXTURE_VIEW ->
-      AndroidView(
-        factory = {
-          TextureView(it).apply {
-            if (currentPlayer.isCommandAvailable(Player.COMMAND_SET_VIDEO_SURFACE))
-              currentPlayer.setVideoTextureView(this)
-          }
-        },
-        onReset = {},
-        modifier = modifier,
+      PlayerSurfaceInternal(
+        player,
+        modifier,
+        createView = { TextureView(it) },
+        setViewOnPlayer = { player, view -> player.setVideoTextureView(view) },
+        clearViewFromPlayer = { player, view -> player.clearVideoTextureView(view) },
       )
     else -> throw IllegalArgumentException("Unrecognized surface type: $surfaceType")
   }
 }
 
+@Composable
+private fun <T : View> PlayerSurfaceInternal(
+  player: Player,
+  modifier: Modifier,
+  createView: (Context) -> T,
+  setViewOnPlayer: (Player, T) -> Unit,
+  clearViewFromPlayer: (Player, T) -> Unit,
+) {
+  var view by remember { mutableStateOf<T?>(null) }
+  var registeredPlayer by remember { mutableStateOf<Player?>(null) }
+  AndroidView(factory = { createView(it).apply { view = this } }, onReset = {}, modifier = modifier)
+  view?.let { view ->
+    LaunchedEffect(view, player) {
+      registeredPlayer?.let { previousPlayer ->
+        if (previousPlayer.isCommandAvailable(Player.COMMAND_SET_VIDEO_SURFACE))
+          clearViewFromPlayer(previousPlayer, view)
+        registeredPlayer = null
+      }
+      if (player.isCommandAvailable(Player.COMMAND_SET_VIDEO_SURFACE)) {
+        setViewOnPlayer(player, view)
+        registeredPlayer = player
+      }
+    }
+  }
+}
+
 /**
  * The type of surface used for media playbacks. One of [SURFACE_TYPE_SURFACE_VIEW] or
  * [SURFACE_TYPE_TEXTURE_VIEW].

libraries/ui_compose/src/test/java/androidx/media3/ui/compose/PlayerSurfaceTest.kt

@@ -21,13 +21,17 @@ import androidx.compose.runtime.MutableIntState
 import androidx.compose.runtime.mutableIntStateOf
 import androidx.compose.runtime.remember
 import androidx.compose.ui.test.junit4.createComposeRule
+import androidx.media3.common.ForwardingPlayer
 import androidx.media3.common.Player
 import androidx.media3.ui.compose.utils.TestPlayer
 import androidx.test.ext.junit.runners.AndroidJUnit4
 import com.google.common.truth.Truth.assertThat
 import org.junit.Rule
 import org.junit.Test
 import org.junit.runner.RunWith
+import org.mockito.ArgumentMatchers.any
+import org.mockito.Mockito.inOrder
+import org.mockito.Mockito.spy
 
 /** Unit test for [PlayerSurface]. */
 @RunWith(AndroidJUnit4::class)
@@ -87,4 +91,30 @@ class PlayerSurfaceTest {
 
     assertThat(player.videoOutput).isInstanceOf(SurfaceView::class.java)
   }
+
+  @Test
+  fun playerSurface_withNewPlayer_unsetsSurfaceOnOldPlayerFirst() {
+    val player0 = TestPlayer()
+    val player1 = TestPlayer()
+    val spyPlayer0 = spy(ForwardingPlayer(player0))
+    val spyPlayer1 = spy(ForwardingPlayer(player1))
+
+    lateinit var playerIndex: MutableIntState
+    composeTestRule.setContent {
+      playerIndex = remember { mutableIntStateOf(0) }
+      PlayerSurface(
+        player = if (playerIndex.intValue == 0) spyPlayer0 else spyPlayer1,
+        surfaceType = SURFACE_TYPE_SURFACE_VIEW,
+      )
+    }
+    composeTestRule.waitForIdle()
+    playerIndex.intValue = 1
+    composeTestRule.waitForIdle()
+
+    assertThat(player0.videoOutput).isNull()
+    assertThat(player1.videoOutput).isNotNull()
+    val inOrder = inOrder(spyPlayer0, spyPlayer1)
+    inOrder.verify(spyPlayer0).clearVideoSurfaceView(any())
+    inOrder.verify(spyPlayer1).setVideoSurfaceView(any())
+  }
 }