Fixes a crash during playback (#1514)

## Description
Fixes a crash that could occur during the beginning of playback

Also adds better handling for clicking next extremely fast

### Technical details

The playlist objects would be created on the IO thread and it had a
`mutableIntStateOf` to track the current index. This meant that there is
a race conditions for the IO thread to commit the snapshot for the main
thread to see them. If the main thread is faster, it will crash.

To fix that, this PR moves the mutable state in `Playlist` into the
`PlaybackState` flow in the view model. This enures both the background
threads and re-compositions have the right state.

### Related issues
Fixes #1509 

### Testing
Emulator

## Screenshots
N/A

## AI or LLM usage
None

app/src/main/java/com/github/damontecres/wholphin/data/model/Playlist.kt

@@ -1,8 +1,5 @@
 package com.github.damontecres.wholphin.data.model
 
-import androidx.compose.runtime.getValue
-import androidx.compose.runtime.mutableIntStateOf
-import androidx.compose.runtime.setValue
 import org.jellyfin.sdk.model.api.MediaType
 import java.util.UUID
 
@@ -11,36 +8,9 @@ import java.util.UUID
  *
  * This is not the same thing as a Jellyfin server playlist
  */
-class Playlist(
+data class Playlist(
-    items: List<PlaylistItem>,
+    val items: List<PlaylistItem>,
-    startIndex: Int = 0,
 ) {
-    val items = items.subList(startIndex, items.size)
-
-    var index by mutableIntStateOf(0)
-
-    fun hasPrevious(): Boolean = index > 0
-
-    fun hasNext(): Boolean = (index + 1) < items.size
-
-    fun getPreviousAndReverse(): PlaylistItem = items[--index]
-
-    fun getAndAdvance(): PlaylistItem = items[++index]
-
-    fun peek(): PlaylistItem? = items.getOrNull(index + 1)
-
-    fun upcomingItems(): List<PlaylistItem> = items.subList(index + 1, items.size)
-
-    fun advanceTo(id: UUID): PlaylistItem? {
-        while (hasNext()) {
-            val potential = getAndAdvance()
-            if (potential.id == id) {
-                return potential
-            }
-        }
-        return null
-    }
-
     companion object {
         const val MAX_SIZE = 100
     }

app/src/main/java/com/github/damontecres/wholphin/services/PlaylistCreator.kt

@@ -78,7 +78,7 @@ class PlaylistCreator
                     .convertAndAddParts(false)
             val startIndex =
                 episodeId?.let { episodes.indexOfFirstOrNull { it.id == episodeId } } ?: 0
-            return Playlist(episodes, startIndex)
+            return Playlist(episodes.subList(startIndex, episodes.size))
         }
 
         /**
@@ -104,7 +104,7 @@ class PlaylistCreator
                     ),
                 )
             val items = GetItemsRequestHandler.execute(api, request).content.items
-            return Playlist(items.convertAndAddParts(), 0)
+            return Playlist(items.convertAndAddParts())
         }
 
         private suspend fun createFromCollection(
@@ -147,7 +147,7 @@ class PlaylistCreator
                     .execute(api, request)
                     .content.items
                     .convertAndAddParts()
-            return Playlist(items, 0)
+            return Playlist(items)
         }
 
         /**
@@ -263,7 +263,7 @@ class PlaylistCreator
                                     }.let(::addAll)
                             }
                         }
-                    PlaylistCreationResult.Success(Playlist(list, 0))
+                    PlaylistCreationResult.Success(Playlist(list))
                 }
 
                 // Not support yet

app/src/main/java/com/github/damontecres/wholphin/ui/playback/PlaybackPage.kt

@@ -287,7 +287,7 @@ fun PlaybackPageContent(
 
             PlaybackAction.Previous -> {
                 val pos = player.currentPosition
-                if (pos < player.maxSeekToPreviousPosition && state.playlist.hasPrevious()) {
+                if (pos < player.maxSeekToPreviousPosition && state.hasPrevious) {
                     viewModel.playPrevious()
                 } else {
                     player.seekToPrevious()
@@ -401,7 +401,7 @@ fun PlaybackPageContent(
                 controllerViewState = controllerViewState,
                 showPlay = playPauseState.showPlay,
                 previousEnabled = true,
-                nextEnabled = state.playlist.hasNext(),
+                nextEnabled = state.hasNext,
                 seekEnabled = true,
                 seekForward = preferences.appPreferences.playbackPreferences.skipForwardMs.milliseconds,
                 seekBack = preferences.appPreferences.playbackPreferences.skipBackMs.milliseconds,
@@ -414,10 +414,8 @@ fun PlaybackPageContent(
                 chapters = state.currentMediaInfo.chapters,
                 trickplayInfo = state.currentMediaInfo.trickPlayInfo,
                 trickplayUrlFor = viewModel::getTrickplayUrl,
-                playlist = state.playlist,
+                queue = remember(state.playlistIndex, state.playlist) { state.upcomingItems },
-                onClickPlaylist = {
+                onClickPlaylist = viewModel::playItemInPlaylist,
-                    viewModel.playItemInPlaylist(it)
-                },
                 currentSegment = state.currentSegment?.segment,
                 showClock = preferences.appPreferences.interfacePreferences.showClock,
                 analyticsState = state.analyticsState,

app/src/main/java/com/github/damontecres/wholphin/ui/playback/PlaybackState.kt

@@ -6,6 +6,7 @@ import androidx.media3.common.text.Cue
 import com.github.damontecres.wholphin.data.model.BaseItem
 import com.github.damontecres.wholphin.data.model.ItemPlayback
 import com.github.damontecres.wholphin.data.model.Playlist
+import com.github.damontecres.wholphin.data.model.PlaylistItem
 import com.github.damontecres.wholphin.preferences.PlayerBackend
 import com.github.damontecres.wholphin.ui.formatBitrate
 import com.github.damontecres.wholphin.util.LoadingState
@@ -21,8 +22,20 @@ data class PlaybackState(
     val analyticsState: AnalyticsState = AnalyticsState(),
     val subtitleCues: List<Cue> = emptyList(),
     val nextUp: BaseItem? = null,
-    val playlist: Playlist = Playlist(listOf()),
+    val playlistIndex: Int = 0,
-)
+    val playlist: Playlist = Playlist(emptyList()),
+) {
+    val hasNext: Boolean get() = (playlistIndex + 1) < playlist.items.size
+    val hasPrevious: Boolean get() = playlistIndex > 0
+    val upcomingItems: List<PlaylistItem>
+        get() =
+            playlist.items.subList(
+                (playlistIndex + 1).coerceAtMost(playlist.items.size),
+                playlist.items.size,
+            )
+
+    fun nextItem(): PlaylistItem? = playlist.items.getOrNull(playlistIndex + 1)
+}
 
 data class PlayerInstance(
     val player: Player,

app/src/main/java/com/github/damontecres/wholphin/ui/playback/PlaybackViewModel.kt

@@ -92,6 +92,7 @@ import kotlinx.coroutines.flow.update
 import kotlinx.coroutines.isActive
 import kotlinx.coroutines.launch
 import kotlinx.coroutines.sync.Mutex
+import kotlinx.coroutines.sync.withLock
 import kotlinx.coroutines.withContext
 import org.jellyfin.sdk.api.client.ApiClient
 import org.jellyfin.sdk.api.client.extensions.mediaInfoApi
@@ -164,7 +165,10 @@ class PlaybackViewModel
         internal lateinit var player: Player
 
         private var mediaSession: MediaSession? = null
-        internal val mutex = Mutex()
+
+        // Mutex & Job for changing playlist index
+        private val playlistMutex = Mutex()
+        private var playlistJob: Job? = null
 
         val controllerViewState =
             ControllerViewState(
@@ -345,7 +349,6 @@ class PlaybackViewModel
 
             viewModelScope.launch(ExceptionHandler()) { controllerViewState.observe() }
 
-            playlistItem.item.type == BaseItemKind.TRAILER
             val intros =
                 // If not resuming playback & cinema mode is enabled, get potential intros
                 if (positionMs == 0L && preferences.appPreferences.playbackPreferences.cinemaMode) {
@@ -1017,7 +1020,7 @@ class PlaybackViewModel
             if (playbackState == Player.STATE_ENDED) {
                 Timber.v("Playback state is STATE_ENDED")
                 viewModelScope.launchDefault {
-                    when (val nextItem = state.value.playlist.peek()) {
+                    when (val nextItem = state.value.nextItem()) {
                         is PlaylistItem.Intro -> {
                             Timber.v("Next item is intro, so playing immediately")
                             playNextUp()
@@ -1093,14 +1096,14 @@ class PlaybackViewModel
                                         currentSegment.type,
                                     )
                                 }
-                                val playlist = state.value.playlist
+                                val state = state.value
 
                                 if (currentSegment.type == MediaSegmentType.OUTRO &&
                                     prefs.showNextUpWhen == ShowNextUpWhen.DURING_CREDITS &&
-                                    playlist.hasNext() &&
+                                    state.hasNext &&
                                     outroShownSegments.add(currentSegment.id)
                                 ) {
-                                    val nextItem = playlist.peek()
+                                    val nextItem = state.nextItem()
                                     if (nextItem is PlaylistItem.Media) {
                                         Timber.v("Setting next up during outro to ${nextItem?.id}")
                                         _state.update { it.copy(nextUp = nextItem.item) }
@@ -1218,30 +1221,50 @@ class PlaybackViewModel
             }
 
         fun playNextUp() {
-            state.value.playlist.let {
+            viewModelScope.launchDefault {
-                if (it.hasNext()) {
+                playlistMutex.withLock {
-                    viewModelScope.launchDefault {
+                    val state = state.value
+                    if (state.hasNext) {
                         cancelUpNextEpisode()
-                        val item = it.getAndAdvance()
+                        val nextIndex = state.playlistIndex + 1
-                        val played = play(item, 0)
+                        val item = state.playlist.items[nextIndex]
-                        if (!played) {
+                        _state.update { it.copy(playlistIndex = nextIndex) }
-                            playNextUp()
+                        playlistJob?.cancel()
-                        }
+                        playlistJob =
+                            viewModelScope.launchDefault {
+                                val played = play(item, 0)
+                                if (!played) {
+                                    playNextUp()
+                                }
+                            }
+                    } else {
+                        Timber.w("Attempting to play next, but there are no more items")
+                        return@launchDefault
                     }
                 }
             }
         }
 
         fun playPrevious() {
-            state.value.playlist.let {
+            viewModelScope.launchDefault {
-                if (it.hasPrevious()) {
+                playlistMutex.withLock {
-                    viewModelScope.launchDefault {
+                    val state = state.value
+                    if (state.hasPrevious) {
                         cancelUpNextEpisode()
-                        val item = it.getPreviousAndReverse()
+                        val previousIndex = state.playlistIndex - 1
-                        val played = play(item, 0)
+                        val item = state.playlist.items[previousIndex]
-                        if (!played) {
+                        _state.update { it.copy(playlistIndex = previousIndex) }
-                            playPrevious()
+                        playlistJob?.cancel()
-                        }
+                        playlistJob =
+                            viewModelScope.launchDefault {
+                                val played = play(item, 0)
+                                if (!played) {
+                                    playPrevious()
+                                }
+                            }
+                    } else {
+                        Timber.w("Attempting to play next, but there are no more items")
+                        return@launchDefault
                     }
                 }
             }
@@ -1252,16 +1275,24 @@ class PlaybackViewModel
         }
 
         fun playItemInPlaylist(item: BaseItem) {
-            state.value.playlist.let { playlist ->
+            viewModelScope.launchDefault {
-                viewModelScope.launchIO {
+                playlistMutex.withLock {
-                    val toPlay = playlist.advanceTo(item.id)
+                    val state = state.value
-                    if (toPlay != null) {
+                    val index = state.playlist.items.indexOfFirst { it.id == item.id }
-                        val played = play(toPlay, 0)
+                    if (index in state.playlist.items.indices) {
-                        if (!played) {
+                        val toPlay = state.playlist.items[index]
-                            playNextUp()
+                        _state.update { it.copy(playlistIndex = index) }
-                        }
+                        playlistJob?.cancel()
+                        playlistJob =
+                            viewModelScope.launchDefault {
+                                val played = play(toPlay, 0)
+                                if (!played) {
+                                    playNextUp()
+                                }
+                            }
                     } else {
-                        // TODO
+                        Timber.w("Item not found in playlist %s", item.id)
+                        return@launchDefault
                     }
                 }
             }

app/src/main/java/com/github/damontecres/wholphin/ui/playback/overlay/ChapterRowOverlay.kt

@@ -31,7 +31,6 @@ import androidx.tv.material3.MaterialTheme
 import androidx.tv.material3.Text
 import com.github.damontecres.wholphin.R
 import com.github.damontecres.wholphin.data.model.Chapter
-import com.github.damontecres.wholphin.data.model.Playlist
 import com.github.damontecres.wholphin.ui.cards.ChapterCard
 import com.github.damontecres.wholphin.ui.components.HiddenFocusBox
 import com.github.damontecres.wholphin.ui.ifElse
@@ -44,7 +43,7 @@ fun ChapterRowOverlay(
     player: Player,
     controllerViewState: ControllerViewState,
     chapters: List<Chapter>,
-    playlist: Playlist,
+    hasNext: Boolean,
     aspectRatio: Float,
     onChangeState: (OverlayViewState) -> Unit,
     modifier: Modifier = Modifier,
@@ -147,7 +146,7 @@ fun ChapterRowOverlay(
                 )
             }
         }
-        if (playlist.hasNext()) {
+        if (hasNext) {
             Text(
                 text = stringResource(R.string.queue),
                 style = MaterialTheme.typography.titleLarge,

app/src/main/java/com/github/damontecres/wholphin/ui/playback/overlay/PlaybackOverlay.kt

@@ -50,7 +50,7 @@ import androidx.tv.material3.Text
 import coil3.compose.AsyncImage
 import com.github.damontecres.wholphin.data.model.BaseItem
 import com.github.damontecres.wholphin.data.model.Chapter
-import com.github.damontecres.wholphin.data.model.Playlist
+import com.github.damontecres.wholphin.data.model.PlaylistItem
 import com.github.damontecres.wholphin.data.model.aspectRatioFloat
 import com.github.damontecres.wholphin.ui.AppColors
 import com.github.damontecres.wholphin.ui.AspectRatios
@@ -91,10 +91,10 @@ fun PlaybackOverlay(
     currentPlayback: CurrentPlayback?,
     currentSegment: MediaSegmentDto?,
     analyticsState: AnalyticsState,
+    queue: List<PlaylistItem>,
     modifier: Modifier = Modifier,
     trickplayInfo: TrickplayInfo? = null,
     trickplayUrlFor: (Int) -> String? = { null },
-    playlist: Playlist = Playlist(listOf(), 0),
     onClickPlaylist: (BaseItem) -> Unit = {},
     seekBarInteractionSource: MutableInteractionSource = remember { MutableInteractionSource() },
 ) {
@@ -180,10 +180,10 @@ fun PlaybackOverlay(
                         }
                     }
                     val nextState =
-                        remember(chapters, playlist) {
+                        remember(chapters, nextEnabled) {
                             if (chapters.isNotEmpty()) {
                                 OverlayViewState.CHAPTERS
-                            } else if (playlist.hasNext()) {
+                            } else if (nextEnabled) {
                                 OverlayViewState.QUEUE
                             } else {
                                 null
@@ -225,7 +225,7 @@ fun PlaybackOverlay(
                             player = player,
                             controllerViewState = controllerViewState,
                             chapters = chapters,
-                            playlist = playlist,
+                            hasNext = nextEnabled,
                             aspectRatio = item?.data?.aspectRatioFloat ?: AspectRatios.WIDE,
                             onChangeState = onChangeState,
                             modifier =
@@ -237,9 +237,9 @@ fun PlaybackOverlay(
                 }
 
                 OverlayViewState.QUEUE -> {
-                    if (playlist.hasNext()) {
+                    if (nextEnabled) {
                         QueueRowOverlay(
-                            playlist = playlist,
+                            queue = queue,
                             controllerViewState = controllerViewState,
                             nextState =
                                 remember(chapters) {

app/src/main/java/com/github/damontecres/wholphin/ui/playback/overlay/QueueRowOverlay.kt

@@ -22,7 +22,7 @@ import androidx.tv.material3.MaterialTheme
 import androidx.tv.material3.Text
 import com.github.damontecres.wholphin.R
 import com.github.damontecres.wholphin.data.model.BaseItem
-import com.github.damontecres.wholphin.data.model.Playlist
+import com.github.damontecres.wholphin.data.model.PlaylistItem
 import com.github.damontecres.wholphin.ui.cards.SeasonCard
 import com.github.damontecres.wholphin.ui.components.HiddenFocusBox
 import com.github.damontecres.wholphin.ui.ifElse
@@ -31,14 +31,13 @@ import com.github.damontecres.wholphin.ui.tryRequestFocus
 
 @Composable
 fun QueueRowOverlay(
-    playlist: Playlist,
+    queue: List<PlaylistItem>,
     controllerViewState: ControllerViewState,
     nextState: OverlayViewState,
     onChangeState: (OverlayViewState) -> Unit,
     onClickPlaylist: (BaseItem) -> Unit,
     modifier: Modifier = Modifier,
 ) {
-    val items = remember { playlist.upcomingItems() }
     val focusRequester = remember { FocusRequester() }
     val hiddenFocusRequester = remember { FocusRequester() }
     LaunchedEffect(Unit) { focusRequester.tryRequestFocus() }
@@ -70,7 +69,7 @@ fun QueueRowOverlay(
                         }
                     },
         ) {
-            itemsIndexed(items) { index, item ->
+            itemsIndexed(queue) { index, item ->
                 val interactionSource =
                     remember { MutableInteractionSource() }
                 val isFocused =