From 824fba8cab5b7884c97cd93822761b0b3d868223 Mon Sep 17 00:00:00 2001 From: Ben Salcie Date: Tue, 1 Jul 2025 11:16:01 +0300 Subject: [PATCH 1/6] refactor: enhance player architecture and fix background playback issue --- .../screens/videoPlayer/VideoPlayerManager.kt | 56 +++++++++++++++++++ .../screens/videoPlayer/VideoPlayerScreen.kt | 46 ++++++++------- .../videoPlayer/VideoPlayerScreenViewModel.kt | 41 +++++++++++--- 3 files changed, 112 insertions(+), 31 deletions(-) create mode 100644 JetStreamCompose/jetstream/src/main/java/com/google/jetstream/presentation/screens/videoPlayer/VideoPlayerManager.kt diff --git a/JetStreamCompose/jetstream/src/main/java/com/google/jetstream/presentation/screens/videoPlayer/VideoPlayerManager.kt b/JetStreamCompose/jetstream/src/main/java/com/google/jetstream/presentation/screens/videoPlayer/VideoPlayerManager.kt new file mode 100644 index 000000000..afcffc12d --- /dev/null +++ b/JetStreamCompose/jetstream/src/main/java/com/google/jetstream/presentation/screens/videoPlayer/VideoPlayerManager.kt @@ -0,0 +1,56 @@ +package com.google.jetstream.presentation.screens.videoPlayer + +import android.content.Context +import androidx.media3.common.C +import androidx.media3.common.Player +import androidx.media3.common.util.UnstableApi +import androidx.media3.datasource.DefaultDataSource +import androidx.media3.exoplayer.ExoPlayer +import androidx.media3.exoplayer.source.ProgressiveMediaSource +import com.google.jetstream.data.entities.MovieDetails +import dagger.hilt.android.qualifiers.ApplicationContext +import javax.inject.Inject + +@UnstableApi +// Do not make this a singleton to prevent Released player from Being invoked for play + +/** + * A manager for the video player. + * This class is responsible for managing the video playback using the ExoPlayer library. + * + * @param VideoPlayerManager Creates an Instance of the player and it available across app for Re-use. + */ +class VideoPlayerManager @Inject constructor( + @ApplicationContext private val context: Context +) { + private var _exoPlayer: ExoPlayer? = ExoPlayer.Builder(context) + .setSeekForwardIncrementMs(10) + .setSeekBackIncrementMs(10) + .setMediaSourceFactory( + ProgressiveMediaSource.Factory(DefaultDataSource.Factory(context)) + ) + .setVideoScalingMode(C.VIDEO_SCALING_MODE_SCALE_TO_FIT_WITH_CROPPING) + .build().apply { + playWhenReady = true + repeatMode = Player.REPEAT_MODE_OFF + } + + val player: ExoPlayer + get() = _exoPlayer ?: throw IllegalStateException("Player has been released") + + fun load(movieDetails: MovieDetails) { + player.apply { + stop() + clearMediaItems() + addMediaItem(movieDetails.intoMediaItem()) + movieDetails.similarMovies.forEach { addMediaItem(it.intoMediaItem()) } + prepare() + } + } + + fun release() { + _exoPlayer?.release() + _exoPlayer = null + } +} + diff --git a/JetStreamCompose/jetstream/src/main/java/com/google/jetstream/presentation/screens/videoPlayer/VideoPlayerScreen.kt b/JetStreamCompose/jetstream/src/main/java/com/google/jetstream/presentation/screens/videoPlayer/VideoPlayerScreen.kt index b9bcc6955..34eec8bbe 100644 --- a/JetStreamCompose/jetstream/src/main/java/com/google/jetstream/presentation/screens/videoPlayer/VideoPlayerScreen.kt +++ b/JetStreamCompose/jetstream/src/main/java/com/google/jetstream/presentation/screens/videoPlayer/VideoPlayerScreen.kt @@ -18,6 +18,7 @@ package com.google.jetstream.presentation.screens.videoPlayer import android.net.Uri import androidx.activity.compose.BackHandler +import androidx.annotation.OptIn import androidx.compose.foundation.focusable import androidx.compose.foundation.layout.Box import androidx.compose.foundation.layout.fillMaxSize @@ -50,10 +51,10 @@ import com.google.jetstream.presentation.screens.videoPlayer.components.VideoPla import com.google.jetstream.presentation.screens.videoPlayer.components.VideoPlayerPulse.Type.FORWARD import com.google.jetstream.presentation.screens.videoPlayer.components.VideoPlayerPulseState import com.google.jetstream.presentation.screens.videoPlayer.components.VideoPlayerState -import com.google.jetstream.presentation.screens.videoPlayer.components.rememberPlayer import com.google.jetstream.presentation.screens.videoPlayer.components.rememberVideoPlayerPulseState import com.google.jetstream.presentation.screens.videoPlayer.components.rememberVideoPlayerState import com.google.jetstream.presentation.utils.handleDPadKeyEvents +import androidx.core.net.toUri object VideoPlayerScreen { const val MovieIdBundleKey = "movieId" @@ -63,40 +64,37 @@ object VideoPlayerScreen { * [Work in progress] A composable screen for playing a video. * * @param onBackPressed The callback to invoke when the user presses the back button. - * @param videoPlayerScreenViewModel The view model for the video player screen. + * @param VideoPlayerScreenViewModel The view model for the video player screen. */ +@OptIn(UnstableApi::class) @Composable fun VideoPlayerScreen( onBackPressed: () -> Unit, - videoPlayerScreenViewModel: VideoPlayerScreenViewModel = hiltViewModel() + viewModel: VideoPlayerScreenViewModel = hiltViewModel() ) { - val uiState by videoPlayerScreenViewModel.uiState.collectAsStateWithLifecycle() - - // TODO: Handle Loading & Error states - when (val s = uiState) { - is VideoPlayerScreenUiState.Loading -> { - Loading(modifier = Modifier.fillMaxSize()) - } - - is VideoPlayerScreenUiState.Error -> { - Error(modifier = Modifier.fillMaxSize()) - } + val uiState by viewModel.uiState.collectAsStateWithLifecycle() + when (val state = uiState) { + is VideoPlayerScreenUiState.Loading -> Loading(modifier = Modifier.fillMaxSize()) + is VideoPlayerScreenUiState.Error -> Error(modifier = Modifier.fillMaxSize()) is VideoPlayerScreenUiState.Done -> { VideoPlayerScreenContent( - movieDetails = s.movieDetails, + movieDetails = state.movieDetails, + exoPlayer = viewModel.player, onBackPressed = onBackPressed ) } } } -@androidx.annotation.OptIn(UnstableApi::class) -@Composable -fun VideoPlayerScreenContent(movieDetails: MovieDetails, onBackPressed: () -> Unit) { - val context = LocalContext.current - val exoPlayer = rememberPlayer(context) +@OptIn(UnstableApi::class) +@Composable +fun VideoPlayerScreenContent( + movieDetails: MovieDetails, + onBackPressed: () -> Unit, + exoPlayer: ExoPlayer +) { val videoPlayerState = rememberVideoPlayerState( hideSeconds = 4, ) @@ -177,7 +175,7 @@ private fun Modifier.dPadEvents( } ) -private fun MovieDetails.intoMediaItem(): MediaItem { +fun MovieDetails.intoMediaItem(): MediaItem { return MediaItem.Builder() .setUri(videoUri) .setSubtitleConfigurations( @@ -186,7 +184,7 @@ private fun MovieDetails.intoMediaItem(): MediaItem { } else { listOf( MediaItem.SubtitleConfiguration - .Builder(Uri.parse(subtitleUri)) + .Builder(subtitleUri.toUri()) .setMimeType("application/vtt") .setLanguage("en") .setSelectionFlags(C.SELECTION_FLAG_DEFAULT) @@ -196,7 +194,7 @@ private fun MovieDetails.intoMediaItem(): MediaItem { ).build() } -private fun Movie.intoMediaItem(): MediaItem { +fun Movie.intoMediaItem(): MediaItem { return MediaItem.Builder() .setUri(videoUri) .setSubtitleConfigurations( @@ -205,7 +203,7 @@ private fun Movie.intoMediaItem(): MediaItem { } else { listOf( MediaItem.SubtitleConfiguration - .Builder(Uri.parse(subtitleUri)) + .Builder(subtitleUri.toUri()) .setMimeType("application/vtt") .setLanguage("en") .setSelectionFlags(C.SELECTION_FLAG_DEFAULT) diff --git a/JetStreamCompose/jetstream/src/main/java/com/google/jetstream/presentation/screens/videoPlayer/VideoPlayerScreenViewModel.kt b/JetStreamCompose/jetstream/src/main/java/com/google/jetstream/presentation/screens/videoPlayer/VideoPlayerScreenViewModel.kt index 3eb778be6..e572602b9 100644 --- a/JetStreamCompose/jetstream/src/main/java/com/google/jetstream/presentation/screens/videoPlayer/VideoPlayerScreenViewModel.kt +++ b/JetStreamCompose/jetstream/src/main/java/com/google/jetstream/presentation/screens/videoPlayer/VideoPlayerScreenViewModel.kt @@ -16,39 +16,66 @@ package com.google.jetstream.presentation.screens.videoPlayer +import androidx.annotation.OptIn import androidx.compose.runtime.Immutable import androidx.lifecycle.SavedStateHandle import androidx.lifecycle.ViewModel import androidx.lifecycle.viewModelScope +import androidx.media3.common.util.UnstableApi +import androidx.media3.exoplayer.ExoPlayer import com.google.jetstream.data.entities.MovieDetails import com.google.jetstream.data.repositories.MovieRepository import dagger.hilt.android.lifecycle.HiltViewModel import javax.inject.Inject import kotlinx.coroutines.flow.SharingStarted +import kotlinx.coroutines.flow.StateFlow import kotlinx.coroutines.flow.map import kotlinx.coroutines.flow.stateIn +@UnstableApi @HiltViewModel -class VideoPlayerScreenViewModel @Inject constructor( +@OptIn(UnstableApi::class) +class VideoPlayerScreenViewModel +@Inject constructor( savedStateHandle: SavedStateHandle, - repository: MovieRepository, + private val repository: MovieRepository, + private val playerManager: VideoPlayerManager ) : ViewModel() { - val uiState = savedStateHandle - .getStateFlow(VideoPlayerScreen.MovieIdBundleKey, null) + + private val movieIdFlow = savedStateHandle.getStateFlow( + VideoPlayerScreen.MovieIdBundleKey, + null + ) + + val uiState: StateFlow = movieIdFlow .map { id -> if (id == null) { VideoPlayerScreenUiState.Error } else { - val details = repository.getMovieDetails(movieId = id) - VideoPlayerScreenUiState.Done(movieDetails = details) + try { + val details = repository.getMovieDetails(id) + playerManager.load(details) + VideoPlayerScreenUiState.Done(details) + } catch (e: Exception) { + VideoPlayerScreenUiState.Error + } } - }.stateIn( + } + .stateIn( scope = viewModelScope, started = SharingStarted.WhileSubscribed(5_000), initialValue = VideoPlayerScreenUiState.Loading ) + + val player: ExoPlayer get() = playerManager.player + + override fun onCleared() { + super.onCleared() + playerManager.release() + } } + @Immutable sealed class VideoPlayerScreenUiState { data object Loading : VideoPlayerScreenUiState() From 4578d093b1c7da928562df8e0a1ed25dbbf30f53 Mon Sep 17 00:00:00 2001 From: Ben Salcie Date: Tue, 1 Jul 2025 11:29:16 +0300 Subject: [PATCH 2/6] Code cleanup and Formatting --- .../presentation/screens/videoPlayer/VideoPlayerManager.kt | 3 ++- .../screens/videoPlayer/VideoPlayerScreenViewModel.kt | 4 ++++ 2 files changed, 6 insertions(+), 1 deletion(-) diff --git a/JetStreamCompose/jetstream/src/main/java/com/google/jetstream/presentation/screens/videoPlayer/VideoPlayerManager.kt b/JetStreamCompose/jetstream/src/main/java/com/google/jetstream/presentation/screens/videoPlayer/VideoPlayerManager.kt index afcffc12d..29f9d54e8 100644 --- a/JetStreamCompose/jetstream/src/main/java/com/google/jetstream/presentation/screens/videoPlayer/VideoPlayerManager.kt +++ b/JetStreamCompose/jetstream/src/main/java/com/google/jetstream/presentation/screens/videoPlayer/VideoPlayerManager.kt @@ -18,7 +18,8 @@ import javax.inject.Inject * A manager for the video player. * This class is responsible for managing the video playback using the ExoPlayer library. * - * @param VideoPlayerManager Creates an Instance of the player and it available across app for Re-use. + * @param VideoPlayerManager Creates an Instance of the player and it available across app for Re-use + * @property release Releases the player to prevent memory leaks */ class VideoPlayerManager @Inject constructor( @ApplicationContext private val context: Context diff --git a/JetStreamCompose/jetstream/src/main/java/com/google/jetstream/presentation/screens/videoPlayer/VideoPlayerScreenViewModel.kt b/JetStreamCompose/jetstream/src/main/java/com/google/jetstream/presentation/screens/videoPlayer/VideoPlayerScreenViewModel.kt index e572602b9..d6b1f69a4 100644 --- a/JetStreamCompose/jetstream/src/main/java/com/google/jetstream/presentation/screens/videoPlayer/VideoPlayerScreenViewModel.kt +++ b/JetStreamCompose/jetstream/src/main/java/com/google/jetstream/presentation/screens/videoPlayer/VideoPlayerScreenViewModel.kt @@ -32,6 +32,10 @@ import kotlinx.coroutines.flow.StateFlow import kotlinx.coroutines.flow.map import kotlinx.coroutines.flow.stateIn + +/** + * A [VideoPlayerScreenViewModel] for the [VideoPlayerScreen] + */ @UnstableApi @HiltViewModel @OptIn(UnstableApi::class) From 9218130d36bb88e92bd2f3accc146b794db9fc61 Mon Sep 17 00:00:00 2001 From: Ben Salcie Date: Tue, 1 Jul 2025 11:31:44 +0300 Subject: [PATCH 3/6] Fixes Invalid Param doc --- .../presentation/screens/videoPlayer/VideoPlayerManager.kt | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/JetStreamCompose/jetstream/src/main/java/com/google/jetstream/presentation/screens/videoPlayer/VideoPlayerManager.kt b/JetStreamCompose/jetstream/src/main/java/com/google/jetstream/presentation/screens/videoPlayer/VideoPlayerManager.kt index 29f9d54e8..30f75486f 100644 --- a/JetStreamCompose/jetstream/src/main/java/com/google/jetstream/presentation/screens/videoPlayer/VideoPlayerManager.kt +++ b/JetStreamCompose/jetstream/src/main/java/com/google/jetstream/presentation/screens/videoPlayer/VideoPlayerManager.kt @@ -18,8 +18,7 @@ import javax.inject.Inject * A manager for the video player. * This class is responsible for managing the video playback using the ExoPlayer library. * - * @param VideoPlayerManager Creates an Instance of the player and it available across app for Re-use - * @property release Releases the player to prevent memory leaks + * @param context The application context, used to create the ExoPlayer instance. */ class VideoPlayerManager @Inject constructor( @ApplicationContext private val context: Context From 18c3e2922cf7d4dfe51b8a6696fc781db1469e99 Mon Sep 17 00:00:00 2001 From: Ben Salcie Date: Tue, 1 Jul 2025 11:33:19 +0300 Subject: [PATCH 4/6] Refactors Ideal Seek Milliseconds for practical user controls --- .../presentation/screens/videoPlayer/VideoPlayerManager.kt | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/JetStreamCompose/jetstream/src/main/java/com/google/jetstream/presentation/screens/videoPlayer/VideoPlayerManager.kt b/JetStreamCompose/jetstream/src/main/java/com/google/jetstream/presentation/screens/videoPlayer/VideoPlayerManager.kt index 30f75486f..c694ef98d 100644 --- a/JetStreamCompose/jetstream/src/main/java/com/google/jetstream/presentation/screens/videoPlayer/VideoPlayerManager.kt +++ b/JetStreamCompose/jetstream/src/main/java/com/google/jetstream/presentation/screens/videoPlayer/VideoPlayerManager.kt @@ -24,8 +24,8 @@ class VideoPlayerManager @Inject constructor( @ApplicationContext private val context: Context ) { private var _exoPlayer: ExoPlayer? = ExoPlayer.Builder(context) - .setSeekForwardIncrementMs(10) - .setSeekBackIncrementMs(10) + .setSeekForwardIncrementMs(10000) + .setSeekBackIncrementMs(10000) .setMediaSourceFactory( ProgressiveMediaSource.Factory(DefaultDataSource.Factory(context)) ) From 523d751a80cdb0c2bb7d379dbffb5fc7b0e9a16a Mon Sep 17 00:00:00 2001 From: Ben Salcie Date: Tue, 1 Jul 2025 11:35:50 +0300 Subject: [PATCH 5/6] Fixes Cases of Unhandled CancellationException --- .../screens/videoPlayer/VideoPlayerScreenViewModel.kt | 3 +++ 1 file changed, 3 insertions(+) diff --git a/JetStreamCompose/jetstream/src/main/java/com/google/jetstream/presentation/screens/videoPlayer/VideoPlayerScreenViewModel.kt b/JetStreamCompose/jetstream/src/main/java/com/google/jetstream/presentation/screens/videoPlayer/VideoPlayerScreenViewModel.kt index d6b1f69a4..46f494e67 100644 --- a/JetStreamCompose/jetstream/src/main/java/com/google/jetstream/presentation/screens/videoPlayer/VideoPlayerScreenViewModel.kt +++ b/JetStreamCompose/jetstream/src/main/java/com/google/jetstream/presentation/screens/videoPlayer/VideoPlayerScreenViewModel.kt @@ -56,10 +56,13 @@ class VideoPlayerScreenViewModel if (id == null) { VideoPlayerScreenUiState.Error } else { + try { val details = repository.getMovieDetails(id) playerManager.load(details) VideoPlayerScreenUiState.Done(details) + } catch (e: kotlinx.coroutines.CancellationException) { + throw e } catch (e: Exception) { VideoPlayerScreenUiState.Error } From 72b73d801d2537e97d12158e85f056b13e6952d9 Mon Sep 17 00:00:00 2001 From: Ben Salcie Date: Tue, 1 Jul 2025 11:52:43 +0300 Subject: [PATCH 6/6] Fixes Suggestion to Remove side effects for player manager loading tranformation --- .../videoPlayer/VideoPlayerScreenViewModel.kt | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/JetStreamCompose/jetstream/src/main/java/com/google/jetstream/presentation/screens/videoPlayer/VideoPlayerScreenViewModel.kt b/JetStreamCompose/jetstream/src/main/java/com/google/jetstream/presentation/screens/videoPlayer/VideoPlayerScreenViewModel.kt index 46f494e67..7caa03f0a 100644 --- a/JetStreamCompose/jetstream/src/main/java/com/google/jetstream/presentation/screens/videoPlayer/VideoPlayerScreenViewModel.kt +++ b/JetStreamCompose/jetstream/src/main/java/com/google/jetstream/presentation/screens/videoPlayer/VideoPlayerScreenViewModel.kt @@ -30,7 +30,9 @@ import javax.inject.Inject import kotlinx.coroutines.flow.SharingStarted import kotlinx.coroutines.flow.StateFlow import kotlinx.coroutines.flow.map +import kotlinx.coroutines.flow.onEach import kotlinx.coroutines.flow.stateIn +import kotlin.coroutines.cancellation.CancellationException /** @@ -56,24 +58,28 @@ class VideoPlayerScreenViewModel if (id == null) { VideoPlayerScreenUiState.Error } else { - try { val details = repository.getMovieDetails(id) - playerManager.load(details) VideoPlayerScreenUiState.Done(details) - } catch (e: kotlinx.coroutines.CancellationException) { + } catch (e: CancellationException) { throw e } catch (e: Exception) { VideoPlayerScreenUiState.Error } } } + .onEach { state -> + if (state is VideoPlayerScreenUiState.Done) { + playerManager.load(state.movieDetails) + } + } .stateIn( scope = viewModelScope, started = SharingStarted.WhileSubscribed(5_000), initialValue = VideoPlayerScreenUiState.Loading ) + val player: ExoPlayer get() = playerManager.player override fun onCleared() {