From 02d6a98ba84f1ea5abc74147a0adfccf65e36d91 Mon Sep 17 00:00:00 2001 From: Ray <154766448+damontecres@users.noreply.github.com> Date: Sun, 4 Jan 2026 17:06:54 -0500 Subject: [PATCH] More subtitle selection fixes (#630) ## Description Fixes more subtitle selection issues, primarily fixes the sort order of subtitle streams. But also prefer default subtitle in some cases Adds test cases for the described situations below Also adds an option in the More dialog to clear track selections made on an item/series. ### Related issues Addresses https://github.com/damontecres/Wholphin/issues/570#issuecomment-3707255647 & https://github.com/damontecres/Wholphin/issues/570#issuecomment-3707292928 --- .../wholphin/data/ItemPlaybackDao.kt | 12 +- .../wholphin/data/ItemPlaybackRepository.kt | 13 ++ .../data/PlaybackLanguageChoiceDao.kt | 4 + .../wholphin/services/StreamChoiceService.kt | 24 +-- .../wholphin/ui/detail/DetailUtils.kt | 19 ++ .../ui/detail/episode/EpisodeDetails.kt | 4 + .../ui/detail/episode/EpisodeViewModel.kt | 15 ++ .../wholphin/ui/detail/movie/MovieDetails.kt | 4 + .../ui/detail/movie/MovieViewModel.kt | 15 ++ .../ui/detail/series/SeriesOverview.kt | 10 +- .../ui/detail/series/SeriesViewModel.kt | 10 + app/src/main/res/values/strings.xml | 1 + .../wholphin/test/TestStreamChoiceService.kt | 185 +++++++++++++++++- 13 files changed, 289 insertions(+), 27 deletions(-) diff --git a/app/src/main/java/com/github/damontecres/wholphin/data/ItemPlaybackDao.kt b/app/src/main/java/com/github/damontecres/wholphin/data/ItemPlaybackDao.kt index dedbb8a7..a3880d30 100644 --- a/app/src/main/java/com/github/damontecres/wholphin/data/ItemPlaybackDao.kt +++ b/app/src/main/java/com/github/damontecres/wholphin/data/ItemPlaybackDao.kt @@ -1,6 +1,7 @@ package com.github.damontecres.wholphin.data import androidx.room.Dao +import androidx.room.Delete import androidx.room.Insert import androidx.room.OnConflictStrategy import androidx.room.Query @@ -11,22 +12,25 @@ import java.util.UUID @Dao interface ItemPlaybackDao { - fun getItem( + suspend fun getItem( user: JellyfinUser, itemId: UUID, ): ItemPlayback? = getItem(user.rowId, itemId) @Query("SELECT * from ItemPlayback WHERE userId=:userId AND itemId=:itemId") - fun getItem( + suspend fun getItem( userId: Int, itemId: UUID, ): ItemPlayback? @Insert(onConflict = OnConflictStrategy.REPLACE) - fun saveItem(item: ItemPlayback): Long + suspend fun saveItem(item: ItemPlayback): Long + + @Delete + suspend fun deleteItem(item: ItemPlayback) @Query("SELECT * from ItemPlayback WHERE userId=:userId") - fun getItems(userId: Int): List + suspend fun getItems(userId: Int): List @Query("SELECT * FROM ItemTrackModification WHERE userId=:userId AND itemId=:itemId") suspend fun getTrackModifications( diff --git a/app/src/main/java/com/github/damontecres/wholphin/data/ItemPlaybackRepository.kt b/app/src/main/java/com/github/damontecres/wholphin/data/ItemPlaybackRepository.kt index 17ad9f0f..460d97b2 100644 --- a/app/src/main/java/com/github/damontecres/wholphin/data/ItemPlaybackRepository.kt +++ b/app/src/main/java/com/github/damontecres/wholphin/data/ItemPlaybackRepository.kt @@ -25,6 +25,7 @@ class ItemPlaybackRepository constructor( val serverRepository: ServerRepository, val itemPlaybackDao: ItemPlaybackDao, + private val playbackLanguageChoiceDao: PlaybackLanguageChoiceDao, private val streamChoiceService: StreamChoiceService, ) { suspend fun getSelectedTracks( @@ -204,6 +205,18 @@ class ItemPlaybackRepository ) } } + + suspend fun deleteChosenStreams(chosenStreams: ChosenStreams?) { + Timber.d("deleteChosenStreams: %s", chosenStreams) + chosenStreams?.plc?.let { + Timber.d("Deleting %s", it) + playbackLanguageChoiceDao.delete(it) + } + chosenStreams?.itemPlayback?.let { + Timber.d("Deleting %s", it) + itemPlaybackDao.deleteItem(it) + } + } } data class ChosenStreams( diff --git a/app/src/main/java/com/github/damontecres/wholphin/data/PlaybackLanguageChoiceDao.kt b/app/src/main/java/com/github/damontecres/wholphin/data/PlaybackLanguageChoiceDao.kt index e268a08c..5f8c16e6 100644 --- a/app/src/main/java/com/github/damontecres/wholphin/data/PlaybackLanguageChoiceDao.kt +++ b/app/src/main/java/com/github/damontecres/wholphin/data/PlaybackLanguageChoiceDao.kt @@ -1,6 +1,7 @@ package com.github.damontecres.wholphin.data import androidx.room.Dao +import androidx.room.Delete import androidx.room.Insert import androidx.room.OnConflictStrategy import androidx.room.Query @@ -17,4 +18,7 @@ interface PlaybackLanguageChoiceDao { @Insert(onConflict = OnConflictStrategy.REPLACE) suspend fun save(plc: PlaybackLanguageChoice): Long + + @Delete + fun delete(plc: PlaybackLanguageChoice) } diff --git a/app/src/main/java/com/github/damontecres/wholphin/services/StreamChoiceService.kt b/app/src/main/java/com/github/damontecres/wholphin/services/StreamChoiceService.kt index ada96c57..98708ead 100644 --- a/app/src/main/java/com/github/damontecres/wholphin/services/StreamChoiceService.kt +++ b/app/src/main/java/com/github/damontecres/wholphin/services/StreamChoiceService.kt @@ -199,15 +199,17 @@ class StreamChoiceService } } val candidates = - candidates.sortedWith( - compareBy( - { it.isDefault }, - { !it.isForced && it.language.equals(subtitleLanguage, true) }, - { it.isForced && it.language.equals(subtitleLanguage, true) }, - { it.isForced && it.language.isUnknown }, - { it.isForced }, - ), - ) + candidates + .sortedWith( + compareByDescending { it.isExternal } + .thenByDescending { it.isDefault } + .thenByDescending { + !it.isForced && it.language.equals(subtitleLanguage, true) + }.thenByDescending { + it.isForced && it.language.equals(subtitleLanguage, true) + }.thenByDescending { it.isForced && it.language.isUnknown } + .thenByDescending { it.isForced }, + ) return when (subtitleMode) { SubtitlePlaybackMode.ALWAYS -> { if (subtitleLanguage.isNotNullOrBlank()) { @@ -232,7 +234,7 @@ class StreamChoiceService SubtitlePlaybackMode.SMART -> { if (subtitleLanguage.isNotNullOrBlank()) { val audioLanguage = userConfig?.audioLanguagePreference - if (audioLanguage.isNotNullOrBlank() && audioLanguage != audioStreamLang) { + if (audioLanguage.isNullOrBlank() || audioLanguage != audioStreamLang) { candidates.firstOrNull { it.language == subtitleLanguage } ?: candidates.firstOrNull { it.language.isUnknown } } else { @@ -240,7 +242,7 @@ class StreamChoiceService ?: candidates.firstOrNull { it.isForced && it.language.isUnknown } } } else { - candidates.firstOrNull { it.isForced } + candidates.firstOrNull { it.isDefault } } } diff --git a/app/src/main/java/com/github/damontecres/wholphin/ui/detail/DetailUtils.kt b/app/src/main/java/com/github/damontecres/wholphin/ui/detail/DetailUtils.kt index c920fb5b..60d8455e 100644 --- a/app/src/main/java/com/github/damontecres/wholphin/ui/detail/DetailUtils.kt +++ b/app/src/main/java/com/github/damontecres/wholphin/ui/detail/DetailUtils.kt @@ -4,6 +4,7 @@ import android.content.Context import androidx.compose.material.icons.Icons import androidx.compose.material.icons.automirrored.filled.ArrowForward import androidx.compose.material.icons.filled.ArrowForward +import androidx.compose.material.icons.filled.Delete import androidx.compose.material.icons.filled.Info import androidx.compose.material.icons.filled.PlayArrow import androidx.compose.material.icons.filled.Refresh @@ -29,6 +30,12 @@ data class MoreDialogActions( var onClickAddPlaylist: (UUID) -> Unit, ) +enum class ClearChosenStreams { + NONE, + ITEM_AND_SERIES, + SERIES, +} + /** * Build the [DialogItem]s when clicking "More" * @@ -53,10 +60,12 @@ fun buildMoreDialogItems( sourceId: UUID?, watched: Boolean, favorite: Boolean, + canClearChosenStreams: Boolean, actions: MoreDialogActions, onChooseVersion: () -> Unit, onChooseTracks: (MediaStreamType) -> Unit, onShowOverview: () -> Unit, + onClearChosenStreams: () -> Unit, ): List = buildList { add( @@ -172,6 +181,16 @@ fun buildMoreDialogItems( }, ) } + if (canClearChosenStreams) { + add( + DialogItem( + context.getString(R.string.clear_track_choices), + Icons.Default.Delete, + ) { + onClearChosenStreams() + }, + ) + } add( DialogItem( context.getString(R.string.play_with_transcoding), diff --git a/app/src/main/java/com/github/damontecres/wholphin/ui/detail/episode/EpisodeDetails.kt b/app/src/main/java/com/github/damontecres/wholphin/ui/detail/episode/EpisodeDetails.kt index c47570f6..6d53b0c1 100644 --- a/app/src/main/java/com/github/damontecres/wholphin/ui/detail/episode/EpisodeDetails.kt +++ b/app/src/main/java/com/github/damontecres/wholphin/ui/detail/episode/EpisodeDetails.kt @@ -155,6 +155,7 @@ fun EpisodeDetails( favorite = ep.data.userData?.isFavorite ?: false, seriesId = ep.data.seriesId, sourceId = chosenStreams?.source?.id?.toUUIDOrNull(), + canClearChosenStreams = chosenStreams?.itemPlayback != null || chosenStreams?.plc != null, actions = moreActions, onChooseVersion = { chooseVersion = @@ -204,6 +205,9 @@ fun EpisodeDetails( ) } }, + onClearChosenStreams = { + viewModel.clearChosenStreams(chosenStreams) + }, ), ) }, diff --git a/app/src/main/java/com/github/damontecres/wholphin/ui/detail/episode/EpisodeViewModel.kt b/app/src/main/java/com/github/damontecres/wholphin/ui/detail/episode/EpisodeViewModel.kt index 584d5510..ee93050e 100644 --- a/app/src/main/java/com/github/damontecres/wholphin/ui/detail/episode/EpisodeViewModel.kt +++ b/app/src/main/java/com/github/damontecres/wholphin/ui/detail/episode/EpisodeViewModel.kt @@ -182,4 +182,19 @@ class EpisodeViewModel release() navigationManager.navigateTo(destination) } + + fun clearChosenStreams(chosenStreams: ChosenStreams?) { + viewModelScope.launchIO { + itemPlaybackRepository.deleteChosenStreams(chosenStreams) + item.value?.let { item -> + val result = + itemPlaybackRepository.getSelectedTracks( + itemId, + item, + userPreferencesService.getCurrent(), + ) + this@EpisodeViewModel.chosenStreams.setValueOnMain(result) + } + } + } } diff --git a/app/src/main/java/com/github/damontecres/wholphin/ui/detail/movie/MovieDetails.kt b/app/src/main/java/com/github/damontecres/wholphin/ui/detail/movie/MovieDetails.kt index 6fc1cc19..a63dd128 100644 --- a/app/src/main/java/com/github/damontecres/wholphin/ui/detail/movie/MovieDetails.kt +++ b/app/src/main/java/com/github/damontecres/wholphin/ui/detail/movie/MovieDetails.kt @@ -193,6 +193,7 @@ fun MovieDetails( favorite = movie.data.userData?.isFavorite ?: false, seriesId = null, sourceId = chosenStreams?.source?.id?.toUUIDOrNull(), + canClearChosenStreams = chosenStreams?.itemPlayback != null || chosenStreams?.plc != null, actions = moreActions, onChooseVersion = { chooseVersion = @@ -241,6 +242,9 @@ fun MovieDetails( files = movie.data.mediaSources.orEmpty(), ) }, + onClearChosenStreams = { + viewModel.clearChosenStreams(chosenStreams) + }, ), ) }, diff --git a/app/src/main/java/com/github/damontecres/wholphin/ui/detail/movie/MovieViewModel.kt b/app/src/main/java/com/github/damontecres/wholphin/ui/detail/movie/MovieViewModel.kt index 793ef8bb..e7bd7168 100644 --- a/app/src/main/java/com/github/damontecres/wholphin/ui/detail/movie/MovieViewModel.kt +++ b/app/src/main/java/com/github/damontecres/wholphin/ui/detail/movie/MovieViewModel.kt @@ -247,4 +247,19 @@ class MovieViewModel release() navigationManager.navigateTo(destination) } + + fun clearChosenStreams(chosenStreams: ChosenStreams?) { + viewModelScope.launchIO { + itemPlaybackRepository.deleteChosenStreams(chosenStreams) + item.value?.let { item -> + val result = + itemPlaybackRepository.getSelectedTracks( + itemId, + item, + userPreferencesService.getCurrent(), + ) + this@MovieViewModel.chosenStreams.setValueOnMain(result) + } + } + } } diff --git a/app/src/main/java/com/github/damontecres/wholphin/ui/detail/series/SeriesOverview.kt b/app/src/main/java/com/github/damontecres/wholphin/ui/detail/series/SeriesOverview.kt index 7575e3a4..891d3bc2 100644 --- a/app/src/main/java/com/github/damontecres/wholphin/ui/detail/series/SeriesOverview.kt +++ b/app/src/main/java/com/github/damontecres/wholphin/ui/detail/series/SeriesOverview.kt @@ -19,6 +19,7 @@ import androidx.hilt.lifecycle.viewmodel.compose.hiltViewModel import androidx.lifecycle.compose.LifecycleResumeEffect import androidx.lifecycle.map import com.github.damontecres.wholphin.R +import com.github.damontecres.wholphin.data.ChosenStreams import com.github.damontecres.wholphin.data.model.BaseItem import com.github.damontecres.wholphin.preferences.UserPreferences import com.github.damontecres.wholphin.ui.RequestOrRestoreFocus @@ -179,6 +180,7 @@ fun SeriesOverview( fun buildMoreForEpisode( ep: BaseItem, + chosenStreams: ChosenStreams?, fromLongClick: Boolean, ): DialogParams = DialogParams( @@ -192,6 +194,7 @@ fun SeriesOverview( favorite = ep.data.userData?.isFavorite ?: false, seriesId = series.id, sourceId = chosenStreams?.source?.id?.toUUIDOrNull(), + canClearChosenStreams = chosenStreams?.itemPlayback != null || chosenStreams?.plc != null, actions = MoreDialogActions( navigateTo = viewModel::navigateTo, @@ -259,6 +262,9 @@ fun SeriesOverview( files = ep.data.mediaSources.orEmpty(), ) }, + onClearChosenStreams = { + viewModel.clearChosenStreams(ep, chosenStreams) + }, ), ) @@ -304,7 +310,7 @@ fun SeriesOverview( ) }, onLongClick = { ep -> - moreDialog = buildMoreForEpisode(ep, true) + moreDialog = buildMoreForEpisode(ep, chosenStreams, true) }, playOnClick = { resume -> rowFocused = EPISODE_ROW @@ -333,7 +339,7 @@ fun SeriesOverview( }, moreOnClick = { episodeList?.getOrNull(position.episodeRowIndex)?.let { ep -> - moreDialog = buildMoreForEpisode(ep, false) + moreDialog = buildMoreForEpisode(ep, chosenStreams, false) } }, overviewOnClick = { diff --git a/app/src/main/java/com/github/damontecres/wholphin/ui/detail/series/SeriesViewModel.kt b/app/src/main/java/com/github/damontecres/wholphin/ui/detail/series/SeriesViewModel.kt index e5fd3cd4..16c6cb37 100644 --- a/app/src/main/java/com/github/damontecres/wholphin/ui/detail/series/SeriesViewModel.kt +++ b/app/src/main/java/com/github/damontecres/wholphin/ui/detail/series/SeriesViewModel.kt @@ -510,6 +510,16 @@ class SeriesViewModel } } } + + fun clearChosenStreams( + item: BaseItem, + chosenStreams: ChosenStreams?, + ) { + viewModelScope.launchIO { + itemPlaybackRepository.deleteChosenStreams(chosenStreams) + lookUpChosenTracks(item.id, item) + } + } } sealed interface EpisodeList { diff --git a/app/src/main/res/values/strings.xml b/app/src/main/res/values/strings.xml index db4e720d..d589ee14 100644 --- a/app/src/main/res/values/strings.xml +++ b/app/src/main/res/values/strings.xml @@ -402,6 +402,7 @@ Local Play trailer No trailers + Clear track choices Disabled diff --git a/app/src/test/java/com/github/damontecres/wholphin/test/TestStreamChoiceService.kt b/app/src/test/java/com/github/damontecres/wholphin/test/TestStreamChoiceService.kt index 7d551049..0f3e5ed7 100644 --- a/app/src/test/java/com/github/damontecres/wholphin/test/TestStreamChoiceService.kt +++ b/app/src/test/java/com/github/damontecres/wholphin/test/TestStreamChoiceService.kt @@ -104,16 +104,6 @@ class TestStreamChoiceServiceBasic( ), itemPlayback = itemPlayback(subtitleIndex = TrackIndex.UNSPECIFIED), ), - TestInput( - 1, - SubtitlePlaybackMode.ALWAYS, - subtitles = - listOf( - subtitle(0, "eng", forced = true, default = true), - subtitle(1, "eng", false), - subtitle(2, "eng", false), - ), - ), ) } } @@ -352,6 +342,31 @@ class TestStreamChoiceServiceSmart( userSubtitleLang = "spa", userAudioLang = "eng", ), + TestInput( + 1, + SubtitlePlaybackMode.SMART, + subtitles = + listOf( + subtitle(0, "eng", false), + subtitle(1, "spa", false), + ), + streamAudioLang = "eng", + userSubtitleLang = "spa", + userAudioLang = null, + ), + TestInput( + 1, + SubtitlePlaybackMode.SMART, + subtitles = + listOf( + subtitle(0, "eng", false), + subtitle(1, "eng", true), + subtitle(2, "spa", false), + ), + streamAudioLang = "eng", + userSubtitleLang = "eng", + userAudioLang = null, + ), ) } } @@ -448,6 +463,156 @@ class TestStreamChoiceServiceOnlyForced( } } +@RunWith(Parameterized::class) +class TestStreamChoiceServiceMultipleChoices( + val input: TestInput, +) { + @Test + fun test() { + runTest(input) + } + + companion object { + @JvmStatic + @Parameterized.Parameters(name = "{index}: {0}") + fun data(): Collection = + listOf( + TestInput( + 0, + SubtitlePlaybackMode.ALWAYS, + subtitles = + listOf( + subtitle(0, "eng", forced = true, default = true), + subtitle(1, "eng", false), + subtitle(2, "eng", false), + ), + ), + TestInput( + 2, + SubtitlePlaybackMode.ALWAYS, + subtitles = + listOf( + subtitle(0, "eng", forced = true, default = false), + subtitle(1, "eng", false), + subtitle(2, "eng", default = true), + ), + ), + TestInput( + 2, + SubtitlePlaybackMode.SMART, + subtitles = + listOf( + subtitle(0, "eng", forced = true, default = false), + subtitle(1, "eng", false), + subtitle(2, "eng", default = true), + ), + userAudioLang = null, + ), + TestInput( + 2, + SubtitlePlaybackMode.SMART, + subtitles = + listOf( + subtitle(0, "eng", forced = true, default = false), + subtitle(1, "eng", false), + subtitle(2, "eng", default = true), + ), + userSubtitleLang = null, + userAudioLang = null, + ), + TestInput( + null, + SubtitlePlaybackMode.SMART, + subtitles = + listOf( + subtitle(0, "eng", forced = true, default = false), + subtitle(1, "eng", false), + subtitle(2, "eng", default = true), + ), + userSubtitleLang = "spa", + userAudioLang = null, + ), + TestInput( + 2, + SubtitlePlaybackMode.SMART, + subtitles = + listOf( + subtitle(0, "eng", forced = true, default = true), + subtitle(1, "eng", false), + subtitle(2, "eng", default = true), + ), + userSubtitleLang = "eng", + userAudioLang = null, + streamAudioLang = "spa", + ), + TestInput( + 0, + SubtitlePlaybackMode.SMART, + subtitles = + listOf( + subtitle(0, "eng", forced = true, default = true), + subtitle(1, "eng", false), + subtitle(2, "eng", default = true), + ), + userSubtitleLang = "eng", + userAudioLang = "eng", + streamAudioLang = "eng", + ), + TestInput( + null, + SubtitlePlaybackMode.SMART, + subtitles = + listOf( + subtitle(0, "eng", forced = true, default = false), + subtitle(1, "eng", false), + subtitle(2, "eng", default = true), + ), + userSubtitleLang = "spa", + userAudioLang = "eng", + streamAudioLang = "spa", + ), + TestInput( + 0, + SubtitlePlaybackMode.SMART, + subtitles = + listOf( + subtitle(0, "spa", forced = true, default = false), + subtitle(1, "spa", false), + subtitle(2, "spa", default = true), + ), + userSubtitleLang = "spa", + userAudioLang = "eng", + streamAudioLang = "eng", + ), + TestInput( + 2, + SubtitlePlaybackMode.SMART, + subtitles = + listOf( + subtitle(0, "spa", forced = true, default = false), + subtitle(1, "spa", false), + subtitle(2, "spa", default = true), + ), + userSubtitleLang = "spa", + userAudioLang = "", + streamAudioLang = "eng", + ), + TestInput( + 2, + SubtitlePlaybackMode.DEFAULT, + subtitles = + listOf( + subtitle(0, "eng", forced = true, default = false), + subtitle(1, "eng", false), + subtitle(2, "eng", default = true), + ), + userSubtitleLang = null, + userAudioLang = null, + ), + ) + } +} + data class TestInput( val expectedIndex: Int?, val userSubtitleMode: SubtitlePlaybackMode?,