From 9345d0a6981ac10c3a8dfd730c77040967bf8ec5 Mon Sep 17 00:00:00 2001 From: Ray <154766448+damontecres@users.noreply.github.com> Date: Sat, 3 Jan 2026 18:09:40 -0500 Subject: [PATCH 1/6] Fix home header not updating & row movement (#627) ## Description Removes an incorrect optimization for showing the home page header Separates logic for focusing on rows on the home page ### Related issues Fixes #624 Fixes #620 --- .../damontecres/wholphin/ui/Extensions.kt | 5 ++-- .../damontecres/wholphin/ui/main/HomePage.kt | 26 ++++++++++++------- 2 files changed, 19 insertions(+), 12 deletions(-) diff --git a/app/src/main/java/com/github/damontecres/wholphin/ui/Extensions.kt b/app/src/main/java/com/github/damontecres/wholphin/ui/Extensions.kt index 31754ef9..4dbb0870 100644 --- a/app/src/main/java/com/github/damontecres/wholphin/ui/Extensions.kt +++ b/app/src/main/java/com/github/damontecres/wholphin/ui/Extensions.kt @@ -78,12 +78,13 @@ inline fun List.indexOfFirstOrNull(predicate: (T) -> Boolean): Int? { /** * Try to call [FocusRequester.requestFocus], but catch & log the exception if something is not configured properly */ -fun FocusRequester.tryRequestFocus(): Boolean = +fun FocusRequester.tryRequestFocus(tag: String? = null): Boolean = try { requestFocus() + tag?.let { Timber.v("Request focus tag=%s", tag) } true } catch (ex: IllegalStateException) { - Timber.w(ex, "Failed to request focus") + Timber.w(ex, "Failed to request focus, tag=%s", tag) false } diff --git a/app/src/main/java/com/github/damontecres/wholphin/ui/main/HomePage.kt b/app/src/main/java/com/github/damontecres/wholphin/ui/main/HomePage.kt index 6cc69dcd..811278fa 100644 --- a/app/src/main/java/com/github/damontecres/wholphin/ui/main/HomePage.kt +++ b/app/src/main/java/com/github/damontecres/wholphin/ui/main/HomePage.kt @@ -226,28 +226,34 @@ fun HomePageContent( mutableStateOf(RowColumn(firstRow, 0)) } val focusedItem = - remember(position) { - position.let { - (homeRows.getOrNull(it.row) as? HomeRowLoadingState.Success)?.items?.getOrNull(it.column) - } + position.let { + (homeRows.getOrNull(it.row) as? HomeRowLoadingState.Success)?.items?.getOrNull(it.column) } val listState = rememberLazyListState() val rowFocusRequesters = remember(homeRows.size) { List(homeRows.size) { FocusRequester() } } - var focused by rememberSaveable { mutableStateOf(false) } + var firstFocused by rememberSaveable { mutableStateOf(false) } LaunchedEffect(homeRows) { - if (!focused) { + if (!firstFocused) { + // Waiting for the first home row to load, then focus on it homeRows .indexOfFirst { it is HomeRowLoadingState.Success && it.items.isNotEmpty() } .takeIf { it >= 0 } ?.let { rowFocusRequesters[it].tryRequestFocus() delay(50) - listState.animateScrollToItem(position.row) - focused = true + listState.scrollToItem(it) + firstFocused = true } - } else { - rowFocusRequesters.getOrNull(position.row)?.tryRequestFocus() + } + } + LaunchedEffect(Unit) { + if (firstFocused) { + // After the first home row was loaded & focused, page recompositions should focus on the positioned row + val index = position.row + rowFocusRequesters.getOrNull(index)?.tryRequestFocus() + delay(50) + listState.scrollToItem(index) } } LaunchedEffect(position) { From 3d4b06db4495fe6084b51ed51a73bea45d45dec0 Mon Sep 17 00:00:00 2001 From: joshjryan Date: Sat, 3 Jan 2026 16:38:07 -0700 Subject: [PATCH 2/6] Guard against navigating to a non-existant tab (#629) Just make sure we don't crash if the selectedTabIndex isn't a positive int. Fixes #628. --------- Co-authored-by: Damontecres --- .../github/damontecres/wholphin/ui/components/TabRow.kt | 4 +++- .../wholphin/ui/detail/series/SeriesViewModel.kt | 7 +++++-- 2 files changed, 8 insertions(+), 3 deletions(-) diff --git a/app/src/main/java/com/github/damontecres/wholphin/ui/components/TabRow.kt b/app/src/main/java/com/github/damontecres/wholphin/ui/components/TabRow.kt index 16e26649..b8607b82 100644 --- a/app/src/main/java/com/github/damontecres/wholphin/ui/components/TabRow.kt +++ b/app/src/main/java/com/github/damontecres/wholphin/ui/components/TabRow.kt @@ -51,7 +51,9 @@ fun TabRow( ) { val state = rememberLazyListState() LaunchedEffect(selectedTabIndex) { - state.animateScrollToItem(selectedTabIndex, -(state.layoutInfo.viewportSize.width / 3.5).toInt()) + if (selectedTabIndex >= 0) { + state.animateScrollToItem(selectedTabIndex, -(state.layoutInfo.viewportSize.width / 3.5).toInt()) + } } val focusRequesters = remember(tabs) { List(tabs.size) { FocusRequester() } } var rowHasFocus by remember { mutableStateOf(false) } 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 181e5b98..e5fd3cd4 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 @@ -161,7 +161,7 @@ class SeriesViewModel } ?: 0 Timber.v("Got initial season index: $index") position.update { - it.copy(seasonTabIndex = index) + it.copy(seasonTabIndex = index.coerceAtLeast(0)) } } } @@ -547,7 +547,10 @@ private suspend fun findIndexOf( val index = if (targetId != null && (targetNum == null || targetNum !in pager.indices)) { // No hint info, so have to check everything - pager.indexOfBlocking { equalsNotNull(it?.id, targetId) } + pager.indexOfBlocking { + equalsNotNull(it?.indexNumber, targetNum) || + equalsNotNull(it?.id, targetId) + } } else if (targetNum != null && targetNum in pager.indices) { // Start searching from the season number and choose direction from there val num = pager.getBlocking(targetNum)?.indexNumber 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 3/6] 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?, From 4f1c73073628f4cf368403b7bb2f12757a95b882 Mon Sep 17 00:00:00 2001 From: Ray <154766448+damontecres@users.noreply.github.com> Date: Sun, 4 Jan 2026 17:25:43 -0500 Subject: [PATCH 4/6] Don't process reqeued MPV commands (#633) ## Description This is an embarrassing bug... In #619, a change was made to avoid a race condition and wait for the video output surface to be ready by re-enqueuing commands. Except I missed cancelling executing the current command. This meant the command would both be executed and queued again. So, if attaching the video surface takes a while, such as needing to wait for a refresh rate change, _a lot_ of commands could have been added to the queue and trigger a constant start-stop playback loop. I think technically it would eventually resolve itself, but can take a while. ### Related issues Should fix #626 --- .../java/com/github/damontecres/wholphin/util/mpv/MpvPlayer.kt | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/app/src/main/java/com/github/damontecres/wholphin/util/mpv/MpvPlayer.kt b/app/src/main/java/com/github/damontecres/wholphin/util/mpv/MpvPlayer.kt index 81eb8669..c2ff22a1 100644 --- a/app/src/main/java/com/github/damontecres/wholphin/util/mpv/MpvPlayer.kt +++ b/app/src/main/java/com/github/damontecres/wholphin/util/mpv/MpvPlayer.kt @@ -848,7 +848,8 @@ class MpvPlayer( // Note: this means nothing will play until it is attached to a surface, // so MpvPlayer can't be used for background audio/music playback Timber.v("MPV is not initialized/attached yet, requeue cmd %s", cmd) - internalHandler.sendMessageDelayed(Message.obtain(msg), 50) + internalHandler.sendMessageDelayed(Message.obtain(msg), 250) + return true } when (cmd) { MpvCommand.PLAY_PAUSE -> { From 356a93310f8fc04a163ea03e3312c040603cf593 Mon Sep 17 00:00:00 2001 From: Ray <154766448+damontecres@users.noreply.github.com> Date: Mon, 5 Jan 2026 15:38:36 -0500 Subject: [PATCH 5/6] MPV: Handle resolution switch (#635) ## Description When switching resolution (and possibly refresh rate less frequently), the initial surface is not always valid. This means, the MPV playback treated it as detaching the surface and would never update. This PR instead subscribes to the `SurfaceHolder`'s callbacks which will provide a new, valid surface once the switch is ready. Then the surface is attached and queued commands execute to start playback. Also temporarily disables content scale options for MPV since the compose implementation conflicts with MPV rendering. ### Related issues Fixes #626 --- .../wholphin/ui/playback/PlaybackDialog.kt | 5 +- .../wholphin/ui/playback/PlaybackPage.kt | 14 ++- .../wholphin/util/mpv/MpvPlayer.kt | 106 ++++++++++++++---- 3 files changed, 99 insertions(+), 26 deletions(-) diff --git a/app/src/main/java/com/github/damontecres/wholphin/ui/playback/PlaybackDialog.kt b/app/src/main/java/com/github/damontecres/wholphin/ui/playback/PlaybackDialog.kt index f6a42f64..1f13b278 100644 --- a/app/src/main/java/com/github/damontecres/wholphin/ui/playback/PlaybackDialog.kt +++ b/app/src/main/java/com/github/damontecres/wholphin/ui/playback/PlaybackDialog.kt @@ -46,6 +46,7 @@ data class PlaybackSettings( @Composable fun PlaybackDialog( enableSubtitleDelay: Boolean, + enableVideoScale: Boolean, type: PlaybackDialogType, settings: PlaybackSettings, onDismissRequest: () -> Unit, @@ -117,7 +118,9 @@ fun PlaybackDialog( buildList { add(stringResource(R.string.audio)) add(stringResource(R.string.playback_speed)) - add(stringResource(R.string.video_scale)) + if (enableVideoScale) { + add(stringResource(R.string.video_scale)) + } if (enableSubtitleDelay) { add(stringResource(R.string.subtitle_delay)) } diff --git a/app/src/main/java/com/github/damontecres/wholphin/ui/playback/PlaybackPage.kt b/app/src/main/java/com/github/damontecres/wholphin/ui/playback/PlaybackPage.kt index 1bf8ec56..3dcf6584 100644 --- a/app/src/main/java/com/github/damontecres/wholphin/ui/playback/PlaybackPage.kt +++ b/app/src/main/java/com/github/damontecres/wholphin/ui/playback/PlaybackPage.kt @@ -40,6 +40,7 @@ import androidx.compose.ui.focus.focusRequester import androidx.compose.ui.graphics.Color import androidx.compose.ui.graphics.RectangleShape import androidx.compose.ui.input.key.onKeyEvent +import androidx.compose.ui.layout.ContentScale import androidx.compose.ui.platform.LocalConfiguration import androidx.compose.ui.platform.LocalDensity import androidx.compose.ui.text.intl.Locale @@ -152,7 +153,7 @@ fun PlaybackPage( var playbackDialog by remember { mutableStateOf(null) } OneTimeLaunchedEffect { if (prefs.playerBackend == PlayerBackend.MPV) { - scope.launch(Dispatchers.Main + ExceptionHandler()) { + scope.launch(Dispatchers.IO + ExceptionHandler()) { preferences.appPreferences.interfacePreferences.subtitlesPreferences.applyToMpv( configuration, density, @@ -161,7 +162,15 @@ fun PlaybackPage( } } AmbientPlayerListener(player) - var contentScale by remember { mutableStateOf(prefs.globalContentScale.scale) } + var contentScale by remember { + mutableStateOf( + if (prefs.playerBackend == PlayerBackend.MPV) { + ContentScale.FillBounds + } else { + prefs.globalContentScale.scale + }, + ) + } var playbackSpeed by remember { mutableFloatStateOf(1.0f) } LaunchedEffect(playbackSpeed) { player.setPlaybackSpeed(playbackSpeed) } @@ -572,6 +581,7 @@ fun PlaybackPage( onPlaybackActionClick = onPlaybackActionClick, onChangeSubtitleDelay = { viewModel.updateSubtitleDelay(it) }, enableSubtitleDelay = player is MpvPlayer, + enableVideoScale = player !is MpvPlayer, ) } } diff --git a/app/src/main/java/com/github/damontecres/wholphin/util/mpv/MpvPlayer.kt b/app/src/main/java/com/github/damontecres/wholphin/util/mpv/MpvPlayer.kt index c2ff22a1..1be415e9 100644 --- a/app/src/main/java/com/github/damontecres/wholphin/util/mpv/MpvPlayer.kt +++ b/app/src/main/java/com/github/damontecres/wholphin/util/mpv/MpvPlayer.kt @@ -69,7 +69,8 @@ class MpvPlayer( ) : BasePlayer(), MPVLib.EventObserver, TrackSelector.InvalidationListener, - Handler.Callback { + Handler.Callback, + SurfaceHolder.Callback { companion object { private const val DEBUG = false } @@ -467,26 +468,55 @@ class MpvPlayer( override fun clearVideoSurfaceHolder(surfaceHolder: SurfaceHolder?): Unit = throw UnsupportedOperationException() + private var surfaceHolder: SurfaceHolder? = null + override fun setVideoSurfaceView(surfaceView: SurfaceView?) { if (DEBUG) Timber.v("setVideoSurfaceView") - val surface = surfaceView?.holder?.surface - if (surface != null && surface.isValid) { - Timber.v("Queued attach") - sendCommand(MpvCommand.ATTACH_SURFACE, surface) - } else { - clearVideoSurfaceView(null) + if (surfaceView != null) { + this.surfaceHolder?.removeCallback(this) + this.surfaceHolder = surfaceView.holder + if (surfaceView.holder != null) { + val surface = surfaceView.holder?.surface + surfaceView.holder.addCallback(this) + Timber.v("Got surface holder: isValid=${surface?.isValid}") + if (surface != null && surface.isValid) { + Timber.v("Queued attach") + sendCommand(MpvCommand.ATTACH_SURFACE, surface) + return + } + } } + clearVideoSurfaceView(null) } override fun clearVideoSurfaceView(surfaceView: SurfaceView?) { - if (surface == surfaceView?.holder?.surface) { + if (surface != null && surface == surfaceView?.holder?.surface) { Timber.d("clearVideoSurfaceView") sendCommand(MpvCommand.ATTACH_SURFACE, null) } else { - Timber.w("clearVideoSurfaceView called with different surface") + Timber.w("clearVideoSurfaceView called with different surface: %s", surfaceView) } } + override fun surfaceChanged( + holder: SurfaceHolder, + format: Int, + width: Int, + height: Int, + ) { + Timber.v("surfaceChanged: format=$format, width=$width, height=$height") + } + + override fun surfaceCreated(holder: SurfaceHolder) { + Timber.v("surfaceCreated") + sendCommand(MpvCommand.ATTACH_SURFACE, holder.surface) + } + + override fun surfaceDestroyed(holder: SurfaceHolder) { + Timber.v("surfaceDestroyed") + sendCommand(MpvCommand.ATTACH_SURFACE, null) + } + override fun setVideoTextureView(textureView: TextureView?): Unit = throw UnsupportedOperationException() override fun clearVideoTextureView(textureView: TextureView?): Unit = throw UnsupportedOperationException() @@ -660,6 +690,7 @@ class MpvPlayer( MPV_EVENT_VIDEO_RECONFIG -> { Timber.d("event: MPV_EVENT_VIDEO_RECONFIG") updateTracksAndNotify() + updateVideoSizeAndNotify() } MPV_EVENT_END_FILE -> { @@ -720,6 +751,19 @@ class MpvPlayer( notifyListeners(EVENT_TRACKS_CHANGED) { onTracksChanged(tracks) } } + private fun updateVideoSizeAndNotify() { + val width = MPVLib.getPropertyInt("width") + val height = MPVLib.getPropertyInt("height") + val videoSize = + if (width != null && height != null) { + VideoSize(width, height) + } else { + VideoSize.UNKNOWN + } + playbackState.update { it.copy(videoSize = videoSize) } + notifyListeners(EVENT_VIDEO_SIZE_CHANGED) { onVideoSizeChanged(videoSize) } + } + private fun loadFile(media: MediaAndPosition) { Timber.v("loadFile: media=$media") playbackState.update { @@ -836,24 +880,34 @@ class MpvPlayer( internalHandler.obtainMessage(cmd.ordinal, obj).sendToTarget() } + private val queuedCommands = mutableListOf>() + override fun handleMessage(msg: Message): Boolean { val cmd = MpvCommand.entries[msg.what] - Timber.d("handleMessage: cmd=$cmd") if (isReleased && cmd != MpvCommand.DESTROY) { Timber.w("Player is released, ignoring command %s", cmd) return true } if (surface == null && !cmd.isLifecycle) { - // If libmpv isn't ready, re-enqueue the messages + // If libmpv isn't ready, ueue the messages // Note: this means nothing will play until it is attached to a surface, // so MpvPlayer can't be used for background audio/music playback - Timber.v("MPV is not initialized/attached yet, requeue cmd %s", cmd) - internalHandler.sendMessageDelayed(Message.obtain(msg), 250) - return true + Timber.v("MPV is not initialized/attached yet, queue cmd %s", cmd) + queuedCommands.add(Pair(cmd, msg.obj)) + } else { + handleCommand(cmd, msg.obj) } + return true + } + + private fun handleCommand( + cmd: MpvCommand, + obj: Any?, + ) { + Timber.d("handleCommand: cmd=$cmd") when (cmd) { MpvCommand.PLAY_PAUSE -> { - val playWhenReady = msg.obj as Boolean + val playWhenReady = obj as Boolean MPVLib.setPropertyBoolean("pause", !playWhenReady) playbackState.update { it.copy(isPaused = !playWhenReady) @@ -867,13 +921,13 @@ class MpvPlayer( } MpvCommand.SET_TRACK_SELECTION -> { - val (propertyName, trackId) = msg.obj as TrackSelection + val (propertyName, trackId) = obj as TrackSelection MPVLib.setPropertyString(propertyName, trackId) updateTracksAndNotify() } MpvCommand.SEEK -> { - val positionMs = msg.obj as Long + val positionMs = obj as Long MPVLib.setPropertyDouble("time-pos", positionMs / 1000.0) playbackState.update { it.copy(positionMs = positionMs) @@ -881,7 +935,7 @@ class MpvPlayer( } MpvCommand.SET_SPEED -> { - val value = msg.obj as Float + val value = obj as Float MPVLib.setPropertyDouble("speed", value.toDouble()) playbackState.update { it.copy(speed = value) @@ -889,7 +943,7 @@ class MpvPlayer( } MpvCommand.SET_SUBTITLE_DELAY -> { - val value = msg.obj as Double + val value = obj as Double MPVLib.setPropertyDouble("sub-delay", value) playbackState.update { it.copy(subtitleDelay = value) @@ -897,11 +951,11 @@ class MpvPlayer( } MpvCommand.LOAD_FILE -> { - loadFile(msg.obj as MediaAndPosition) + loadFile(obj as MediaAndPosition) } MpvCommand.ATTACH_SURFACE -> { - val surface = msg.obj as Surface? + val surface = obj as Surface? if (surface == null || (this.surface != null && this.surface != surface)) { // If clearing or changing the surface MPVLib.detachSurface() @@ -914,6 +968,13 @@ class MpvPlayer( this.surface = surface MPVLib.setOptionString("force-window", "yes") Timber.d("Attached surface") + if (queuedCommands.isNotEmpty()) { + Timber.d("Processing queued commands") + while (queuedCommands.isNotEmpty()) { + val msg = queuedCommands.removeAt(0) + handleCommand(msg.first, msg.second) + } + } } } @@ -928,7 +989,6 @@ class MpvPlayer( Timber.d("MPVLib destroyed") } } - return true } } @@ -1024,7 +1084,7 @@ private data class PlaybackState( val EMPTY = PlaybackState( timestamp = C.TIME_UNSET, - isLoadingFile = false, + isLoadingFile = true, media = null, positionMs = C.TIME_UNSET, durationMs = C.TIME_UNSET, From 6c7a703b9852521502750376d24206c2d16f35a0 Mon Sep 17 00:00:00 2001 From: Damontecres Date: Mon, 5 Jan 2026 15:51:06 -0500 Subject: [PATCH 6/6] Release v0.3.11