diff --git a/RELEASENOTES.md b/RELEASENOTES.md index 0ee60e5cba..6b5e6bf6d6 100644 --- a/RELEASENOTES.md +++ b/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 diff --git a/libraries/ui_compose/src/main/java/androidx/media3/ui/compose/PlayerSurface.kt b/libraries/ui_compose/src/main/java/androidx/media3/ui/compose/PlayerSurface.kt index a89b8c97b5..77d83d31a2 100644 --- a/libraries/ui_compose/src/main/java/androidx/media3/ui/compose/PlayerSurface.kt +++ b/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 PlayerSurfaceInternal( + player: Player, + modifier: Modifier, + createView: (Context) -> T, + setViewOnPlayer: (Player, T) -> Unit, + clearViewFromPlayer: (Player, T) -> Unit, +) { + var view by remember { mutableStateOf(null) } + var registeredPlayer by remember { mutableStateOf(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]. diff --git a/libraries/ui_compose/src/test/java/androidx/media3/ui/compose/PlayerSurfaceTest.kt b/libraries/ui_compose/src/test/java/androidx/media3/ui/compose/PlayerSurfaceTest.kt index 81ed7a4c83..efe319f59b 100644 --- a/libraries/ui_compose/src/test/java/androidx/media3/ui/compose/PlayerSurfaceTest.kt +++ b/libraries/ui_compose/src/test/java/androidx/media3/ui/compose/PlayerSurfaceTest.kt @@ -21,6 +21,7 @@ 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 @@ -28,6 +29,9 @@ 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()) + } }