Repository: Automattic/pocket-casts-android
File: modules/features/endofyear/src/main/java/au/com/shiftyjelly/pocketcasts/endofyear/ui/components/VideoSurface.kt (Kotlin)

Code before:
import androidx.media3.common.MediaItem
import androidx.media3.common.Player
import androidx.media3.common.Player.REPEAT_MODE_ALL
import androidx.media3.common.Player.RepeatMode
import androidx.media3.exoplayer.ExoPlayer
import androidx.media3.ui.compose.PlayerSurface
import androidx.media3.ui.compose.SURFACE_TYPE_TEXTURE_VIEW
import androidx.media3.ui.compose.state.rememberPresentationState

@Composable
fun VideoSurface(
    @RawRes videoResourceId: Int,
    modifier: Modifier = Modifier,
    backgroundColor: Color = Color.Black,
    @RepeatMode repeat: Int = REPEAT_MODE_ALL,
    play: Boolean = true,
    videoRatio: Float? = null,
) {
    val context = LocalContext.current
    var player by remember { mutableStateOf<Player?>(null) }
    val presentationState = rememberPresentationState(player)

    LifecycleStartEffect(Unit) {
        val uri = Uri.Builder()
            .scheme(ContentResolver.SCHEME_ANDROID_RESOURCE)
            .authority(context.packageName)
            .path(videoResourceId.toString())
            .build()
        player = ExoPlayer.Builder(context).build().apply {
            setMediaItem(MediaItem.fromUri(uri))
            repeatMode = repeat
            prepare()
            playWhenReady = play
        }
        onStopOrDispose {
            player?.apply { release() }
            player = null
        }
    }

    Box(
        contentAlignment = Alignment.Center,
        modifier = modifier,
    ) {
        player?.let {
            PlayerSurface(
                player = it,
                // use TextureView instead of SurfaceView to avoid rendering issues on Android 24
                surfaceType = SURFACE_TYPE_TEXTURE_VIEW,
                modifier = videoRatio?.let { ratio -> Modifier.aspectRatio(ratio) } ?: Modifier,
            )

Review comment (refactor):
⚠️ This doesn't look correct. It will recreate `ExoPlayer` on every stop/start cycle. `ExoPlayer` should be created once and cleaned up before destroying. On stop/start we should just pause and resume playback of the media item.

Another issue is that `VideoSurface` looks like a component that someone might move to generic components in the future, but it abstracts `ExoPlayer` too much. `ExoPlayer` for video isn't meant to be used like that, as [it is limited by hardware resources](https://github.com/androidx/media/issues/1880). Just like we don't create a new instance of `ExoPlayer` for each audio file we play, we shouldn't create a new `ExoPlayer` for each video file we play.

After looking at the `PlusInterstitialStory` I understand that we need to have two `ExoPlayer` instances for the background video.

I suggest to change this component to `internal` and document why it shouldn't be used for general purposes.

Code after:
import androidx.media3.common.Player.REPEAT_MODE_ALL
import androidx.media3.common.Player.RepeatMode
import androidx.media3.common.util.UnstableApi
import androidx.media3.exoplayer.ExoPlayer
import androidx.media3.ui.compose.PlayerSurface
import androidx.media3.ui.compose.SURFACE_TYPE_TEXTURE_VIEW
import androidx.media3.ui.compose.state.rememberPresentationState

/**
 * Only use for end of year videos and not podcast videos as using the ExoPlayer in this way isn't optimal.
 */
@UnstableApi
@Composable
internal fun VideoSurface(
    @RawRes videoResourceId: Int,
    modifier: Modifier = Modifier,
    backgroundColor: Color = Color.Black,
    @RepeatMode repeat: Int = REPEAT_MODE_ALL,
    play: Boolean = true,
    videoRatio: Float? = null,
) {
    val context = LocalContext.current
    val player = remember { mutableStateOf<Player?>(null) }
    val presentationState = rememberPresentationState(player.value)

    LifecycleStartEffect(Unit) {
        val uri = Uri.Builder()
            .scheme(ContentResolver.SCHEME_ANDROID_RESOURCE)
            .authority(context.packageName)
            .path(videoResourceId.toString())
            .build()
        player.value = ExoPlayer.Builder(context).build().apply {
            setMediaItem(MediaItem.fromUri(uri))
            repeatMode = repeat
            prepare()
            playWhenReady = play
        }
        onStopOrDispose {
            player.value?.apply { release() }
            player.value = null
        }
    }

    Box(
        contentAlignment = Alignment.Center,
        modifier = modifier,
    ) {
        player.value?.let {
            PlayerSurface(
                player = it,
                // use TextureView instead of SurfaceView to avoid rendering issues on Android 24