From 8c1a818f51a23e3ba0b71b8dcc6cfe12b0c52bc4 Mon Sep 17 00:00:00 2001 From: Innokentii Enikeev Date: Wed, 26 Aug 2026 19:59:13 +0200 Subject: [PATCH 01/25] Fix embedded subtitle discovery and merging --- .../com/kino/puber/data/api/models/Models.kt | 5 +- .../ui/feature/player/model/PlayerUIMapper.kt | 15 +- .../ui/feature/player/model/PlayerUIModels.kt | 12 + .../player/vm/AudioTrackPreferenceResolver.kt | 28 ++- .../feature/player/vm/PlaybackController.kt | 112 +++++++-- .../puber/ui/feature/player/vm/PlayerVM.kt | 34 ++- .../feature/player/vm/SubtitleTrackMerger.kt | 109 +++++++++ app/src/main/res/values/player.xml | 1 + .../puber/data/api/models/SubtitleLinkTest.kt | 29 +++ .../model/PlayerUIMapperSubtitleTest.kt | 72 ++++++ .../vm/AudioTrackPreferenceResolverTest.kt | 100 ++++++++ .../ui/feature/player/vm/PlayerVMTest.kt | 49 ++++ .../player/vm/SubtitleTrackMergerTest.kt | 213 ++++++++++++++++++ 13 files changed, 745 insertions(+), 34 deletions(-) create mode 100644 app/src/main/java/com/kino/puber/ui/feature/player/vm/SubtitleTrackMerger.kt create mode 100644 app/src/test/kotlin/com/kino/puber/data/api/models/SubtitleLinkTest.kt create mode 100644 app/src/test/kotlin/com/kino/puber/ui/feature/player/model/PlayerUIMapperSubtitleTest.kt create mode 100644 app/src/test/kotlin/com/kino/puber/ui/feature/player/vm/SubtitleTrackMergerTest.kt diff --git a/app/src/main/java/com/kino/puber/data/api/models/Models.kt b/app/src/main/java/com/kino/puber/data/api/models/Models.kt index 4dc26ade..4ddf7cc1 100644 --- a/app/src/main/java/com/kino/puber/data/api/models/Models.kt +++ b/app/src/main/java/com/kino/puber/data/api/models/Models.kt @@ -374,7 +374,10 @@ data class SubtitleLink( val url: String, val shift: Int? = null, val embed: Boolean? = null, -) +) { + val shouldSideLoad: Boolean + get() = embed != true +} @Serializable data class TVChannel( diff --git a/app/src/main/java/com/kino/puber/ui/feature/player/model/PlayerUIMapper.kt b/app/src/main/java/com/kino/puber/ui/feature/player/model/PlayerUIMapper.kt index 6b894570..33800283 100644 --- a/app/src/main/java/com/kino/puber/ui/feature/player/model/PlayerUIMapper.kt +++ b/app/src/main/java/com/kino/puber/ui/feature/player/model/PlayerUIMapper.kt @@ -46,14 +46,14 @@ internal class PlayerUIMapper( url = "", ) ) - val duplicateLanguages = subtitles - ?.groupingBy { it.lang } - ?.eachCount() - ?.filterValues { it > 1 } - ?.keys - .orEmpty() + val playableSubtitles = subtitles.orEmpty() + val duplicateLanguages = playableSubtitles + .groupingBy { it.lang } + .eachCount() + .filterValues { it > 1 } + .keys val duplicateLanguageCounters = mutableMapOf() - subtitles?.forEachIndexed { index, sub -> + playableSubtitles.forEachIndexed { index, sub -> val duplicateIndex = duplicateLanguageCounters.compute(sub.lang) { _, count -> count?.inc() ?: 1 } ?: 1 @@ -67,6 +67,7 @@ internal class PlayerUIMapper( }, language = sub.lang, url = sub.url, + isEmbedded = sub.embed == true, ) ) } diff --git a/app/src/main/java/com/kino/puber/ui/feature/player/model/PlayerUIModels.kt b/app/src/main/java/com/kino/puber/ui/feature/player/model/PlayerUIModels.kt index 40e3a284..096c8d27 100644 --- a/app/src/main/java/com/kino/puber/ui/feature/player/model/PlayerUIModels.kt +++ b/app/src/main/java/com/kino/puber/ui/feature/player/model/PlayerUIModels.kt @@ -15,8 +15,20 @@ internal data class SubtitleTrackUIState( val label: String, val language: String, val url: String, + val isEmbedded: Boolean = false, + val isForced: Boolean? = null, + val playerTrackId: String? = null, + val playerGroupIndex: Int? = null, + val playerTrackIndex: Int? = null, ) +internal val SubtitleTrackUIState.isOff: Boolean + get() = language.isEmpty() && + url.isEmpty() && + playerTrackId == null && + playerGroupIndex == null && + playerTrackIndex == null + @Immutable internal data class SoundModeUIState( val index: Int, diff --git a/app/src/main/java/com/kino/puber/ui/feature/player/vm/AudioTrackPreferenceResolver.kt b/app/src/main/java/com/kino/puber/ui/feature/player/vm/AudioTrackPreferenceResolver.kt index ea841129..ea97e90b 100644 --- a/app/src/main/java/com/kino/puber/ui/feature/player/vm/AudioTrackPreferenceResolver.kt +++ b/app/src/main/java/com/kino/puber/ui/feature/player/vm/AudioTrackPreferenceResolver.kt @@ -2,6 +2,7 @@ package com.kino.puber.ui.feature.player.vm import com.kino.puber.ui.feature.player.model.AudioTrackUIState import com.kino.puber.ui.feature.player.model.SubtitleTrackUIState +import com.kino.puber.ui.feature.player.model.isOff internal class AudioTrackPreferenceResolver { @@ -25,11 +26,13 @@ internal class AudioTrackPreferenceResolver { tracks: List, preferredLang: String?, preferredUrl: String?, + preferredPlayerTrackId: String? = null, ): Int { val matchers = listOf( { exactSubtitleUrlMatch(tracks, preferredUrl) }, { stableSubtitleUrlMatch(tracks, preferredUrl) }, - { unambiguousSubtitleLanguageMatch(tracks, preferredLang) }, + { exactPlayerTrackIdMatch(tracks, preferredPlayerTrackId) }, + { subtitleLanguageMatch(tracks, preferredLang) }, ) return matchers.firstNotNullOfOrNull { matcher -> matcher().takeIf { it >= 0 } @@ -40,7 +43,7 @@ internal class AudioTrackPreferenceResolver { tracks: List, preferredUrl: String?, ): Int { - if (preferredUrl == null) return NO_MATCH + if (preferredUrl.isNullOrEmpty()) return NO_MATCH return tracks.indexOfFirst { it.url == preferredUrl } } @@ -48,10 +51,19 @@ internal class AudioTrackPreferenceResolver { tracks: List, preferredUrl: String?, ): Int { - val preferredKey = preferredUrl?.stableSubtitleKey() ?: return NO_MATCH + if (preferredUrl.isNullOrEmpty()) return NO_MATCH + val preferredKey = preferredUrl.stableSubtitleKey() return tracks.indexOfFirst { it.url.stableSubtitleKey() == preferredKey } } + private fun exactPlayerTrackIdMatch( + tracks: List, + preferredPlayerTrackId: String?, + ): Int { + if (preferredPlayerTrackId.isNullOrEmpty()) return NO_MATCH + return tracks.indexOfFirst { it.playerTrackId == preferredPlayerTrackId } + } + private fun exactLabelMatch( tracks: List, preferredLabel: String?, @@ -89,13 +101,17 @@ internal class AudioTrackPreferenceResolver { return tracks.indexOfFirst { it.language == preferredLang } } - private fun unambiguousSubtitleLanguageMatch( + private fun subtitleLanguageMatch( tracks: List, preferredLang: String?, ): Int { if (preferredLang == null) return NO_MATCH - val matches = tracks.withIndex().filter { it.value.language == preferredLang } - return matches.singleOrNull()?.index ?: NO_MATCH + if (preferredLang.isEmpty()) return tracks.indexOfFirst { it.isOff } + val matches = tracks.withIndex().filter { + sameSubtitleLanguage(it.value.language, preferredLang) + } + val manifestMatches = matches.filter { it.value.playerTrackId != null } + return manifestMatches.singleOrNull()?.index ?: matches.singleOrNull()?.index ?: NO_MATCH } /** Extracts voice type from HLS labels like "03. Многоголосый. Red Head Sound (RUS)". */ diff --git a/app/src/main/java/com/kino/puber/ui/feature/player/vm/PlaybackController.kt b/app/src/main/java/com/kino/puber/ui/feature/player/vm/PlaybackController.kt index 8d0977b8..9352b31c 100644 --- a/app/src/main/java/com/kino/puber/ui/feature/player/vm/PlaybackController.kt +++ b/app/src/main/java/com/kino/puber/ui/feature/player/vm/PlaybackController.kt @@ -33,10 +33,16 @@ import com.kino.puber.data.repository.PlayerPreferencesRepository import com.kino.puber.ui.feature.player.model.AudioTrackUIState import com.kino.puber.ui.feature.player.model.BufferPreset import com.kino.puber.ui.feature.player.model.SubtitleTrackUIState +import com.kino.puber.ui.feature.player.model.isOff +import java.util.Locale internal interface PlaybackControl { interface Callback : PlaybackEventSink { - fun onTracksUpdated(audioTracks: List, selectedIndex: Int) + fun onTracksUpdated( + audioTracks: List, + selectedIndex: Int, + subtitleTracks: List = emptyList(), + ) fun onError(message: String) } @@ -317,7 +323,7 @@ internal class PlaybackController( override fun selectSubtitle(track: SubtitleTrackUIState?) { val player = exoPlayer ?: return - if (track == null || track.url.isEmpty()) { + if (track == null || track.isOff) { pendingSubtitleTrack = null player.trackSelectionParameters = player.trackSelectionParameters .buildUpon() @@ -360,7 +366,40 @@ internal class PlaybackController( } private fun buildMediaItem(streamUrl: String, subtitles: List?): MediaItem { - return mediaItemFactory.build(streamUrl, subtitles) + val builder = MediaItem.Builder().setUri(streamUrl) + if (streamUrl.isHlsStreamUrl()) { + builder.setMimeType(MimeTypes.APPLICATION_M3U8) + } + if (!subtitles.isNullOrEmpty()) { + val subtitleConfigs = subtitles.mapNotNull { sub -> + if (!sub.shouldSideLoad) return@mapNotNull null + val subtitleUrl = sub.url + val stableKey = subtitleUrl.stableSubtitleKey() + MediaItem.SubtitleConfiguration.Builder(subtitleUrl.toUri()) + .setMimeType(subtitleMimeType(subtitleUrl)) + .setLanguage(sub.lang) + .setLabel(stableKey) + .setId(stableKey) + .build() + } + if (subtitleConfigs.isNotEmpty()) { + builder.setSubtitleConfigurations(subtitleConfigs) + } + } + return builder.build() + } + + private fun subtitleMimeType(url: String): String { + val normalizedUrl = url + .substringBefore('?') + .substringBefore('#') + .lowercase(Locale.ROOT) + return when { + normalizedUrl.endsWith(".vtt") || normalizedUrl.endsWith(".webvtt") -> MimeTypes.TEXT_VTT + normalizedUrl.endsWith(".ass") || normalizedUrl.endsWith(".ssa") -> MimeTypes.TEXT_SSA + normalizedUrl.endsWith(".ttml") || normalizedUrl.endsWith(".xml") -> MimeTypes.APPLICATION_TTML + else -> MimeTypes.APPLICATION_SUBRIP + } } @OptIn(UnstableApi::class) @@ -383,13 +422,12 @@ internal class PlaybackController( @OptIn(UnstableApi::class) private fun setMediaSource(player: ExoPlayer, mediaItem: MediaItem, streamUrl: String) { val dsFactory = dataSourceFactory ?: return - if (streamUrl.contains(".m3u8") || streamUrl.contains("hls")) { - val hlsMediaItem = mediaItem.buildUpon() - .setMimeType(MimeTypes.APPLICATION_M3U8) - .build() + if (streamUrl.isHlsStreamUrl()) { + // DefaultMediaSourceFactory merges MediaItem subtitle configurations with the HLS source. + // Creating HlsMediaSource directly silently drops every side-loaded subtitle configuration. val hlsSource = createMediaSourceFactory(dsFactory) .setLoadErrorHandlingPolicy(HlsErrorPolicy()) - .createMediaSource(hlsMediaItem) + .createMediaSource(mediaItem) player.setMediaSource(hlsSource) } else { player.setMediaItem(mediaItem) @@ -537,8 +575,6 @@ internal class PlaybackController( private fun notifyTracksUpdated(callback: PlaybackControl.Callback?) { val player = exoPlayer ?: return val audioGroups = player.currentTracks.groups.filter { it.type == C.TRACK_TYPE_AUDIO } - if (audioGroups.isEmpty()) return - val audioTracks = audioGroups.mapIndexed { index, group -> val format = group.getTrackFormat(0) val label = format.label ?: format.language ?: "Track ${index + 1}" @@ -549,7 +585,29 @@ internal class PlaybackController( ) } val selectedIndex = audioGroups.indexOfFirst { it.isSelected }.coerceAtLeast(0) - callback?.onTracksUpdated(audioTracks, selectedIndex) + var subtitleIndex = 0 + val subtitleTracks = player.currentTracks.groups + .filter { it.type == C.TRACK_TYPE_TEXT } + .flatMapIndexed { groupIndex, group -> + (0 until group.length).map { trackIndex -> + val format = group.getTrackFormat(trackIndex) + subtitleIndex += 1 + SubtitleTrackUIState( + index = subtitleIndex, + label = format.label + ?: format.language + ?: context.getString(R.string.player_subtitle_unknown, subtitleIndex), + language = format.language.orEmpty(), + url = "", + isEmbedded = true, + isForced = format.selectionFlags and C.SELECTION_FLAG_FORCED != 0, + playerTrackId = format.id, + playerGroupIndex = groupIndex, + playerTrackIndex = trackIndex, + ) + } + } + callback?.onTracksUpdated(audioTracks, selectedIndex, subtitleTracks) } private fun applyPendingSubtitleSelection() { @@ -580,11 +638,29 @@ internal class PlaybackController( textGroups: List, ): TextTrackSelection? { return findTextTrackBy(textGroups) { format -> - format.id == track.url + track.url.isNotEmpty() && format.id == track.url } ?: findTextTrackBy(textGroups) { format -> - format.id == stableKey || format.label == stableKey - } ?: findTextTrackBySubtitleIndex(textGroups, track.index) - ?: findUnambiguousTextTrackByLanguage(textGroups, track.language) + track.playerTrackId != null && format.id == track.playerTrackId + } ?: findTextTrackBy(textGroups) { format -> + stableKey.isNotEmpty() && (format.id == stableKey || format.label == stableKey) + } ?: findUnambiguousTextTrackByLanguage(textGroups, track.language) + ?: findTextTrackByPlayerCoordinates(textGroups, track) + ?: track.takeUnless { it.isEmbedded }?.let { + findTextTrackBySubtitleIndex(textGroups, it.index) + } + } + + private fun findTextTrackByPlayerCoordinates( + textGroups: List, + track: SubtitleTrackUIState, + ): TextTrackSelection? { + val group = track.playerGroupIndex?.let(textGroups::getOrNull) + val trackIndex = track.playerTrackIndex + return if (group != null && trackIndex != null && trackIndex in 0 until group.length) { + TextTrackSelection(group = group, trackIndex = trackIndex) + } else { + null + } } // Media3 may not expose SubtitleConfiguration id/label for every source type. @@ -612,7 +688,7 @@ internal class PlaybackController( val matches = textGroups.flatMap { group -> (0 until group.length).mapNotNull { trackIndex -> group.getTrackFormat(trackIndex).takeIf { format -> - format.language == language + format.language?.let { sameSubtitleLanguage(it, language) } == true }?.let { TextTrackSelection(group = group, trackIndex = trackIndex) } @@ -648,3 +724,7 @@ internal class PlaybackController( const val BITS_PER_MEGABIT = 1_000_000.0 } } + +private fun String.isHlsStreamUrl(): Boolean { + return contains(".m3u8", ignoreCase = true) || contains("hls", ignoreCase = true) +} diff --git a/app/src/main/java/com/kino/puber/ui/feature/player/vm/PlayerVM.kt b/app/src/main/java/com/kino/puber/ui/feature/player/vm/PlayerVM.kt index c90933a9..ae05fd61 100644 --- a/app/src/main/java/com/kino/puber/ui/feature/player/vm/PlayerVM.kt +++ b/app/src/main/java/com/kino/puber/ui/feature/player/vm/PlayerVM.kt @@ -26,6 +26,7 @@ import com.kino.puber.domain.interactor.player.WatchedDetailsRefreshException import com.kino.puber.ui.feature.player.model.SkipSegmentUIState import com.kino.puber.ui.feature.player.model.ActivePanel import com.kino.puber.ui.feature.player.model.AudioTrackUIState +import com.kino.puber.ui.feature.player.model.SubtitleTrackUIState import com.kino.puber.ui.feature.player.model.FocusTarget import com.kino.puber.ui.feature.player.model.PlayerAction import com.kino.puber.ui.feature.player.model.PlayPauseIndicatorState @@ -120,6 +121,7 @@ internal class PlayerVM( ) private var currentMedia: CurrentMedia? = null + private var apiSubtitleTracks: List = emptyList() private var mediaGeneration = 0L private var controlsHideJob: Job? = null @@ -152,6 +154,7 @@ internal class PlayerVM( private val controlsStateMachine = ControlsStateMachine() private val progressTracker = ProgressTracker() private val audioTrackPreferenceResolver = AudioTrackPreferenceResolver() + private val subtitleTrackMerger = SubtitleTrackMerger() private val debugOverlayEnabled = interactor.isDebugOverlayEnabled() private val playbackCallback = object : PlaybackControl.Callback { @@ -181,16 +184,36 @@ internal class PlayerVM( } } - override fun onTracksUpdated(audioTracks: List, selectedIndex: Int) { + override fun onTracksUpdated( + audioTracks: List, + selectedIndex: Int, + subtitleTracks: List, + ) { + val currentContent = (stateValue as? PlayerViewState.Content)?.content ?: return + val previousSubtitle = currentContent.subtitleTracks + .getOrNull(currentContent.selectedSubtitleIndex) + val mergedSubtitleTracks = subtitleTrackMerger.merge(apiSubtitleTracks, subtitleTracks) + val mergedSelectedIndex = previousSubtitle?.let { selectedTrack -> + audioTrackPreferenceResolver.findSubtitleTrackIndex( + tracks = mergedSubtitleTracks, + preferredLang = selectedTrack.language, + preferredUrl = selectedTrack.url, + preferredPlayerTrackId = selectedTrack.playerTrackId, + ) + }?.takeIf { it >= 0 } ?: 0 updateContent { copy( audioTracks = audioTracks, selectedAudioTrackIndex = selectedIndex, + subtitleTracks = mergedSubtitleTracks, + selectedSubtitleIndex = mergedSelectedIndex, ) } if (!tracksRestoredForCurrentMedia) { tracksRestoredForCurrentMedia = true - restoreTrackPreferences() + if (!restoreTrackPreferences(hasDiscoveredSubtitleTracks = subtitleTracks.isNotEmpty())) { + tracksRestoredForCurrentMedia = false + } } } @@ -288,6 +311,7 @@ internal class PlayerVM( ).copy( isMarkCurrentWatchedInFlight = watchedMutationsInFlight[token.key].orZero() > 0, ) + apiSubtitleTracks = contentState.subtitleTracks controlsHideJob?.cancel() controlsStateMachine.initialize(resumeDialogVisible = resumeDialog != null) @@ -340,12 +364,12 @@ internal class PlayerVM( playbackController.switchStream(streamUrl, media.subtitles) } - private fun restoreTrackPreferences() { + private fun restoreTrackPreferences(hasDiscoveredSubtitleTracks: Boolean): Boolean { val preferredLabel = interactor.getPreferredAudioLabel(params.itemId) val preferredLang = interactor.getPreferredAudioLang(params.itemId) val subtitleLang = interactor.getPreferredSubtitleLang(params.itemId) val subtitleUrl = interactor.getPreferredSubtitleUrl(params.itemId) - val content = (stateValue as? PlayerViewState.Content)?.content ?: return + val content = (stateValue as? PlayerViewState.Content)?.content ?: return false val audioIndex = audioTrackPreferenceResolver.findAudioTrackIndex( tracks = content.audioTracks, preferredLabel = preferredLabel, @@ -364,6 +388,8 @@ internal class PlayerVM( if (subtitleIndex >= 0) { applySubtitleSelection(subtitleIndex, persist = false) } + val waitsForManifestLanguage = !subtitleLang.isNullOrEmpty() && subtitleUrl.isNullOrEmpty() + return subtitleIndex >= 0 || !waitsForManifestLanguage || hasDiscoveredSubtitleTracks } override fun onAction(action: UIAction) { diff --git a/app/src/main/java/com/kino/puber/ui/feature/player/vm/SubtitleTrackMerger.kt b/app/src/main/java/com/kino/puber/ui/feature/player/vm/SubtitleTrackMerger.kt new file mode 100644 index 00000000..1f590e8a --- /dev/null +++ b/app/src/main/java/com/kino/puber/ui/feature/player/vm/SubtitleTrackMerger.kt @@ -0,0 +1,109 @@ +package com.kino.puber.ui.feature.player.vm + +import com.kino.puber.ui.feature.player.model.SubtitleTrackUIState +import com.kino.puber.ui.feature.player.model.isOff +import java.util.Locale + +internal class SubtitleTrackMerger { + + fun merge( + apiTracks: List, + playerTracks: List, + ): List { + val offTrack = apiTracks.firstOrNull { it.isOff } ?: SubtitleTrackUIState( + index = 0, + label = "", + language = "", + url = "", + ) + val apiSubtitles = apiTracks.filterNot { it.isOff } + val playerSubtitles = playerTracks.filterNot { it.isOff } + val availablePlayerIndices = playerSubtitles.indices.toMutableSet() + val matches = mutableMapOf() + + apiSubtitles.forEachIndexed { apiIndex, apiTrack -> + findExactIdentityMatch(apiTrack, playerSubtitles, availablePlayerIndices)?.let { playerIndex -> + matches[apiIndex] = playerIndex + availablePlayerIndices.remove(playerIndex) + } + } + + apiSubtitles.forEachIndexed { apiIndex, apiTrack -> + if (apiIndex in matches || !apiTrack.isEmbedded) return@forEachIndexed + findEmbeddedLanguageMatch(apiTrack, playerSubtitles, availablePlayerIndices)?.let { playerIndex -> + matches[apiIndex] = playerIndex + availablePlayerIndices.remove(playerIndex) + } + } + + val merged = buildList { + add(offTrack) + apiSubtitles.forEachIndexed { apiIndex, apiTrack -> + val playerTrack = matches[apiIndex]?.let(playerSubtitles::get) + add(if (playerTrack == null) apiTrack else apiTrack.withPlayerIdentity(playerTrack)) + } + availablePlayerIndices.forEach { playerIndex -> + add(playerSubtitles[playerIndex]) + } + } + return merged.mapIndexed { index, track -> track.copy(index = index) } + } + + private fun findExactIdentityMatch( + apiTrack: SubtitleTrackUIState, + playerTracks: List, + availablePlayerIndices: Set, + ): Int? { + val apiKey = apiTrack.url.stableSubtitleKey().takeIf { it.isNotEmpty() } ?: return null + return availablePlayerIndices.firstOrNull { playerIndex -> + val playerTrack = playerTracks[playerIndex] + val playerIdKey = playerTrack.playerTrackId + ?.stableSubtitleKey() + ?.takeIf { it.isNotEmpty() } + playerTrack.playerTrackId == apiTrack.url || + playerIdKey == apiKey || + playerTrack.label == apiKey + } + } + + private fun findEmbeddedLanguageMatch( + apiTrack: SubtitleTrackUIState, + playerTracks: List, + availablePlayerIndices: Set, + ): Int? { + val languageMatches = availablePlayerIndices.filter { playerIndex -> + sameSubtitleLanguage(apiTrack.language, playerTracks[playerIndex].language) + } + if (languageMatches.isEmpty()) return null + return apiTrack.isForced?.let { forced -> + languageMatches.firstOrNull { playerTracks[it].isForced == forced } + ?: languageMatches.singleOrNull() + } ?: languageMatches.first() + } + + private fun SubtitleTrackUIState.withPlayerIdentity( + playerTrack: SubtitleTrackUIState, + ): SubtitleTrackUIState = copy( + playerTrackId = playerTrack.playerTrackId, + playerGroupIndex = playerTrack.playerGroupIndex, + playerTrackIndex = playerTrack.playerTrackIndex, + isForced = isForced ?: playerTrack.isForced, + ) +} + +internal fun sameSubtitleLanguage(first: String, second: String): Boolean { + if (first.isBlank() || second.isBlank()) return false + return canonicalSubtitleLanguage(first) == canonicalSubtitleLanguage(second) +} + +private fun canonicalSubtitleLanguage(language: String): String { + val normalized = language + .trim() + .lowercase(Locale.ROOT) + .substringBefore('-') + .substringBefore('_') + return runCatching { Locale.forLanguageTag(normalized).isO3Language } + .getOrNull() + ?.takeIf { it.isNotBlank() && it != "und" } + ?: normalized +} diff --git a/app/src/main/res/values/player.xml b/app/src/main/res/values/player.xml index 054aa4e9..da220e12 100644 --- a/app/src/main/res/values/player.xml +++ b/app/src/main/res/values/player.xml @@ -15,6 +15,7 @@ СУБТИТРЫ Выкл. %1$s · вариант %2$d + Субтитры %1$d Аа Размер Маленький Средний diff --git a/app/src/test/kotlin/com/kino/puber/data/api/models/SubtitleLinkTest.kt b/app/src/test/kotlin/com/kino/puber/data/api/models/SubtitleLinkTest.kt new file mode 100644 index 00000000..365eb71b --- /dev/null +++ b/app/src/test/kotlin/com/kino/puber/data/api/models/SubtitleLinkTest.kt @@ -0,0 +1,29 @@ +package com.kino.puber.data.api.models + +import org.junit.jupiter.api.Assertions.assertFalse +import org.junit.jupiter.api.Assertions.assertTrue +import org.junit.jupiter.api.Test + +internal class SubtitleLinkTest { + + @Test + fun shouldSideLoad_includesExternalSubtitle() { + val externalSubtitle = SubtitleLink( + lang = "eng", + url = "https://cdn.test/subtitle.srt", + ) + + assertTrue(externalSubtitle.shouldSideLoad) + } + + @Test + fun shouldSideLoad_excludesEmbeddedSubtitle() { + val subtitle = SubtitleLink( + lang = "eng", + url = "https://cdn.test/subtitle.srt", + embed = true, + ) + + assertFalse(subtitle.shouldSideLoad) + } +} diff --git a/app/src/test/kotlin/com/kino/puber/ui/feature/player/model/PlayerUIMapperSubtitleTest.kt b/app/src/test/kotlin/com/kino/puber/ui/feature/player/model/PlayerUIMapperSubtitleTest.kt new file mode 100644 index 00000000..f3f65239 --- /dev/null +++ b/app/src/test/kotlin/com/kino/puber/ui/feature/player/model/PlayerUIMapperSubtitleTest.kt @@ -0,0 +1,72 @@ +package com.kino.puber.ui.feature.player.model + +import android.content.Context +import com.kino.puber.R +import com.kino.puber.data.api.models.SubtitleLink +import io.mockk.every +import io.mockk.mockk +import org.junit.jupiter.api.Assertions.assertEquals +import org.junit.jupiter.api.Test + +internal class PlayerUIMapperSubtitleTest { + + private val context = mockk(relaxed = true).also { context -> + every { context.getString(R.string.player_subtitles_off) } returns "Off" + every { + context.getString(R.string.player_subtitle_variant_label, any(), any()) + } answers { + val formatArgs = secondArg>() + "${formatArgs[0]} variant ${formatArgs[1]}" + } + } + private val mapper = PlayerUIMapper(context) + + @Test + fun mapSubtitleTracks_returnsOnlyOffTrack_whenInputIsEmpty() { + val result = mapper.mapSubtitleTracks(emptyList()) + + assertEquals(listOf(0), result.map { it.index }) + assertEquals(listOf(""), result.map { it.language }) + assertEquals(listOf(""), result.map { it.url }) + } + + @Test + fun mapSubtitleTracks_preservesLanguagesAndEmbedMetadata_inApiOrder() { + val embeddedUrl = "https://cdn.test/subtitles/russian.vtt" + val externalUrl = "https://cdn.test/subtitles/english.vtt" + + val result = mapper.mapSubtitleTracks( + listOf( + SubtitleLink(lang = "rus", url = embeddedUrl, embed = true), + SubtitleLink(lang = "eng", url = externalUrl, embed = false), + ) + ) + + assertEquals(listOf(0, 1, 2), result.map { it.index }) + assertEquals(listOf("", "rus", "eng"), result.map { it.language }) + assertEquals(listOf("", embeddedUrl, externalUrl), result.map { it.url }) + assertEquals(listOf(false, true, false), result.map { it.isEmbedded }) + } + + @Test + fun mapSubtitleTracks_keepsSameLanguageVariantsDistinct() { + val result = mapper.mapSubtitleTracks( + listOf( + SubtitleLink( + lang = "rus", + url = "https://cdn.test/subtitles/russian-full.vtt", + embed = true, + ), + SubtitleLink( + lang = "rus", + url = "https://cdn.test/subtitles/russian-forced.vtt", + embed = false, + ), + ) + ) + + assertEquals(listOf("rus variant 1", "rus variant 2"), result.drop(1).map { it.label }) + assertEquals(listOf("rus", "rus"), result.drop(1).map { it.language }) + assertEquals(listOf(true, false), result.drop(1).map { it.isEmbedded }) + } +} diff --git a/app/src/test/kotlin/com/kino/puber/ui/feature/player/vm/AudioTrackPreferenceResolverTest.kt b/app/src/test/kotlin/com/kino/puber/ui/feature/player/vm/AudioTrackPreferenceResolverTest.kt index bbb47ae2..7bf9e78f 100644 --- a/app/src/test/kotlin/com/kino/puber/ui/feature/player/vm/AudioTrackPreferenceResolverTest.kt +++ b/app/src/test/kotlin/com/kino/puber/ui/feature/player/vm/AudioTrackPreferenceResolverTest.kt @@ -61,4 +61,104 @@ internal class AudioTrackPreferenceResolverTest { assertEquals(-1, resolver.findSubtitleTrackIndex(tracks, "rus", null)) } + + @Test + fun findSubtitleTrackIndex_usesLanguageForUrlLessManifestTrack() { + val tracks = listOf( + subtitleTrack(index = 0, language = "", url = ""), + subtitleTrack(index = 1, language = "en", url = "", playerTrackId = "hls-english"), + ) + + val result = resolver.findSubtitleTrackIndex( + tracks = tracks, + preferredLang = "eng", + preferredUrl = "", + ) + + assertEquals(1, result) + } + + @Test + fun findSubtitleTrackIndex_preservesExplicitOffPreference() { + val tracks = listOf( + subtitleTrack(index = 0, language = "", url = ""), + subtitleTrack(index = 1, language = "eng", url = "", playerTrackId = "hls-english"), + ) + + val result = resolver.findSubtitleTrackIndex( + tracks = tracks, + preferredLang = "", + preferredUrl = "", + ) + + assertEquals(0, result) + } + + @Test + fun findSubtitleTrackIndex_prefersCurrentPlayerIdentity_overLanguageFallback() { + val tracks = listOf( + subtitleTrack(index = 0, language = "", url = ""), + subtitleTrack(index = 1, language = "rus", url = "", playerTrackId = "full"), + subtitleTrack(index = 2, language = "rus", url = "", playerTrackId = "forced"), + ) + + val result = resolver.findSubtitleTrackIndex( + tracks = tracks, + preferredLang = "rus", + preferredUrl = "", + preferredPlayerTrackId = "forced", + ) + + assertEquals(2, result) + } + + @Test + fun findSubtitleTrackIndex_prefersSavedUrl_overCurrentPlayerIdentity() { + val externalUrl = "https://cdn.test/subtitles/russian.vtt" + val tracks = listOf( + subtitleTrack(index = 0, language = "", url = ""), + subtitleTrack(index = 1, language = "rus", url = externalUrl), + subtitleTrack(index = 2, language = "rus", url = "", playerTrackId = "hls-russian"), + ) + + val result = resolver.findSubtitleTrackIndex( + tracks = tracks, + preferredLang = "rus", + preferredUrl = externalUrl, + preferredPlayerTrackId = "hls-russian", + ) + + assertEquals(1, result) + } + + @Test + fun findSubtitleTrackIndex_returnsNoMatch_untilPreferredManifestLanguageAppears() { + val tracks = listOf( + subtitleTrack(index = 0, language = "", url = ""), + subtitleTrack(index = 1, language = "rus", url = "https://cdn.test/subtitles/russian.vtt"), + ) + + val result = resolver.findSubtitleTrackIndex( + tracks = tracks, + preferredLang = "ukr", + preferredUrl = "", + ) + + assertEquals(-1, result) + } + + private fun subtitleTrack( + index: Int, + language: String, + url: String, + playerTrackId: String? = null, + ) = SubtitleTrackUIState( + index = index, + label = "Track $index", + language = language, + url = url, + playerTrackId = playerTrackId, + playerGroupIndex = playerTrackId?.let { index - 1 }, + playerTrackIndex = playerTrackId?.let { 0 }, + ) } diff --git a/app/src/test/kotlin/com/kino/puber/ui/feature/player/vm/PlayerVMTest.kt b/app/src/test/kotlin/com/kino/puber/ui/feature/player/vm/PlayerVMTest.kt index c4ccc14c..9d34ac7e 100644 --- a/app/src/test/kotlin/com/kino/puber/ui/feature/player/vm/PlayerVMTest.kt +++ b/app/src/test/kotlin/com/kino/puber/ui/feature/player/vm/PlayerVMTest.kt @@ -731,6 +731,55 @@ internal class PlayerVMTest : PlayerVMTestFixture() { verify { playbackController.selectSubtitle(testSubtitleTracks[0]) } } + @Test + fun tracksUpdated_addsAndSelectsManifestOnlySubtitle_withLanguagePreference() { + val vm = startedVM() + val audioTracks = listOf(AudioTrackUIState(0, "English", "eng")) + val manifestTrack = testSubtitleTracks.first().copy( + index = 1, + label = "Ukrainian HLS", + language = "uk", + playerTrackId = "hls-ukrainian", + playerGroupIndex = 0, + playerTrackIndex = 0, + ) + + callbackSlot.captured.onTracksUpdated(audioTracks, 0, listOf(manifestTrack)) + vm.onAction(PlayerAction.SelectSubtitle(3)) + + val selectedTrack = contentState(vm).subtitleTracks[3] + assertEquals("uk", selectedTrack.language) + assertEquals("hls-ukrainian", selectedTrack.playerTrackId) + verify { playbackController.selectSubtitle(selectedTrack) } + verify { interactor.saveTrackPreferences(42, "eng", "English", "uk", null) } + } + + @Test + fun tracksUpdated_defersUrlLessLanguagePreference_untilManifestTracksAppear() { + every { interactor.getPreferredSubtitleLang(42) } returns "ukr" + every { interactor.getPreferredSubtitleUrl(42) } returns "" + val vm = startedVM() + val audioTracks = listOf(AudioTrackUIState(0, "English", "eng")) + val manifestTrack = testSubtitleTracks.first().copy( + index = 1, + label = "Ukrainian HLS", + language = "uk", + playerTrackId = "hls-ukrainian", + playerGroupIndex = 0, + playerTrackIndex = 0, + ) + + callbackSlot.captured.onTracksUpdated(audioTracks, 0, emptyList()) + verify(exactly = 0) { playbackController.selectSubtitle(any()) } + + callbackSlot.captured.onTracksUpdated(audioTracks, 0, listOf(manifestTrack)) + + val selectedTrack = contentState(vm).subtitleTracks[3] + assertEquals(3, contentState(vm).selectedSubtitleIndex) + assertEquals("hls-ukrainian", selectedTrack.playerTrackId) + verify { playbackController.selectSubtitle(selectedTrack) } + } + @Test fun tracksUpdated_restoresPreferredSubtitleByUrl_beforeLanguage() { every { interactor.getPreferredSubtitleLang(42) } returns "rus" diff --git a/app/src/test/kotlin/com/kino/puber/ui/feature/player/vm/SubtitleTrackMergerTest.kt b/app/src/test/kotlin/com/kino/puber/ui/feature/player/vm/SubtitleTrackMergerTest.kt new file mode 100644 index 00000000..0fef693a --- /dev/null +++ b/app/src/test/kotlin/com/kino/puber/ui/feature/player/vm/SubtitleTrackMergerTest.kt @@ -0,0 +1,213 @@ +package com.kino.puber.ui.feature.player.vm + +import com.kino.puber.ui.feature.player.model.SubtitleTrackUIState +import org.junit.jupiter.api.Assertions.assertEquals +import org.junit.jupiter.api.Assertions.assertFalse +import org.junit.jupiter.api.Assertions.assertTrue +import org.junit.jupiter.api.Test + +internal class SubtitleTrackMergerTest { + + private val merger = SubtitleTrackMerger() + + @Test + fun merge_matchesExactExternalIdentity_beforeEmbeddedLanguage() { + val apiTracks = listOf( + offTrack(), + apiTrack( + index = 1, + label = "Russian embedded", + language = "rus", + url = "https://api.test/subtitles/embedded-rus.srt", + embedded = true, + ), + apiTrack( + index = 2, + label = "Russian external", + language = "rus", + url = "https://api.test/subtitles/external-rus.srt", + embedded = false, + ), + ) + val playerTracks = listOf( + playerTrack( + index = 1, + label = "external-rus.srt", + language = "rus", + id = "external-rus.srt", + groupIndex = 0, + ), + playerTrack( + index = 2, + label = "Русские полные", + language = "ru", + id = "hls-russian-full", + groupIndex = 1, + ), + ) + + val result = merger.merge(apiTracks, playerTracks) + + assertEquals( + listOf("Off", "Russian embedded", "Russian external"), + result.map { it.label }, + ) + assertEquals("hls-russian-full", result[1].playerTrackId) + assertEquals("rus", result[1].language) + assertEquals("external-rus.srt", result[2].playerTrackId) + } + + @Test + fun merge_matchesEmbeddedVariants_byForcedMetadata() { + val apiTracks = listOf( + offTrack(), + apiTrack(1, "Russian full", "rus", embedded = true, forced = false), + apiTrack(2, "Russian forced", "rus", embedded = true, forced = true), + ) + val playerTracks = listOf( + playerTrack(1, "Russian forced HLS", "ru", "forced", 0, forced = true), + playerTrack(2, "Russian full HLS", "ru", "full", 1, forced = false), + ) + + val result = merger.merge(apiTracks, playerTracks) + + assertEquals("full", result[1].playerTrackId) + assertFalse(result[1].isForced!!) + assertEquals("forced", result[2].playerTrackId) + assertTrue(result[2].isForced!!) + } + + @Test + fun merge_appendsManifestOnlyTracks_withoutChangingTheirLanguage() { + val playerTrack = playerTrack( + index = 7, + label = "Українські", + language = "uk", + id = "hls-ukrainian", + groupIndex = 2, + ) + + val result = merger.merge(listOf(offTrack()), listOf(playerTrack)) + + assertEquals(listOf("", "uk"), result.map { it.language }) + assertEquals(listOf(0, 1), result.map { it.index }) + assertEquals("hls-ukrainian", result[1].playerTrackId) + assertEquals(2, result[1].playerGroupIndex) + } + + @Test + fun merge_doesNotCollapseExternalAndManifestTracks_byLanguageAlone() { + val apiTracks = listOf( + offTrack(), + apiTrack( + index = 1, + label = "Russian external", + language = "rus", + url = "https://api.test/subtitles/external.srt", + embedded = false, + ), + ) + val playerTracks = listOf( + playerTrack(1, "Russian HLS", "ru", "hls-russian", 0), + ) + + val result = merger.merge(apiTracks, playerTracks) + + assertEquals(listOf("Off", "Russian external", "Russian HLS"), result.map { it.label }) + assertEquals("https://api.test/subtitles/external.srt", result[1].url) + assertEquals("hls-russian", result[2].playerTrackId) + } + + @Test + fun merge_matchesSideLoadedTrackByStableUrl_whenHostAndTokenChange() { + val apiTrack = apiTrack( + index = 1, + label = "Russian external", + language = "rus", + url = "https://old-cdn.test/subtitles/russian.vtt?token=expired", + embedded = false, + ) + val playerTrack = playerTrack( + index = 1, + label = "russian.vtt", + language = "ru", + id = "https://new-cdn.test/subtitles/russian.vtt?token=fresh", + groupIndex = 0, + ) + + val result = merger.merge(listOf(offTrack(), apiTrack), listOf(playerTrack)) + + assertEquals(listOf("Off", "Russian external"), result.map { it.label }) + assertEquals(playerTrack.playerTrackId, result[1].playerTrackId) + assertEquals("rus", result[1].language) + } + + @Test + fun merge_keepsUnmatchedEmbeddedAndManifestTracks_withoutFalseLanguageMatch() { + val embeddedRussian = apiTrack( + index = 1, + label = "Russian embedded", + language = "rus", + embedded = true, + ) + val manifestEnglish = playerTrack( + index = 1, + label = "English HLS", + language = "en", + id = "hls-english", + groupIndex = 0, + ) + + val result = merger.merge( + listOf(offTrack(), embeddedRussian), + listOf(manifestEnglish), + ) + + assertEquals(listOf("Off", "Russian embedded", "English HLS"), result.map { it.label }) + assertEquals(listOf("", "rus", "en"), result.map { it.language }) + assertEquals(null, result[1].playerTrackId) + assertEquals("hls-english", result[2].playerTrackId) + } + + private fun offTrack() = SubtitleTrackUIState( + index = 0, + label = "Off", + language = "", + url = "", + ) + + private fun apiTrack( + index: Int, + label: String, + language: String, + url: String = "", + embedded: Boolean, + forced: Boolean? = null, + ) = SubtitleTrackUIState( + index = index, + label = label, + language = language, + url = url, + isEmbedded = embedded, + isForced = forced, + ) + + private fun playerTrack( + index: Int, + label: String, + language: String, + id: String, + groupIndex: Int, + forced: Boolean = false, + ) = SubtitleTrackUIState( + index = index, + label = label, + language = language, + url = "", + isEmbedded = true, + isForced = forced, + playerTrackId = id, + playerGroupIndex = groupIndex, + playerTrackIndex = 0, + ) +} From 105d59e6b2ef833e337af4cd6c8615e17ff94114 Mon Sep 17 00:00:00 2001 From: Innokentii Enikeev Date: Thu, 27 Aug 2026 19:32:18 +0200 Subject: [PATCH 02/25] Handle same-language subtitle variants --- .../com/kino/puber/data/api/models/Models.kt | 11 ++++ .../ui/feature/player/model/PlayerUIMapper.kt | 1 + .../player/vm/AudioTrackPreferenceResolver.kt | 13 +++++ .../puber/ui/feature/player/vm/PlayerVM.kt | 7 ++- .../puber/data/api/models/SubtitleLinkTest.kt | 25 +++++++++ .../model/PlayerUIMapperSubtitleTest.kt | 5 +- .../vm/AudioTrackPreferenceResolverTest.kt | 34 +++++++++++ .../player/vm/PlayerVMSubtitleVariantTest.kt | 56 +++++++++++++++++++ .../ui/feature/player/vm/PlayerVMTest.kt | 2 +- 9 files changed, 148 insertions(+), 6 deletions(-) create mode 100644 app/src/test/kotlin/com/kino/puber/ui/feature/player/vm/PlayerVMSubtitleVariantTest.kt diff --git a/app/src/main/java/com/kino/puber/data/api/models/Models.kt b/app/src/main/java/com/kino/puber/data/api/models/Models.kt index 4ddf7cc1..f8fde9bd 100644 --- a/app/src/main/java/com/kino/puber/data/api/models/Models.kt +++ b/app/src/main/java/com/kino/puber/data/api/models/Models.kt @@ -377,8 +377,19 @@ data class SubtitleLink( ) { val shouldSideLoad: Boolean get() = embed != true + + // KinoPub does not expose a forced flag separately, but marks the variant in the subtitle path. + val isForced: Boolean + get() = FORCED_SUBTITLE_TOKEN.containsMatchIn( + url.substringBefore('?').substringBefore('#'), + ) } +private val FORCED_SUBTITLE_TOKEN = Regex( + pattern = """(^|[/_.-])forced([/_.-]|$)""", + option = RegexOption.IGNORE_CASE, +) + @Serializable data class TVChannel( val id: Int, val title: String, val stream: String? = null, val epg: List? = null, diff --git a/app/src/main/java/com/kino/puber/ui/feature/player/model/PlayerUIMapper.kt b/app/src/main/java/com/kino/puber/ui/feature/player/model/PlayerUIMapper.kt index 33800283..c3aa1ae1 100644 --- a/app/src/main/java/com/kino/puber/ui/feature/player/model/PlayerUIMapper.kt +++ b/app/src/main/java/com/kino/puber/ui/feature/player/model/PlayerUIMapper.kt @@ -68,6 +68,7 @@ internal class PlayerUIMapper( language = sub.lang, url = sub.url, isEmbedded = sub.embed == true, + isForced = sub.isForced, ) ) } diff --git a/app/src/main/java/com/kino/puber/ui/feature/player/vm/AudioTrackPreferenceResolver.kt b/app/src/main/java/com/kino/puber/ui/feature/player/vm/AudioTrackPreferenceResolver.kt index ea97e90b..999446f1 100644 --- a/app/src/main/java/com/kino/puber/ui/feature/player/vm/AudioTrackPreferenceResolver.kt +++ b/app/src/main/java/com/kino/puber/ui/feature/player/vm/AudioTrackPreferenceResolver.kt @@ -31,6 +31,8 @@ internal class AudioTrackPreferenceResolver { val matchers = listOf( { exactSubtitleUrlMatch(tracks, preferredUrl) }, { stableSubtitleUrlMatch(tracks, preferredUrl) }, + { exactPlayerTrackIdMatch(tracks, preferredUrl) }, + { stablePlayerTrackIdMatch(tracks, preferredUrl) }, { exactPlayerTrackIdMatch(tracks, preferredPlayerTrackId) }, { subtitleLanguageMatch(tracks, preferredLang) }, ) @@ -64,6 +66,17 @@ internal class AudioTrackPreferenceResolver { return tracks.indexOfFirst { it.playerTrackId == preferredPlayerTrackId } } + private fun stablePlayerTrackIdMatch( + tracks: List, + preferredPlayerTrackId: String?, + ): Int { + if (preferredPlayerTrackId.isNullOrEmpty()) return NO_MATCH + val preferredKey = preferredPlayerTrackId.stableSubtitleKey() + return tracks.indexOfFirst { + it.playerTrackId?.stableSubtitleKey() == preferredKey + } + } + private fun exactLabelMatch( tracks: List, preferredLabel: String?, diff --git a/app/src/main/java/com/kino/puber/ui/feature/player/vm/PlayerVM.kt b/app/src/main/java/com/kino/puber/ui/feature/player/vm/PlayerVM.kt index ae05fd61..440c0aff 100644 --- a/app/src/main/java/com/kino/puber/ui/feature/player/vm/PlayerVM.kt +++ b/app/src/main/java/com/kino/puber/ui/feature/player/vm/PlayerVM.kt @@ -388,8 +388,8 @@ internal class PlayerVM( if (subtitleIndex >= 0) { applySubtitleSelection(subtitleIndex, persist = false) } - val waitsForManifestLanguage = !subtitleLang.isNullOrEmpty() && subtitleUrl.isNullOrEmpty() - return subtitleIndex >= 0 || !waitsForManifestLanguage || hasDiscoveredSubtitleTracks + val hasSubtitlePreference = !subtitleLang.isNullOrEmpty() || !subtitleUrl.isNullOrEmpty() + return subtitleIndex >= 0 || !hasSubtitlePreference || hasDiscoveredSubtitleTracks } override fun onAction(action: UIAction) { @@ -1373,7 +1373,8 @@ internal class PlayerVM( audioLang = audioTrack?.language?.takeIf { it.isNotEmpty() }, audioLabel = audioTrack?.label?.takeIf { it.isNotEmpty() }, subtitleLang = subtitle?.language?.takeIf { it.isNotEmpty() }, - subtitleUrl = subtitle?.url?.takeIf { it.isNotEmpty() }, + subtitleUrl = subtitle?.url?.takeIf { it.isNotEmpty() } + ?: subtitle?.playerTrackId?.takeIf { it.isNotEmpty() }, ) } diff --git a/app/src/test/kotlin/com/kino/puber/data/api/models/SubtitleLinkTest.kt b/app/src/test/kotlin/com/kino/puber/data/api/models/SubtitleLinkTest.kt index 365eb71b..8aeaf2bc 100644 --- a/app/src/test/kotlin/com/kino/puber/data/api/models/SubtitleLinkTest.kt +++ b/app/src/test/kotlin/com/kino/puber/data/api/models/SubtitleLinkTest.kt @@ -26,4 +26,29 @@ internal class SubtitleLinkTest { assertFalse(subtitle.shouldSideLoad) } + + @Test + fun isForced_detectsForcedMarkerInPath_ignoringCaseAndQuery() { + val subtitle = SubtitleLink( + lang = "rus", + url = "https://cdn.test/subtitles/RUS-FORCED.vtt?token=forced-value", + ) + + assertTrue(subtitle.isForced) + } + + @Test + fun isForced_doesNotUseQueryOrPartialWordAsMarker() { + val regularSubtitle = SubtitleLink( + lang = "rus", + url = "https://cdn.test/subtitles/russian.vtt?mode=forced", + ) + val unforcedSubtitle = SubtitleLink( + lang = "rus", + url = "https://cdn.test/subtitles/russian-unforced.vtt", + ) + + assertFalse(regularSubtitle.isForced) + assertFalse(unforcedSubtitle.isForced) + } } diff --git a/app/src/test/kotlin/com/kino/puber/ui/feature/player/model/PlayerUIMapperSubtitleTest.kt b/app/src/test/kotlin/com/kino/puber/ui/feature/player/model/PlayerUIMapperSubtitleTest.kt index f3f65239..f131886c 100644 --- a/app/src/test/kotlin/com/kino/puber/ui/feature/player/model/PlayerUIMapperSubtitleTest.kt +++ b/app/src/test/kotlin/com/kino/puber/ui/feature/player/model/PlayerUIMapperSubtitleTest.kt @@ -60,13 +60,14 @@ internal class PlayerUIMapperSubtitleTest { SubtitleLink( lang = "rus", url = "https://cdn.test/subtitles/russian-forced.vtt", - embed = false, + embed = true, ), ) ) assertEquals(listOf("rus variant 1", "rus variant 2"), result.drop(1).map { it.label }) assertEquals(listOf("rus", "rus"), result.drop(1).map { it.language }) - assertEquals(listOf(true, false), result.drop(1).map { it.isEmbedded }) + assertEquals(listOf(true, true), result.drop(1).map { it.isEmbedded }) + assertEquals(listOf(false, true), result.drop(1).map { it.isForced }) } } diff --git a/app/src/test/kotlin/com/kino/puber/ui/feature/player/vm/AudioTrackPreferenceResolverTest.kt b/app/src/test/kotlin/com/kino/puber/ui/feature/player/vm/AudioTrackPreferenceResolverTest.kt index 7bf9e78f..5cd72c62 100644 --- a/app/src/test/kotlin/com/kino/puber/ui/feature/player/vm/AudioTrackPreferenceResolverTest.kt +++ b/app/src/test/kotlin/com/kino/puber/ui/feature/player/vm/AudioTrackPreferenceResolverTest.kt @@ -131,6 +131,40 @@ internal class AudioTrackPreferenceResolverTest { assertEquals(1, result) } + @Test + fun findSubtitleTrackIndex_usesSavedManifestIdentity_forSameLanguageVariants() { + val tracks = listOf( + subtitleTrack(index = 0, language = "", url = ""), + subtitleTrack(index = 1, language = "rus", url = "", playerTrackId = "rus-full"), + subtitleTrack(index = 2, language = "rus", url = "", playerTrackId = "rus-forced"), + ) + + val result = resolver.findSubtitleTrackIndex( + tracks = tracks, + preferredLang = "rus", + preferredUrl = "rus-forced", + ) + + assertEquals(2, result) + } + + @Test + fun findSubtitleTrackIndex_doesNotGuessBetweenSameLanguageVariants() { + val tracks = listOf( + subtitleTrack(index = 0, language = "", url = ""), + subtitleTrack(index = 1, language = "rus", url = "", playerTrackId = "rus-full"), + subtitleTrack(index = 2, language = "rus", url = "", playerTrackId = "rus-forced"), + ) + + val result = resolver.findSubtitleTrackIndex( + tracks = tracks, + preferredLang = "rus", + preferredUrl = null, + ) + + assertEquals(-1, result) + } + @Test fun findSubtitleTrackIndex_returnsNoMatch_untilPreferredManifestLanguageAppears() { val tracks = listOf( diff --git a/app/src/test/kotlin/com/kino/puber/ui/feature/player/vm/PlayerVMSubtitleVariantTest.kt b/app/src/test/kotlin/com/kino/puber/ui/feature/player/vm/PlayerVMSubtitleVariantTest.kt new file mode 100644 index 00000000..2e794454 --- /dev/null +++ b/app/src/test/kotlin/com/kino/puber/ui/feature/player/vm/PlayerVMSubtitleVariantTest.kt @@ -0,0 +1,56 @@ +package com.kino.puber.ui.feature.player.vm + +import com.kino.puber.ui.feature.player.model.AudioTrackUIState +import com.kino.puber.util.MainDispatcherExtension +import io.mockk.every +import io.mockk.verify +import org.junit.jupiter.api.Assertions.assertEquals +import org.junit.jupiter.api.Test +import org.junit.jupiter.api.extension.RegisterExtension + +internal class PlayerVMSubtitleVariantTest : PlayerVMTestFixture() { + + companion object { + @JvmField + @RegisterExtension + val mainDispatcher = MainDispatcherExtension() + } + + @Test + fun tracksUpdated_restoresForcedManifestSubtitleBySavedIdentity() { + every { interactor.getPreferredSubtitleLang(42) } returns "rus" + every { interactor.getPreferredSubtitleUrl(42) } returns "hls-russian-forced" + val vm = startedVM() + val audioTracks = listOf(AudioTrackUIState(0, "English", "eng")) + val manifestTracks = listOf( + testSubtitleTracks.first().copy( + index = 1, + label = "Russian full", + language = "ru", + isForced = false, + playerTrackId = "hls-russian-full", + playerGroupIndex = 0, + playerTrackIndex = 0, + ), + testSubtitleTracks.first().copy( + index = 2, + label = "Russian forced", + language = "ru", + isForced = true, + playerTrackId = "hls-russian-forced", + playerGroupIndex = 1, + playerTrackIndex = 0, + ), + ) + + callbackSlot.captured.onTracksUpdated(audioTracks, 0, emptyList()) + verify(exactly = 0) { playbackController.selectSubtitle(any()) } + + callbackSlot.captured.onTracksUpdated(audioTracks, 0, manifestTracks) + + val selectedTrack = contentState(vm).subtitleTracks[4] + assertEquals("hls-russian-forced", selectedTrack.playerTrackId) + assertEquals(4, contentState(vm).selectedSubtitleIndex) + verify { playbackController.selectSubtitle(selectedTrack) } + } +} diff --git a/app/src/test/kotlin/com/kino/puber/ui/feature/player/vm/PlayerVMTest.kt b/app/src/test/kotlin/com/kino/puber/ui/feature/player/vm/PlayerVMTest.kt index 9d34ac7e..28efd614 100644 --- a/app/src/test/kotlin/com/kino/puber/ui/feature/player/vm/PlayerVMTest.kt +++ b/app/src/test/kotlin/com/kino/puber/ui/feature/player/vm/PlayerVMTest.kt @@ -751,7 +751,7 @@ internal class PlayerVMTest : PlayerVMTestFixture() { assertEquals("uk", selectedTrack.language) assertEquals("hls-ukrainian", selectedTrack.playerTrackId) verify { playbackController.selectSubtitle(selectedTrack) } - verify { interactor.saveTrackPreferences(42, "eng", "English", "uk", null) } + verify { interactor.saveTrackPreferences(42, "eng", "English", "uk", "hls-ukrainian") } } @Test From 0ea108eccd4aa5b43263f3d43d266fa347243215 Mon Sep 17 00:00:00 2001 From: Innokentii Enikeev Date: Fri, 28 Aug 2026 00:19:07 +0200 Subject: [PATCH 03/25] Fix HLS subtitle identity mapping --- .../com/kino/puber/data/api/models/Models.kt | 15 +- .../player/component/AudioSubtitlesPanel.kt | 9 +- .../player/component/SettingsPanelColumn.kt | 5 +- .../ui/feature/player/model/PlayerUIMapper.kt | 22 +-- .../ui/feature/player/model/PlayerUIModels.kt | 4 +- .../player/vm/AudioTrackPreferenceResolver.kt | 62 +++--- .../feature/player/vm/PlaybackController.kt | 160 ++++++++++------ .../puber/ui/feature/player/vm/PlayerVM.kt | 51 +++-- .../player/vm/SubtitleTrackIdentity.kt | 27 +++ .../feature/player/vm/SubtitleTrackMerger.kt | 96 ++++------ app/src/main/res/values/player.xml | 2 +- .../puber/data/api/models/SubtitleLinkTest.kt | 18 ++ .../component/AudioSubtitlesPanelTest.kt | 30 +++ .../model/PlayerUIMapperSubtitleTest.kt | 31 +-- .../vm/AudioTrackPreferenceResolverTest.kt | 94 ++++++++- .../player/vm/PlayerVMSubtitleVariantTest.kt | 137 +++++++++++++- .../ui/feature/player/vm/PlayerVMTest.kt | 66 +------ .../feature/player/vm/PlayerVMTestFixture.kt | 11 ++ .../player/vm/SubtitleTrackMergerTest.kt | 179 +++++++++++++++--- 19 files changed, 728 insertions(+), 291 deletions(-) create mode 100644 app/src/test/kotlin/com/kino/puber/ui/feature/player/component/AudioSubtitlesPanelTest.kt diff --git a/app/src/main/java/com/kino/puber/data/api/models/Models.kt b/app/src/main/java/com/kino/puber/data/api/models/Models.kt index f8fde9bd..60894c21 100644 --- a/app/src/main/java/com/kino/puber/data/api/models/Models.kt +++ b/app/src/main/java/com/kino/puber/data/api/models/Models.kt @@ -374,15 +374,22 @@ data class SubtitleLink( val url: String, val shift: Int? = null, val embed: Boolean? = null, + val forced: Boolean? = null, + val file: String? = null, ) { val shouldSideLoad: Boolean get() = embed != true - // KinoPub does not expose a forced flag separately, but marks the variant in the subtitle path. + // Older API responses may omit the dedicated forced flag. + val forcedState: Boolean? + get() = forced ?: true.takeIf { + FORCED_SUBTITLE_TOKEN.containsMatchIn( + url.substringBefore('?').substringBefore('#'), + ) + } + val isForced: Boolean - get() = FORCED_SUBTITLE_TOKEN.containsMatchIn( - url.substringBefore('?').substringBefore('#'), - ) + get() = forcedState == true } private val FORCED_SUBTITLE_TOKEN = Regex( diff --git a/app/src/main/java/com/kino/puber/ui/feature/player/component/AudioSubtitlesPanel.kt b/app/src/main/java/com/kino/puber/ui/feature/player/component/AudioSubtitlesPanel.kt index 26485d61..b45ca72f 100644 --- a/app/src/main/java/com/kino/puber/ui/feature/player/component/AudioSubtitlesPanel.kt +++ b/app/src/main/java/com/kino/puber/ui/feature/player/component/AudioSubtitlesPanel.kt @@ -205,7 +205,10 @@ private fun RowScope.SubtitleColumn( panelFocusRequester: FocusRequester?, onSubtitleSelected: (Int) -> Unit, ) { - val labels = remember(subtitleTracks) { subtitleTracks.map { it.label } } + val forcedLabel = stringResource(R.string.player_subtitle_forced) + val labels = remember(subtitleTracks, forcedLabel) { + subtitleTracks.map { it.subtitlePickerLabel(forcedLabel) } + } SettingsPanelColumn( header = stringResource(R.string.player_panel_subtitles), items = labels, @@ -217,6 +220,10 @@ private fun RowScope.SubtitleColumn( ) } +internal fun SubtitleTrackUIState.subtitlePickerLabel(forcedLabel: String): String { + return if (isForced == true) "$label · $forcedLabel" else label +} + @Composable private fun BoxScope.SubtitleSizeButton( onClick: () -> Unit, diff --git a/app/src/main/java/com/kino/puber/ui/feature/player/component/SettingsPanelColumn.kt b/app/src/main/java/com/kino/puber/ui/feature/player/component/SettingsPanelColumn.kt index 90b1250d..279ba62f 100644 --- a/app/src/main/java/com/kino/puber/ui/feature/player/component/SettingsPanelColumn.kt +++ b/app/src/main/java/com/kino/puber/ui/feature/player/component/SettingsPanelColumn.kt @@ -19,6 +19,8 @@ import androidx.compose.ui.focus.FocusRequester import androidx.compose.ui.focus.focusRequester import androidx.compose.ui.graphics.Color import androidx.compose.ui.platform.testTag +import androidx.compose.ui.semantics.selected +import androidx.compose.ui.semantics.semantics import androidx.compose.ui.unit.dp import androidx.tv.material3.ClickableSurfaceDefaults import androidx.tv.material3.Icon @@ -90,7 +92,8 @@ private fun SettingsPanelItem( onClick = onClick, modifier = Modifier .then(testTag?.let { Modifier.testTag(it) } ?: Modifier) - .then(focusRequester?.let { Modifier.focusRequester(it) } ?: Modifier), + .then(focusRequester?.let { Modifier.focusRequester(it) } ?: Modifier) + .semantics { this.selected = selected }, colors = ClickableSurfaceDefaults.colors( containerColor = Color.Transparent, focusedContainerColor = colors.primary.copy(alpha = 0.2f), diff --git a/app/src/main/java/com/kino/puber/ui/feature/player/model/PlayerUIMapper.kt b/app/src/main/java/com/kino/puber/ui/feature/player/model/PlayerUIMapper.kt index c3aa1ae1..f9b0766e 100644 --- a/app/src/main/java/com/kino/puber/ui/feature/player/model/PlayerUIMapper.kt +++ b/app/src/main/java/com/kino/puber/ui/feature/player/model/PlayerUIMapper.kt @@ -46,29 +46,15 @@ internal class PlayerUIMapper( url = "", ) ) - val playableSubtitles = subtitles.orEmpty() - val duplicateLanguages = playableSubtitles - .groupingBy { it.lang } - .eachCount() - .filterValues { it > 1 } - .keys - val duplicateLanguageCounters = mutableMapOf() - playableSubtitles.forEachIndexed { index, sub -> - val duplicateIndex = duplicateLanguageCounters.compute(sub.lang) { _, count -> - count?.inc() ?: 1 - } ?: 1 + subtitles?.forEachIndexed { index, sub -> result.add( SubtitleTrackUIState( index = index + 1, - label = if (sub.lang in duplicateLanguages) { - context.getString(R.string.player_subtitle_variant_label, sub.lang, duplicateIndex) - } else { - sub.lang - }, + label = sub.lang, language = sub.lang, url = sub.url, - isEmbedded = sub.embed == true, - isForced = sub.isForced, + isForced = sub.forcedState, + sourceFile = sub.file, ) ) } diff --git a/app/src/main/java/com/kino/puber/ui/feature/player/model/PlayerUIModels.kt b/app/src/main/java/com/kino/puber/ui/feature/player/model/PlayerUIModels.kt index 096c8d27..dc1569fb 100644 --- a/app/src/main/java/com/kino/puber/ui/feature/player/model/PlayerUIModels.kt +++ b/app/src/main/java/com/kino/puber/ui/feature/player/model/PlayerUIModels.kt @@ -15,9 +15,10 @@ internal data class SubtitleTrackUIState( val label: String, val language: String, val url: String, - val isEmbedded: Boolean = false, val isForced: Boolean? = null, + val sourceFile: String? = null, val playerTrackId: String? = null, + val playerTrackUri: String? = null, val playerGroupIndex: Int? = null, val playerTrackIndex: Int? = null, ) @@ -26,6 +27,7 @@ internal val SubtitleTrackUIState.isOff: Boolean get() = language.isEmpty() && url.isEmpty() && playerTrackId == null && + playerTrackUri == null && playerGroupIndex == null && playerTrackIndex == null diff --git a/app/src/main/java/com/kino/puber/ui/feature/player/vm/AudioTrackPreferenceResolver.kt b/app/src/main/java/com/kino/puber/ui/feature/player/vm/AudioTrackPreferenceResolver.kt index 999446f1..b606bf9c 100644 --- a/app/src/main/java/com/kino/puber/ui/feature/player/vm/AudioTrackPreferenceResolver.kt +++ b/app/src/main/java/com/kino/puber/ui/feature/player/vm/AudioTrackPreferenceResolver.kt @@ -27,13 +27,19 @@ internal class AudioTrackPreferenceResolver { preferredLang: String?, preferredUrl: String?, preferredPlayerTrackId: String? = null, + preferredPlayerGroupIndex: Int? = null, + preferredPlayerTrackIndex: Int? = null, ): Int { val matchers = listOf( - { exactSubtitleUrlMatch(tracks, preferredUrl) }, - { stableSubtitleUrlMatch(tracks, preferredUrl) }, - { exactPlayerTrackIdMatch(tracks, preferredUrl) }, - { stablePlayerTrackIdMatch(tracks, preferredUrl) }, - { exactPlayerTrackIdMatch(tracks, preferredPlayerTrackId) }, + { subtitleIdentityMatch(tracks, preferredUrl) }, + { subtitleIdentityMatch(tracks, preferredPlayerTrackId) }, + { + playerCoordinatesMatch( + tracks, + preferredPlayerGroupIndex, + preferredPlayerTrackIndex, + ) + }, { subtitleLanguageMatch(tracks, preferredLang) }, ) return matchers.firstNotNullOfOrNull { matcher -> @@ -41,40 +47,26 @@ internal class AudioTrackPreferenceResolver { } ?: NO_MATCH } - private fun exactSubtitleUrlMatch( + private fun subtitleIdentityMatch( tracks: List, - preferredUrl: String?, - ): Int { - if (preferredUrl.isNullOrEmpty()) return NO_MATCH - return tracks.indexOfFirst { it.url == preferredUrl } - } - - private fun stableSubtitleUrlMatch( - tracks: List, - preferredUrl: String?, + preferredIdentity: String?, ): Int { - if (preferredUrl.isNullOrEmpty()) return NO_MATCH - val preferredKey = preferredUrl.stableSubtitleKey() - return tracks.indexOfFirst { it.url.stableSubtitleKey() == preferredKey } + if (preferredIdentity.isNullOrEmpty()) return NO_MATCH + return tracks.withIndex().filter { (_, track) -> + track.identities.any { identity -> sameSubtitleIdentity(identity, preferredIdentity) } + }.singleOrNull()?.index ?: NO_MATCH } - private fun exactPlayerTrackIdMatch( + private fun playerCoordinatesMatch( tracks: List, - preferredPlayerTrackId: String?, + preferredGroupIndex: Int?, + preferredTrackIndex: Int?, ): Int { - if (preferredPlayerTrackId.isNullOrEmpty()) return NO_MATCH - return tracks.indexOfFirst { it.playerTrackId == preferredPlayerTrackId } - } - - private fun stablePlayerTrackIdMatch( - tracks: List, - preferredPlayerTrackId: String?, - ): Int { - if (preferredPlayerTrackId.isNullOrEmpty()) return NO_MATCH - val preferredKey = preferredPlayerTrackId.stableSubtitleKey() - return tracks.indexOfFirst { - it.playerTrackId?.stableSubtitleKey() == preferredKey - } + if (preferredGroupIndex == null || preferredTrackIndex == null) return NO_MATCH + return tracks.withIndex().filter { (_, track) -> + track.playerGroupIndex == preferredGroupIndex && + track.playerTrackIndex == preferredTrackIndex + }.singleOrNull()?.index ?: NO_MATCH } private fun exactLabelMatch( @@ -143,3 +135,7 @@ internal class AudioTrackPreferenceResolver { val NUMBER_PREFIX_REGEX = Regex("""^\d+\.\s*""") } } + +private val SubtitleTrackUIState.identities: List + get() = listOfNotNull(url, sourceFile, playerTrackUri, playerTrackId) + .filter { it.isNotEmpty() } diff --git a/app/src/main/java/com/kino/puber/ui/feature/player/vm/PlaybackController.kt b/app/src/main/java/com/kino/puber/ui/feature/player/vm/PlaybackController.kt index 9352b31c..3be6754c 100644 --- a/app/src/main/java/com/kino/puber/ui/feature/player/vm/PlaybackController.kt +++ b/app/src/main/java/com/kino/puber/ui/feature/player/vm/PlaybackController.kt @@ -17,15 +17,17 @@ import androidx.media3.datasource.DataSource import androidx.media3.datasource.cache.CacheDataSource import androidx.media3.datasource.okhttp.OkHttpDataSource import androidx.media3.exoplayer.DefaultLoadControl +import androidx.media3.exoplayer.ExoPlayer +import androidx.media3.exoplayer.hls.HlsManifest +import androidx.media3.exoplayer.hls.HlsMediaSource +import androidx.media3.exoplayer.hls.playlist.HlsMultivariantPlaylist import androidx.media3.exoplayer.mediacodec.MediaCodecRenderer import androidx.media3.exoplayer.source.BehindLiveWindowException -import androidx.media3.extractor.DefaultExtractorsFactory -import okhttp3.OkHttpClient -import androidx.media3.exoplayer.ExoPlayer +import androidx.media3.exoplayer.source.DefaultMediaSourceFactory import androidx.media3.exoplayer.trackselection.AdaptiveTrackSelection import androidx.media3.exoplayer.trackselection.DefaultTrackSelector import androidx.media3.exoplayer.upstream.DefaultBandwidthMeter -import androidx.media3.exoplayer.source.DefaultMediaSourceFactory +import androidx.media3.extractor.DefaultExtractorsFactory import com.kino.puber.BuildConfig import com.kino.puber.R import com.kino.puber.data.api.models.SubtitleLink @@ -35,6 +37,7 @@ import com.kino.puber.ui.feature.player.model.BufferPreset import com.kino.puber.ui.feature.player.model.SubtitleTrackUIState import com.kino.puber.ui.feature.player.model.isOff import java.util.Locale +import okhttp3.OkHttpClient internal interface PlaybackControl { interface Callback : PlaybackEventSink { @@ -346,6 +349,7 @@ internal class PlaybackController( exoPlayer = null trackSelector = null dataSourceFactory = null + pendingSubtitleTrack = null } @OptIn(UnstableApi::class) @@ -367,10 +371,13 @@ internal class PlaybackController( private fun buildMediaItem(streamUrl: String, subtitles: List?): MediaItem { val builder = MediaItem.Builder().setUri(streamUrl) - if (streamUrl.isHlsStreamUrl()) { + val isHlsStream = streamUrl.isHlsStreamUrl() + if (isHlsStream) { builder.setMimeType(MimeTypes.APPLICATION_M3U8) } - if (!subtitles.isNullOrEmpty()) { + // KinoPub HLS manifests already contain the API subtitle list as renditions. + // Adding the API URLs again creates a second set of Media3 text tracks. + if (!isHlsStream && !subtitles.isNullOrEmpty()) { val subtitleConfigs = subtitles.mapNotNull { sub -> if (!sub.shouldSideLoad) return@mapNotNull null val subtitleUrl = sub.url @@ -423,9 +430,8 @@ internal class PlaybackController( private fun setMediaSource(player: ExoPlayer, mediaItem: MediaItem, streamUrl: String) { val dsFactory = dataSourceFactory ?: return if (streamUrl.isHlsStreamUrl()) { - // DefaultMediaSourceFactory merges MediaItem subtitle configurations with the HLS source. - // Creating HlsMediaSource directly silently drops every side-loaded subtitle configuration. - val hlsSource = createMediaSourceFactory(dsFactory) + val hlsSource = HlsMediaSource.Factory(dsFactory) + .setAllowChunklessPreparation(true) .setLoadErrorHandlingPolicy(HlsErrorPolicy()) .createMediaSource(mediaItem) player.setMediaSource(hlsSource) @@ -575,6 +581,8 @@ internal class PlaybackController( private fun notifyTracksUpdated(callback: PlaybackControl.Callback?) { val player = exoPlayer ?: return val audioGroups = player.currentTracks.groups.filter { it.type == C.TRACK_TYPE_AUDIO } + val textGroups = player.currentTracks.groups.filter { it.type == C.TRACK_TYPE_TEXT } + if (audioGroups.isEmpty() && textGroups.isEmpty()) return val audioTracks = audioGroups.mapIndexed { index, group -> val format = group.getTrackFormat(0) val label = format.label ?: format.language ?: "Track ${index + 1}" @@ -585,29 +593,85 @@ internal class PlaybackController( ) } val selectedIndex = audioGroups.indexOfFirst { it.isSelected }.coerceAtLeast(0) + val hlsSubtitles = (player.currentManifest as? HlsManifest) + ?.multivariantPlaylist + ?.subtitles + .orEmpty() + callback?.onTracksUpdated( + audioTracks, + selectedIndex, + buildSubtitleTracks(textGroups, hlsSubtitles), + ) + } + + private fun buildSubtitleTracks( + textGroups: List, + hlsSubtitles: List, + ): List { var subtitleIndex = 0 - val subtitleTracks = player.currentTracks.groups - .filter { it.type == C.TRACK_TYPE_TEXT } - .flatMapIndexed { groupIndex, group -> - (0 until group.length).map { trackIndex -> - val format = group.getTrackFormat(trackIndex) - subtitleIndex += 1 - SubtitleTrackUIState( - index = subtitleIndex, - label = format.label - ?: format.language - ?: context.getString(R.string.player_subtitle_unknown, subtitleIndex), - language = format.language.orEmpty(), - url = "", - isEmbedded = true, - isForced = format.selectionFlags and C.SELECTION_FLAG_FORCED != 0, - playerTrackId = format.id, - playerGroupIndex = groupIndex, - playerTrackIndex = trackIndex, - ) - } + val textTrackCount = textGroups.sumOf { it.length } + return textGroups.flatMapIndexed { groupIndex, group -> + (0 until group.length).map { trackIndex -> + val format = group.getTrackFormat(trackIndex) + subtitleIndex += 1 + buildSubtitleTrack( + index = subtitleIndex, + format = format, + hlsRendition = findHlsRendition( + format = format, + renditionIndex = subtitleIndex - 1, + textTrackCount = textTrackCount, + hlsSubtitles = hlsSubtitles, + ), + groupIndex = groupIndex, + trackIndex = trackIndex, + ) } - callback?.onTracksUpdated(audioTracks, selectedIndex, subtitleTracks) + } + } + + private fun findHlsRendition( + format: Format, + renditionIndex: Int, + textTrackCount: Int, + hlsSubtitles: List, + ): HlsMultivariantPlaylist.Rendition? { + return hlsSubtitles.filter { rendition -> + format.id != null && rendition.format.id == format.id + }.singleOrNull() ?: hlsSubtitles.filter { rendition -> + rendition.format.label == format.label && + sameSubtitleLanguage( + rendition.format.language.orEmpty(), + format.language.orEmpty(), + ) + }.singleOrNull() ?: hlsSubtitles.getOrNull(renditionIndex) + .takeIf { hlsSubtitles.size == textTrackCount } + } + + private fun buildSubtitleTrack( + index: Int, + format: Format, + hlsRendition: HlsMultivariantPlaylist.Rendition?, + groupIndex: Int, + trackIndex: Int, + ): SubtitleTrackUIState { + val identityFormat = hlsRendition?.format + val language = format.language ?: identityFormat?.language.orEmpty() + val fallbackLabel = format.label + ?: identityFormat?.label + ?: context.getString(R.string.player_subtitle_unknown, index) + return SubtitleTrackUIState( + index = index, + label = subtitleTrackDisplayLabel(language, fallbackLabel), + language = language, + url = "", + isForced = (format.selectionFlags or (identityFormat?.selectionFlags ?: 0)) and + C.SELECTION_FLAG_FORCED != 0, + playerTrackId = format.id ?: identityFormat?.id, + playerTrackUri = hlsRendition?.url?.toString(), + playerGroupIndex = groupIndex, + playerTrackIndex = trackIndex, + ) } private fun applyPendingSubtitleSelection() { @@ -619,16 +683,14 @@ internal class PlaybackController( val stableKey = track.url.stableSubtitleKey() val textGroups = player.currentTracks.groups.filter { it.type == C.TRACK_TYPE_TEXT } val target = findTextTrack(track, stableKey, textGroups) + if (target == null) return val builder = player.trackSelectionParameters .buildUpon() .clearOverridesOfType(C.TRACK_TYPE_TEXT) .setTrackTypeDisabled(C.TRACK_TYPE_TEXT, false) - .setPreferredTextLanguage(track.language) - if (target != null) { - builder.setOverrideForType( + .setOverrideForType( TrackSelectionOverride(target.group.mediaTrackGroup, target.trackIndex), ) - } player.trackSelectionParameters = builder.build() } @@ -643,11 +705,8 @@ internal class PlaybackController( track.playerTrackId != null && format.id == track.playerTrackId } ?: findTextTrackBy(textGroups) { format -> stableKey.isNotEmpty() && (format.id == stableKey || format.label == stableKey) - } ?: findUnambiguousTextTrackByLanguage(textGroups, track.language) - ?: findTextTrackByPlayerCoordinates(textGroups, track) - ?: track.takeUnless { it.isEmbedded }?.let { - findTextTrackBySubtitleIndex(textGroups, it.index) - } + } ?: findTextTrackByPlayerCoordinates(textGroups, track) + ?: findUnambiguousTextTrackByLanguage(textGroups, track.language) } private fun findTextTrackByPlayerCoordinates( @@ -663,23 +722,6 @@ internal class PlaybackController( } } - // Media3 may not expose SubtitleConfiguration id/label for every source type. - // The current track list still preserves the subtitle configuration order. - private fun findTextTrackBySubtitleIndex( - textGroups: List, - subtitleIndex: Int, - ): TextTrackSelection? { - val targetIndex = subtitleIndex - 1 - if (targetIndex < 0) return null - return textGroups - .flatMap { group -> - (0 until group.length).map { trackIndex -> - TextTrackSelection(group = group, trackIndex = trackIndex) - } - } - .getOrNull(targetIndex) - } - private fun findUnambiguousTextTrackByLanguage( textGroups: List, language: String, @@ -701,13 +743,13 @@ internal class PlaybackController( textGroups: List, predicate: (Format) -> Boolean, ): TextTrackSelection? { - return textGroups.firstNotNullOfOrNull { group -> - (0 until group.length).firstNotNullOfOrNull { trackIndex -> + return textGroups.flatMap { group -> + (0 until group.length).mapNotNull { trackIndex -> group.getTrackFormat(trackIndex).takeIf(predicate)?.let { TextTrackSelection(group = group, trackIndex = trackIndex) } } - } + }.singleOrNull() } private data class TextTrackSelection( diff --git a/app/src/main/java/com/kino/puber/ui/feature/player/vm/PlayerVM.kt b/app/src/main/java/com/kino/puber/ui/feature/player/vm/PlayerVM.kt index 440c0aff..2004cf95 100644 --- a/app/src/main/java/com/kino/puber/ui/feature/player/vm/PlayerVM.kt +++ b/app/src/main/java/com/kino/puber/ui/feature/player/vm/PlayerVM.kt @@ -27,6 +27,7 @@ import com.kino.puber.ui.feature.player.model.SkipSegmentUIState import com.kino.puber.ui.feature.player.model.ActivePanel import com.kino.puber.ui.feature.player.model.AudioTrackUIState import com.kino.puber.ui.feature.player.model.SubtitleTrackUIState +import com.kino.puber.ui.feature.player.model.isOff import com.kino.puber.ui.feature.player.model.FocusTarget import com.kino.puber.ui.feature.player.model.PlayerAction import com.kino.puber.ui.feature.player.model.PlayPauseIndicatorState @@ -192,13 +193,19 @@ internal class PlayerVM( val currentContent = (stateValue as? PlayerViewState.Content)?.content ?: return val previousSubtitle = currentContent.subtitleTracks .getOrNull(currentContent.selectedSubtitleIndex) - val mergedSubtitleTracks = subtitleTrackMerger.merge(apiSubtitleTracks, subtitleTracks) + val previousPlayerTracks = currentContent.subtitleTracks.filter { + it.playerGroupIndex != null && it.playerTrackIndex != null + } + val effectivePlayerTracks = subtitleTracks.ifEmpty { previousPlayerTracks } + val mergedSubtitleTracks = subtitleTrackMerger.merge(apiSubtitleTracks, effectivePlayerTracks) val mergedSelectedIndex = previousSubtitle?.let { selectedTrack -> audioTrackPreferenceResolver.findSubtitleTrackIndex( tracks = mergedSubtitleTracks, preferredLang = selectedTrack.language, preferredUrl = selectedTrack.url, preferredPlayerTrackId = selectedTrack.playerTrackId, + preferredPlayerGroupIndex = selectedTrack.playerGroupIndex, + preferredPlayerTrackIndex = selectedTrack.playerTrackIndex, ) }?.takeIf { it >= 0 } ?: 0 updateContent { @@ -211,7 +218,7 @@ internal class PlayerVM( } if (!tracksRestoredForCurrentMedia) { tracksRestoredForCurrentMedia = true - if (!restoreTrackPreferences(hasDiscoveredSubtitleTracks = subtitleTracks.isNotEmpty())) { + if (!restoreTrackPreferences(hasDiscoveredSubtitleTracks = effectivePlayerTracks.isNotEmpty())) { tracksRestoredForCurrentMedia = false } } @@ -311,11 +318,7 @@ internal class PlayerVM( ).copy( isMarkCurrentWatchedInFlight = watchedMutationsInFlight[token.key].orZero() > 0, ) - apiSubtitleTracks = contentState.subtitleTracks - - controlsHideJob?.cancel() - controlsStateMachine.initialize(resumeDialogVisible = resumeDialog != null) - updateViewState(PlayerViewState.Content(contentState.withControlsState(controlsStateMachine.state))) + showInitialContent(contentState, resumeDialog) autoMarkHandledToken = token.takeIf { resolved.isCurrentMediaWatched } episodeSwitchInProgress = false initializePlayer(savedPosition = if (resumeDialog != null) null else 0L) @@ -324,6 +327,25 @@ internal class PlayerVM( loadSkipSegments(item, resolved.seasonNumber, resolved.episodeNumber, token) } + private fun showInitialContent( + contentState: PlayerContentState, + resumeDialog: ResumeDialogState?, + ) { + apiSubtitleTracks = contentState.subtitleTracks + controlsHideJob?.cancel() + controlsStateMachine.initialize(resumeDialogVisible = resumeDialog != null) + updateViewState( + PlayerViewState.Content( + contentState + .copy( + subtitleTracks = contentState.subtitleTracks.filter { it.isOff }, + selectedSubtitleIndex = 0, + ) + .withControlsState(controlsStateMachine.state), + ), + ) + } + private fun isCurrentPrepare(generation: Long): Boolean { return !closing && generation == mediaGeneration } @@ -380,11 +402,15 @@ internal class PlayerVM( applyAudioTrackSelection(audioIndex, persist = false) } - val subtitleIndex = audioTrackPreferenceResolver.findSubtitleTrackIndex( - tracks = content.subtitleTracks, - preferredLang = subtitleLang, - preferredUrl = subtitleUrl, - ) + val subtitleIndex = if (hasDiscoveredSubtitleTracks) { + audioTrackPreferenceResolver.findSubtitleTrackIndex( + tracks = content.subtitleTracks, + preferredLang = subtitleLang, + preferredUrl = subtitleUrl, + ) + } else { + -1 + } if (subtitleIndex >= 0) { applySubtitleSelection(subtitleIndex, persist = false) } @@ -1374,6 +1400,7 @@ internal class PlayerVM( audioLabel = audioTrack?.label?.takeIf { it.isNotEmpty() }, subtitleLang = subtitle?.language?.takeIf { it.isNotEmpty() }, subtitleUrl = subtitle?.url?.takeIf { it.isNotEmpty() } + ?: subtitle?.playerTrackUri?.takeIf { it.isNotEmpty() } ?: subtitle?.playerTrackId?.takeIf { it.isNotEmpty() }, ) } diff --git a/app/src/main/java/com/kino/puber/ui/feature/player/vm/SubtitleTrackIdentity.kt b/app/src/main/java/com/kino/puber/ui/feature/player/vm/SubtitleTrackIdentity.kt index 9e2f8486..c3b92017 100644 --- a/app/src/main/java/com/kino/puber/ui/feature/player/vm/SubtitleTrackIdentity.kt +++ b/app/src/main/java/com/kino/puber/ui/feature/player/vm/SubtitleTrackIdentity.kt @@ -10,3 +10,30 @@ internal fun String.stableSubtitleKey(): String { ?: substringBefore('?').substringBefore('#') return path.substringAfter(SUBTITLES_PATH_MARKER, path) } + +internal fun sameSubtitleIdentity(first: String, second: String): Boolean { + if (first == second) return true + val firstPath = first.subtitleIdentityPathOrNull() + val secondPath = second.subtitleIdentityPathOrNull() + return firstPath != null && secondPath != null && ( + firstPath == secondPath || + firstPath.endsWith("/$secondPath") || + secondPath.endsWith("/$firstPath") || + firstPath.stableSubtitleKey() == secondPath.stableSubtitleKey() + ) +} + +private fun String.subtitleIdentityPathOrNull(): String? { + if (isEmpty()) return null + val path = (runCatching { URI(this).path }.getOrNull() + ?: substringBefore('?').substringBefore('#')) + .trim('/') + return path.takeIf { candidate -> + candidate.contains('/') || SUBTITLE_FILE_EXTENSION.containsMatchIn(candidate) + } +} + +private val SUBTITLE_FILE_EXTENSION = Regex( + pattern = """\.(srt|vtt|webvtt|ass|ssa|ttml|xml)$""", + option = RegexOption.IGNORE_CASE, +) diff --git a/app/src/main/java/com/kino/puber/ui/feature/player/vm/SubtitleTrackMerger.kt b/app/src/main/java/com/kino/puber/ui/feature/player/vm/SubtitleTrackMerger.kt index 1f590e8a..59bc674e 100644 --- a/app/src/main/java/com/kino/puber/ui/feature/player/vm/SubtitleTrackMerger.kt +++ b/app/src/main/java/com/kino/puber/ui/feature/player/vm/SubtitleTrackMerger.kt @@ -18,76 +18,52 @@ internal class SubtitleTrackMerger { ) val apiSubtitles = apiTracks.filterNot { it.isOff } val playerSubtitles = playerTracks.filterNot { it.isOff } - val availablePlayerIndices = playerSubtitles.indices.toMutableSet() - val matches = mutableMapOf() - - apiSubtitles.forEachIndexed { apiIndex, apiTrack -> - findExactIdentityMatch(apiTrack, playerSubtitles, availablePlayerIndices)?.let { playerIndex -> - matches[apiIndex] = playerIndex - availablePlayerIndices.remove(playerIndex) - } + if (playerSubtitles.isEmpty()) { + return listOf(offTrack.copy(index = 0)) } - apiSubtitles.forEachIndexed { apiIndex, apiTrack -> - if (apiIndex in matches || !apiTrack.isEmbedded) return@forEachIndexed - findEmbeddedLanguageMatch(apiTrack, playerSubtitles, availablePlayerIndices)?.let { playerIndex -> - matches[apiIndex] = playerIndex - availablePlayerIndices.remove(playerIndex) - } + val availableApiIndices = apiSubtitles.indices.toMutableSet() + val enrichedPlayerTracks = playerSubtitles.map { playerTrack -> + findExactIdentityMatch(playerTrack, apiSubtitles, availableApiIndices) + ?.let { apiIndex -> + availableApiIndices.remove(apiIndex) + playerTrack.withApiMetadata(apiSubtitles[apiIndex]) + } + ?: playerTrack } - - val merged = buildList { - add(offTrack) - apiSubtitles.forEachIndexed { apiIndex, apiTrack -> - val playerTrack = matches[apiIndex]?.let(playerSubtitles::get) - add(if (playerTrack == null) apiTrack else apiTrack.withPlayerIdentity(playerTrack)) - } - availablePlayerIndices.forEach { playerIndex -> - add(playerSubtitles[playerIndex]) - } + return (listOf(offTrack) + enrichedPlayerTracks).mapIndexed { index, track -> + track.copy(index = index) } - return merged.mapIndexed { index, track -> track.copy(index = index) } } private fun findExactIdentityMatch( - apiTrack: SubtitleTrackUIState, - playerTracks: List, - availablePlayerIndices: Set, + playerTrack: SubtitleTrackUIState, + apiTracks: List, + availableApiIndices: Set, ): Int? { - val apiKey = apiTrack.url.stableSubtitleKey().takeIf { it.isNotEmpty() } ?: return null - return availablePlayerIndices.firstOrNull { playerIndex -> - val playerTrack = playerTracks[playerIndex] - val playerIdKey = playerTrack.playerTrackId - ?.stableSubtitleKey() - ?.takeIf { it.isNotEmpty() } - playerTrack.playerTrackId == apiTrack.url || - playerIdKey == apiKey || - playerTrack.label == apiKey - } + val playerIdentities = listOfNotNull( + playerTrack.playerTrackUri, + playerTrack.playerTrackId, + ).filter { it.isNotEmpty() } + if (playerIdentities.isEmpty()) return null + return availableApiIndices.filter { apiIndex -> + val apiTrack = apiTracks[apiIndex] + val apiIdentities = listOfNotNull(apiTrack.sourceFile, apiTrack.url) + .filter { it.isNotEmpty() } + apiIdentities.any { apiIdentity -> + playerIdentities.any { playerIdentity -> + sameSubtitleIdentity(apiIdentity, playerIdentity) + } + } + }.singleOrNull() } - private fun findEmbeddedLanguageMatch( + private fun SubtitleTrackUIState.withApiMetadata( apiTrack: SubtitleTrackUIState, - playerTracks: List, - availablePlayerIndices: Set, - ): Int? { - val languageMatches = availablePlayerIndices.filter { playerIndex -> - sameSubtitleLanguage(apiTrack.language, playerTracks[playerIndex].language) - } - if (languageMatches.isEmpty()) return null - return apiTrack.isForced?.let { forced -> - languageMatches.firstOrNull { playerTracks[it].isForced == forced } - ?: languageMatches.singleOrNull() - } ?: languageMatches.first() - } - - private fun SubtitleTrackUIState.withPlayerIdentity( - playerTrack: SubtitleTrackUIState, ): SubtitleTrackUIState = copy( - playerTrackId = playerTrack.playerTrackId, - playerGroupIndex = playerTrack.playerGroupIndex, - playerTrackIndex = playerTrack.playerTrackIndex, - isForced = isForced ?: playerTrack.isForced, + url = apiTrack.url, + sourceFile = apiTrack.sourceFile, + isForced = apiTrack.isForced ?: isForced, ) } @@ -96,6 +72,10 @@ internal fun sameSubtitleLanguage(first: String, second: String): Boolean { return canonicalSubtitleLanguage(first) == canonicalSubtitleLanguage(second) } +internal fun subtitleTrackDisplayLabel(language: String, fallbackLabel: String): String { + return canonicalSubtitleLanguage(language).ifEmpty { fallbackLabel } +} + private fun canonicalSubtitleLanguage(language: String): String { val normalized = language .trim() diff --git a/app/src/main/res/values/player.xml b/app/src/main/res/values/player.xml index da220e12..ae00a226 100644 --- a/app/src/main/res/values/player.xml +++ b/app/src/main/res/values/player.xml @@ -14,7 +14,7 @@ АУДИО СУБТИТРЫ Выкл. - %1$s · вариант %2$d + форсированные Субтитры %1$d Аа Размер Маленький diff --git a/app/src/test/kotlin/com/kino/puber/data/api/models/SubtitleLinkTest.kt b/app/src/test/kotlin/com/kino/puber/data/api/models/SubtitleLinkTest.kt index 8aeaf2bc..04156e27 100644 --- a/app/src/test/kotlin/com/kino/puber/data/api/models/SubtitleLinkTest.kt +++ b/app/src/test/kotlin/com/kino/puber/data/api/models/SubtitleLinkTest.kt @@ -51,4 +51,22 @@ internal class SubtitleLinkTest { assertFalse(regularSubtitle.isForced) assertFalse(unforcedSubtitle.isForced) } + + @Test + fun isForced_usesApiFlagInsteadOfUrlHeuristic() { + val forcedSubtitle = SubtitleLink( + lang = "rus", + url = "https://cdn.test/subtitles/russian.srt", + forced = true, + file = "/a/71/29725.srt", + ) + val regularSubtitle = SubtitleLink( + lang = "rus", + url = "https://cdn.test/subtitles/russian-forced.srt", + forced = false, + ) + + assertTrue(forcedSubtitle.isForced) + assertFalse(regularSubtitle.isForced) + } } diff --git a/app/src/test/kotlin/com/kino/puber/ui/feature/player/component/AudioSubtitlesPanelTest.kt b/app/src/test/kotlin/com/kino/puber/ui/feature/player/component/AudioSubtitlesPanelTest.kt new file mode 100644 index 00000000..08a92830 --- /dev/null +++ b/app/src/test/kotlin/com/kino/puber/ui/feature/player/component/AudioSubtitlesPanelTest.kt @@ -0,0 +1,30 @@ +package com.kino.puber.ui.feature.player.component + +import com.kino.puber.ui.feature.player.model.SubtitleTrackUIState +import org.junit.jupiter.api.Assertions.assertEquals +import org.junit.jupiter.api.Test + +internal class AudioSubtitlesPanelTest { + + @Test + fun subtitlePickerLabel_marksForcedTrack_withoutManifestNumbering() { + val track = subtitleTrack(label = "rus", isForced = true) + + assertEquals("rus · форсированные", track.subtitlePickerLabel("форсированные")) + } + + @Test + fun subtitlePickerLabel_keepsRegularTrackLanguageOnly() { + val track = subtitleTrack(label = "rus", isForced = false) + + assertEquals("rus", track.subtitlePickerLabel("форсированные")) + } + + private fun subtitleTrack(label: String, isForced: Boolean) = SubtitleTrackUIState( + index = 1, + label = label, + language = "rus", + url = "", + isForced = isForced, + ) +} diff --git a/app/src/test/kotlin/com/kino/puber/ui/feature/player/model/PlayerUIMapperSubtitleTest.kt b/app/src/test/kotlin/com/kino/puber/ui/feature/player/model/PlayerUIMapperSubtitleTest.kt index f131886c..5370d8a8 100644 --- a/app/src/test/kotlin/com/kino/puber/ui/feature/player/model/PlayerUIMapperSubtitleTest.kt +++ b/app/src/test/kotlin/com/kino/puber/ui/feature/player/model/PlayerUIMapperSubtitleTest.kt @@ -12,12 +12,6 @@ internal class PlayerUIMapperSubtitleTest { private val context = mockk(relaxed = true).also { context -> every { context.getString(R.string.player_subtitles_off) } returns "Off" - every { - context.getString(R.string.player_subtitle_variant_label, any(), any()) - } answers { - val formatArgs = secondArg>() - "${formatArgs[0]} variant ${formatArgs[1]}" - } } private val mapper = PlayerUIMapper(context) @@ -31,13 +25,19 @@ internal class PlayerUIMapperSubtitleTest { } @Test - fun mapSubtitleTracks_preservesLanguagesAndEmbedMetadata_inApiOrder() { + fun mapSubtitleTracks_preservesLanguagesAndIdentityMetadata_inApiOrder() { val embeddedUrl = "https://cdn.test/subtitles/russian.vtt" val externalUrl = "https://cdn.test/subtitles/english.vtt" val result = mapper.mapSubtitleTracks( listOf( - SubtitleLink(lang = "rus", url = embeddedUrl, embed = true), + SubtitleLink( + lang = "rus", + url = embeddedUrl, + embed = true, + forced = false, + file = "/a/71/russian.vtt", + ), SubtitleLink(lang = "eng", url = externalUrl, embed = false), ) ) @@ -45,7 +45,8 @@ internal class PlayerUIMapperSubtitleTest { assertEquals(listOf(0, 1, 2), result.map { it.index }) assertEquals(listOf("", "rus", "eng"), result.map { it.language }) assertEquals(listOf("", embeddedUrl, externalUrl), result.map { it.url }) - assertEquals(listOf(false, true, false), result.map { it.isEmbedded }) + assertEquals(listOf(null, "/a/71/russian.vtt", null), result.map { it.sourceFile }) + assertEquals(listOf(null, false, null), result.map { it.isForced }) } @Test @@ -56,18 +57,26 @@ internal class PlayerUIMapperSubtitleTest { lang = "rus", url = "https://cdn.test/subtitles/russian-full.vtt", embed = true, + forced = false, ), SubtitleLink( lang = "rus", url = "https://cdn.test/subtitles/russian-forced.vtt", embed = true, + forced = true, ), ) ) - assertEquals(listOf("rus variant 1", "rus variant 2"), result.drop(1).map { it.label }) + assertEquals(listOf("rus", "rus"), result.drop(1).map { it.label }) assertEquals(listOf("rus", "rus"), result.drop(1).map { it.language }) - assertEquals(listOf(true, true), result.drop(1).map { it.isEmbedded }) + assertEquals( + listOf( + "https://cdn.test/subtitles/russian-full.vtt", + "https://cdn.test/subtitles/russian-forced.vtt", + ), + result.drop(1).map { it.url }, + ) assertEquals(listOf(false, true), result.drop(1).map { it.isForced }) } } diff --git a/app/src/test/kotlin/com/kino/puber/ui/feature/player/vm/AudioTrackPreferenceResolverTest.kt b/app/src/test/kotlin/com/kino/puber/ui/feature/player/vm/AudioTrackPreferenceResolverTest.kt index 5cd72c62..84008a80 100644 --- a/app/src/test/kotlin/com/kino/puber/ui/feature/player/vm/AudioTrackPreferenceResolverTest.kt +++ b/app/src/test/kotlin/com/kino/puber/ui/feature/player/vm/AudioTrackPreferenceResolverTest.kt @@ -112,6 +112,75 @@ internal class AudioTrackPreferenceResolverTest { assertEquals(2, result) } + @Test + fun findSubtitleTrackIndex_usesPlayerCoordinates_whenSelectedFormatLosesItsId() { + val tracks = listOf( + subtitleTrack(index = 0, language = "", url = ""), + subtitleTrack(index = 1, language = "eng", url = "", groupIndex = 5), + subtitleTrack(index = 2, language = "eng", url = "", groupIndex = 6), + subtitleTrack(index = 3, language = "eng", url = "", groupIndex = 7), + ) + + val result = resolver.findSubtitleTrackIndex( + tracks = tracks, + preferredLang = "eng", + preferredUrl = "", + preferredPlayerTrackId = "manifest-id-that-disappeared", + preferredPlayerGroupIndex = 6, + preferredPlayerTrackIndex = 0, + ) + + assertEquals(2, result) + } + + @Test + fun findSubtitleTrackIndex_matchesSavedManifestUri_whenSignedTokenChanges() { + val tracks = listOf( + subtitleTrack(index = 0, language = "", url = ""), + subtitleTrack( + index = 1, + language = "rus", + url = "", + playerTrackUri = "https://new.test/pd/subtitle/a/71/russian.srt?token=fresh", + ), + ) + + val result = resolver.findSubtitleTrackIndex( + tracks = tracks, + preferredLang = "rus", + preferredUrl = "https://old.test/pd/subtitle/a/71/russian.srt?token=expired", + ) + + assertEquals(1, result) + } + + @Test + fun findSubtitleTrackIndex_doesNotGuessWhenStableIdentityMatchesMultipleTracks() { + val tracks = listOf( + subtitleTrack(index = 0, language = "", url = ""), + subtitleTrack( + index = 1, + language = "rus", + url = "", + playerTrackUri = "https://cdn.test/a/russian.srt", + ), + subtitleTrack( + index = 2, + language = "rus", + url = "", + playerTrackUri = "https://cdn.test/b/russian.srt", + ), + ) + + val result = resolver.findSubtitleTrackIndex( + tracks = tracks, + preferredLang = "rus", + preferredUrl = "russian.srt", + ) + + assertEquals(-1, result) + } + @Test fun findSubtitleTrackIndex_prefersSavedUrl_overCurrentPlayerIdentity() { val externalUrl = "https://cdn.test/subtitles/russian.vtt" @@ -148,6 +217,23 @@ internal class AudioTrackPreferenceResolverTest { assertEquals(2, result) } + @Test + fun findSubtitleTrackIndex_keepsHashSuffixInOpaqueManifestIdentity() { + val tracks = listOf( + subtitleTrack(index = 0, language = "", url = ""), + subtitleTrack(index = 1, language = "rus", url = "", playerTrackId = "subs:Russian #02"), + subtitleTrack(index = 2, language = "rus", url = "", playerTrackId = "subs:Russian #03"), + ) + + val result = resolver.findSubtitleTrackIndex( + tracks = tracks, + preferredLang = "rus", + preferredUrl = "subs:Russian #03", + ) + + assertEquals(2, result) + } + @Test fun findSubtitleTrackIndex_doesNotGuessBetweenSameLanguageVariants() { val tracks = listOf( @@ -186,13 +272,17 @@ internal class AudioTrackPreferenceResolverTest { language: String, url: String, playerTrackId: String? = null, + playerTrackUri: String? = null, + groupIndex: Int? = playerTrackId?.let { index - 1 }, + trackIndex: Int? = groupIndex?.let { 0 }, ) = SubtitleTrackUIState( index = index, label = "Track $index", language = language, url = url, playerTrackId = playerTrackId, - playerGroupIndex = playerTrackId?.let { index - 1 }, - playerTrackIndex = playerTrackId?.let { 0 }, + playerTrackUri = playerTrackUri, + playerGroupIndex = groupIndex, + playerTrackIndex = trackIndex, ) } diff --git a/app/src/test/kotlin/com/kino/puber/ui/feature/player/vm/PlayerVMSubtitleVariantTest.kt b/app/src/test/kotlin/com/kino/puber/ui/feature/player/vm/PlayerVMSubtitleVariantTest.kt index 2e794454..ee7f1049 100644 --- a/app/src/test/kotlin/com/kino/puber/ui/feature/player/vm/PlayerVMSubtitleVariantTest.kt +++ b/app/src/test/kotlin/com/kino/puber/ui/feature/player/vm/PlayerVMSubtitleVariantTest.kt @@ -1,6 +1,8 @@ package com.kino.puber.ui.feature.player.vm import com.kino.puber.ui.feature.player.model.AudioTrackUIState +import com.kino.puber.ui.feature.player.model.PlayerAction +import com.kino.puber.ui.feature.player.model.SubtitleTrackUIState import com.kino.puber.util.MainDispatcherExtension import io.mockk.every import io.mockk.verify @@ -16,6 +18,82 @@ internal class PlayerVMSubtitleVariantTest : PlayerVMTestFixture() { val mainDispatcher = MainDispatcherExtension() } + @Test + fun tracksUpdated_addsAndSelectsManifestOnlySubtitle_withLanguagePreference() { + val vm = startedVM() + val audioTracks = listOf(AudioTrackUIState(0, "English", "eng")) + val manifestTrack = testSubtitleTracks.first().copy( + index = 1, + label = "Ukrainian HLS", + language = "uk", + playerTrackId = "hls-ukrainian", + playerTrackUri = "https://cdn.test/subtitle/ukrainian.vtt", + playerGroupIndex = 0, + playerTrackIndex = 0, + ) + + callbackSlot.captured.onTracksUpdated(audioTracks, 0, listOf(manifestTrack)) + vm.onAction(PlayerAction.SelectSubtitle(1)) + + val selectedTrack = contentState(vm).subtitleTracks[1] + assertEquals("uk", selectedTrack.language) + assertEquals("hls-ukrainian", selectedTrack.playerTrackId) + verify { playbackController.selectSubtitle(selectedTrack) } + verify { + interactor.saveTrackPreferences( + 42, + "eng", + "English", + "uk", + "https://cdn.test/subtitle/ukrainian.vtt", + ) + } + } + + @Test + fun tracksUpdated_defersUrlLessLanguagePreference_untilManifestTracksAppear() { + every { interactor.getPreferredSubtitleLang(42) } returns "ukr" + every { interactor.getPreferredSubtitleUrl(42) } returns "" + val vm = startedVM() + val audioTracks = listOf(AudioTrackUIState(0, "English", "eng")) + val manifestTrack = testSubtitleTracks.first().copy( + index = 1, + label = "Ukrainian HLS", + language = "uk", + playerTrackId = "hls-ukrainian", + playerGroupIndex = 0, + playerTrackIndex = 0, + ) + + callbackSlot.captured.onTracksUpdated(audioTracks, 0, emptyList()) + verify(exactly = 0) { playbackController.selectSubtitle(any()) } + + callbackSlot.captured.onTracksUpdated(audioTracks, 0, listOf(manifestTrack)) + + val selectedTrack = contentState(vm).subtitleTracks[1] + assertEquals(1, contentState(vm).selectedSubtitleIndex) + assertEquals("hls-ukrainian", selectedTrack.playerTrackId) + verify { playbackController.selectSubtitle(selectedTrack) } + } + + @Test + fun tracksUpdated_defersUrlPreference_untilPlayerTrackAppears() { + every { interactor.getPreferredSubtitleLang(42) } returns "rus" + every { interactor.getPreferredSubtitleUrl(42) } returns + "https://test/subtitles/rus-forced.vtt" + val vm = startedVM() + val audioTracks = listOf(AudioTrackUIState(0, "English", "eng")) + + callbackSlot.captured.onTracksUpdated(audioTracks, 0, emptyList()) + verify(exactly = 0) { playbackController.selectSubtitle(any()) } + + callbackSlot.captured.onTracksUpdated(audioTracks, 0, testDiscoveredSubtitleTracks) + + val selectedTrack = contentState(vm).subtitleTracks[2] + assertEquals(2, contentState(vm).selectedSubtitleIndex) + verify { playbackController.selectSubtitle(selectedTrack) } + } + @Test fun tracksUpdated_restoresForcedManifestSubtitleBySavedIdentity() { every { interactor.getPreferredSubtitleLang(42) } returns "rus" @@ -48,9 +126,64 @@ internal class PlayerVMSubtitleVariantTest : PlayerVMTestFixture() { callbackSlot.captured.onTracksUpdated(audioTracks, 0, manifestTracks) - val selectedTrack = contentState(vm).subtitleTracks[4] + val selectedTrack = contentState(vm).subtitleTracks[2] assertEquals("hls-russian-forced", selectedTrack.playerTrackId) - assertEquals(4, contentState(vm).selectedSubtitleIndex) + assertEquals(2, contentState(vm).selectedSubtitleIndex) verify { playbackController.selectSubtitle(selectedTrack) } } + + @Test + fun tracksUpdated_keepsSelectedVariant_whenSelectedFormatIdDisappears() { + val vm = startedVM() + val audioTracks = listOf(AudioTrackUIState(0, "English", "eng")) + val manifestTracks = listOf( + manifestTrack(1, "English 1", "manifest-eng-1", groupIndex = 5), + manifestTrack(2, "English 2", "manifest-eng-2", groupIndex = 6), + manifestTrack(3, "English 3", "manifest-eng-3", groupIndex = 7), + ) + callbackSlot.captured.onTracksUpdated(audioTracks, 0, manifestTracks) + vm.onAction(PlayerAction.SelectSubtitle(2)) + + callbackSlot.captured.onTracksUpdated( + audioTracks, + 0, + manifestTracks.map { it.copy(playerTrackId = null, playerTrackUri = null) }, + ) + + assertEquals(2, contentState(vm).selectedSubtitleIndex) + assertEquals(6, contentState(vm).subtitleTracks[2].playerGroupIndex) + } + + @Test + fun tracksUpdated_keepsSelectedVariant_duringTransientEmptyTrackUpdate() { + val vm = startedVM() + val audioTracks = listOf(AudioTrackUIState(0, "English", "eng")) + val manifestTracks = listOf( + manifestTrack(1, "English 1", "manifest-eng-1", groupIndex = 5), + manifestTrack(2, "English 2", "manifest-eng-2", groupIndex = 6), + ) + callbackSlot.captured.onTracksUpdated(audioTracks, 0, manifestTracks) + vm.onAction(PlayerAction.SelectSubtitle(2)) + + callbackSlot.captured.onTracksUpdated(audioTracks, 0, emptyList()) + + assertEquals(2, contentState(vm).selectedSubtitleIndex) + assertEquals("manifest-eng-2", contentState(vm).subtitleTracks[2].playerTrackId) + } + + private fun manifestTrack( + index: Int, + label: String, + id: String, + groupIndex: Int, + ) = SubtitleTrackUIState( + index = index, + label = label, + language = "en", + url = "", + playerTrackId = id, + playerTrackUri = "https://cdn.test/subtitle/$id.vtt", + playerGroupIndex = groupIndex, + playerTrackIndex = 0, + ) } diff --git a/app/src/test/kotlin/com/kino/puber/ui/feature/player/vm/PlayerVMTest.kt b/app/src/test/kotlin/com/kino/puber/ui/feature/player/vm/PlayerVMTest.kt index 28efd614..4439626c 100644 --- a/app/src/test/kotlin/com/kino/puber/ui/feature/player/vm/PlayerVMTest.kt +++ b/app/src/test/kotlin/com/kino/puber/ui/feature/player/vm/PlayerVMTest.kt @@ -718,9 +718,10 @@ internal class PlayerVMTest : PlayerVMTestFixture() { @Test fun selectSubtitle_updatesStateAndDelegates() { val vm = startedVM() + callbackSlot.captured.onTracksUpdated(testContentState.audioTracks, 0, testDiscoveredSubtitleTracks) vm.onAction(PlayerAction.SelectSubtitle(1)) assertEquals(1, contentState(vm).selectedSubtitleIndex) - verify { playbackController.selectSubtitle(testSubtitleTracks[1]) } + verify { playbackController.selectSubtitle(contentState(vm).subtitleTracks[1]) } } @Test @@ -731,55 +732,6 @@ internal class PlayerVMTest : PlayerVMTestFixture() { verify { playbackController.selectSubtitle(testSubtitleTracks[0]) } } - @Test - fun tracksUpdated_addsAndSelectsManifestOnlySubtitle_withLanguagePreference() { - val vm = startedVM() - val audioTracks = listOf(AudioTrackUIState(0, "English", "eng")) - val manifestTrack = testSubtitleTracks.first().copy( - index = 1, - label = "Ukrainian HLS", - language = "uk", - playerTrackId = "hls-ukrainian", - playerGroupIndex = 0, - playerTrackIndex = 0, - ) - - callbackSlot.captured.onTracksUpdated(audioTracks, 0, listOf(manifestTrack)) - vm.onAction(PlayerAction.SelectSubtitle(3)) - - val selectedTrack = contentState(vm).subtitleTracks[3] - assertEquals("uk", selectedTrack.language) - assertEquals("hls-ukrainian", selectedTrack.playerTrackId) - verify { playbackController.selectSubtitle(selectedTrack) } - verify { interactor.saveTrackPreferences(42, "eng", "English", "uk", "hls-ukrainian") } - } - - @Test - fun tracksUpdated_defersUrlLessLanguagePreference_untilManifestTracksAppear() { - every { interactor.getPreferredSubtitleLang(42) } returns "ukr" - every { interactor.getPreferredSubtitleUrl(42) } returns "" - val vm = startedVM() - val audioTracks = listOf(AudioTrackUIState(0, "English", "eng")) - val manifestTrack = testSubtitleTracks.first().copy( - index = 1, - label = "Ukrainian HLS", - language = "uk", - playerTrackId = "hls-ukrainian", - playerGroupIndex = 0, - playerTrackIndex = 0, - ) - - callbackSlot.captured.onTracksUpdated(audioTracks, 0, emptyList()) - verify(exactly = 0) { playbackController.selectSubtitle(any()) } - - callbackSlot.captured.onTracksUpdated(audioTracks, 0, listOf(manifestTrack)) - - val selectedTrack = contentState(vm).subtitleTracks[3] - assertEquals(3, contentState(vm).selectedSubtitleIndex) - assertEquals("hls-ukrainian", selectedTrack.playerTrackId) - verify { playbackController.selectSubtitle(selectedTrack) } - } - @Test fun tracksUpdated_restoresPreferredSubtitleByUrl_beforeLanguage() { every { interactor.getPreferredSubtitleLang(42) } returns "rus" @@ -787,9 +739,9 @@ internal class PlayerVMTest : PlayerVMTestFixture() { val vm = startedVM() val tracks = listOf(AudioTrackUIState(0, "English", "eng"), AudioTrackUIState(1, "Russian", "rus")) - callbackSlot.captured.onTracksUpdated(tracks, 0) + callbackSlot.captured.onTracksUpdated(tracks, 0, testDiscoveredSubtitleTracks) - verify { playbackController.selectSubtitle(testSubtitleTracks[2]) } + verify { playbackController.selectSubtitle(contentState(vm).subtitleTracks[2]) } assertEquals(2, contentState(vm).selectedSubtitleIndex) } @@ -801,9 +753,9 @@ internal class PlayerVMTest : PlayerVMTestFixture() { val vm = startedVM() val tracks = listOf(AudioTrackUIState(0, "English", "eng"), AudioTrackUIState(1, "Russian", "rus")) - callbackSlot.captured.onTracksUpdated(tracks, 0) + callbackSlot.captured.onTracksUpdated(tracks, 0, testDiscoveredSubtitleTracks) - verify { playbackController.selectSubtitle(testSubtitleTracks[2]) } + verify { playbackController.selectSubtitle(contentState(vm).subtitleTracks[2]) } assertEquals(2, contentState(vm).selectedSubtitleIndex) } @@ -814,7 +766,7 @@ internal class PlayerVMTest : PlayerVMTestFixture() { val vm = startedVM() val tracks = listOf(AudioTrackUIState(0, "English", "eng"), AudioTrackUIState(1, "Russian", "rus")) - callbackSlot.captured.onTracksUpdated(tracks, 0) + callbackSlot.captured.onTracksUpdated(tracks, 0, testDiscoveredSubtitleTracks) verify(exactly = 0) { playbackController.selectSubtitle(any()) } assertEquals(0, contentState(vm).selectedSubtitleIndex) @@ -828,10 +780,10 @@ internal class PlayerVMTest : PlayerVMTestFixture() { val vm = startedVM() val tracks = listOf(AudioTrackUIState(0, "English", "eng"), AudioTrackUIState(1, "Russian", "rus")) - callbackSlot.captured.onTracksUpdated(tracks, 0) + callbackSlot.captured.onTracksUpdated(tracks, 0, testDiscoveredSubtitleTracks) verify { playbackController.selectAudioTrack(1) } - verify { playbackController.selectSubtitle(testSubtitleTracks[2]) } + verify { playbackController.selectSubtitle(contentState(vm).subtitleTracks[2]) } verify(exactly = 0) { interactor.saveTrackPreferences(any(), any(), any(), any(), any()) } assertEquals(1, contentState(vm).selectedAudioTrackIndex) assertEquals(2, contentState(vm).selectedSubtitleIndex) diff --git a/app/src/test/kotlin/com/kino/puber/ui/feature/player/vm/PlayerVMTestFixture.kt b/app/src/test/kotlin/com/kino/puber/ui/feature/player/vm/PlayerVMTestFixture.kt index b8a5971f..c23518d8 100644 --- a/app/src/test/kotlin/com/kino/puber/ui/feature/player/vm/PlayerVMTestFixture.kt +++ b/app/src/test/kotlin/com/kino/puber/ui/feature/player/vm/PlayerVMTestFixture.kt @@ -200,6 +200,17 @@ internal abstract class PlayerVMTestFixture { ), ) + protected val testDiscoveredSubtitleTracks = testSubtitleTracks.drop(1).mapIndexed { index, track -> + track.copy( + index = index + 1, + url = "", + playerTrackId = track.url, + playerTrackUri = track.url, + playerGroupIndex = index, + playerTrackIndex = 0, + ) + } + protected val testContentState = PlayerContentState( title = "Breaking Bad", subtitle = "S1E1", diff --git a/app/src/test/kotlin/com/kino/puber/ui/feature/player/vm/SubtitleTrackMergerTest.kt b/app/src/test/kotlin/com/kino/puber/ui/feature/player/vm/SubtitleTrackMergerTest.kt index 0fef693a..4b5f1450 100644 --- a/app/src/test/kotlin/com/kino/puber/ui/feature/player/vm/SubtitleTrackMergerTest.kt +++ b/app/src/test/kotlin/com/kino/puber/ui/feature/player/vm/SubtitleTrackMergerTest.kt @@ -11,7 +11,14 @@ internal class SubtitleTrackMergerTest { private val merger = SubtitleTrackMerger() @Test - fun merge_matchesExactExternalIdentity_beforeEmbeddedLanguage() { + fun subtitleTrackDisplayLabel_hidesManifestNumberingAndUsesLowercaseIso3Language() { + assertEquals("rus", subtitleTrackDisplayLabel("RU", "RUS #03")) + assertEquals("spa", subtitleTrackDisplayLabel("es-ES", "SPA #01")) + assertEquals("Unknown", subtitleTrackDisplayLabel("", "Unknown")) + } + + @Test + fun merge_usesPlayerTracksAsBackbone_andEnrichesExactIdentity() { val apiTracks = listOf( offTrack(), apiTrack( @@ -19,14 +26,12 @@ internal class SubtitleTrackMergerTest { label = "Russian embedded", language = "rus", url = "https://api.test/subtitles/embedded-rus.srt", - embedded = true, ), apiTrack( index = 2, label = "Russian external", language = "rus", url = "https://api.test/subtitles/external-rus.srt", - embedded = false, ), ) val playerTracks = listOf( @@ -49,20 +54,21 @@ internal class SubtitleTrackMergerTest { val result = merger.merge(apiTracks, playerTracks) assertEquals( - listOf("Off", "Russian embedded", "Russian external"), + listOf("Off", "external-rus.srt", "Русские полные"), result.map { it.label }, ) - assertEquals("hls-russian-full", result[1].playerTrackId) - assertEquals("rus", result[1].language) - assertEquals("external-rus.srt", result[2].playerTrackId) + assertEquals("external-rus.srt", result[1].playerTrackId) + assertEquals("https://api.test/subtitles/external-rus.srt", result[1].url) + assertEquals("hls-russian-full", result[2].playerTrackId) + assertEquals("ru", result[2].language) } @Test - fun merge_matchesEmbeddedVariants_byForcedMetadata() { + fun merge_doesNotPairSameLanguageVariants_withoutExactIdentity() { val apiTracks = listOf( offTrack(), - apiTrack(1, "Russian full", "rus", embedded = true, forced = false), - apiTrack(2, "Russian forced", "rus", embedded = true, forced = true), + apiTrack(1, "Russian full", "rus", forced = false), + apiTrack(2, "Russian forced", "rus", forced = true), ) val playerTracks = listOf( playerTrack(1, "Russian forced HLS", "ru", "forced", 0, forced = true), @@ -71,10 +77,11 @@ internal class SubtitleTrackMergerTest { val result = merger.merge(apiTracks, playerTracks) - assertEquals("full", result[1].playerTrackId) - assertFalse(result[1].isForced!!) - assertEquals("forced", result[2].playerTrackId) - assertTrue(result[2].isForced!!) + assertEquals(listOf("Off", "Russian forced HLS", "Russian full HLS"), result.map { it.label }) + assertEquals("forced", result[1].playerTrackId) + assertTrue(result[1].isForced!!) + assertEquals("full", result[2].playerTrackId) + assertFalse(result[2].isForced!!) } @Test @@ -96,7 +103,7 @@ internal class SubtitleTrackMergerTest { } @Test - fun merge_doesNotCollapseExternalAndManifestTracks_byLanguageAlone() { + fun merge_dropsUnmatchedApiTrack_whenPlayerTracksAreAvailable() { val apiTracks = listOf( offTrack(), apiTrack( @@ -104,7 +111,6 @@ internal class SubtitleTrackMergerTest { label = "Russian external", language = "rus", url = "https://api.test/subtitles/external.srt", - embedded = false, ), ) val playerTracks = listOf( @@ -113,9 +119,107 @@ internal class SubtitleTrackMergerTest { val result = merger.merge(apiTracks, playerTracks) - assertEquals(listOf("Off", "Russian external", "Russian HLS"), result.map { it.label }) - assertEquals("https://api.test/subtitles/external.srt", result[1].url) - assertEquals("hls-russian", result[2].playerTrackId) + assertEquals(listOf("Off", "Russian HLS"), result.map { it.label }) + assertEquals("", result[1].url) + assertEquals("hls-russian", result[1].playerTrackId) + } + + @Test + fun merge_doesNotTreatDisplayLabelAsTrackIdentity() { + val apiTrack = apiTrack( + index = 1, + label = "Spanish API", + language = "spa", + url = "https://api.test/subtitles/spanish.srt", + ) + val playerTrack = playerTrack( + index = 1, + label = "spanish.srt", + language = "rus", + id = "hls-russian", + groupIndex = 0, + ) + + val result = merger.merge(listOf(offTrack(), apiTrack), listOf(playerTrack)) + + assertEquals("", result[1].url) + assertEquals("rus", result[1].language) + assertEquals("hls-russian", result[1].playerTrackId) + } + + @Test + fun merge_keepsManifestOrder_forSameLanguageVariants() { + val apiTracks = listOf( + offTrack(), + apiTrack(1, "rus #1", "rus"), + apiTrack(2, "rus #2", "rus"), + apiTrack(3, "rus #3", "rus"), + apiTrack(4, "spa", "spa"), + ) + val playerTracks = listOf( + playerTrack(1, "Russian 1", "ru", "rus-1", 0), + playerTrack(2, "Spanish", "es", "spa", 1), + playerTrack(3, "Russian 2", "ru", "rus-2", 2), + playerTrack(4, "Russian 3", "ru", "rus-3", 3), + ) + + val result = merger.merge(apiTracks, playerTracks) + + assertEquals( + listOf("Off", "Russian 1", "Spanish", "Russian 2", "Russian 3"), + result.map { it.label }, + ) + assertEquals(listOf(null, 0, 1, 2, 3), result.map { it.playerGroupIndex }) + assertEquals("rus-3", result[4].playerTrackId) + } + + @Test + fun merge_usesApiFilePathToMatchHlsRenditionUri_whenLanguageOrderDiffers() { + val apiTracks = listOf( + offTrack(), + apiTrack( + index = 1, + label = "rus #1", + language = "rus", + sourceFile = "/a/71/first.srt", + ), + apiTrack( + index = 2, + label = "rus #2", + language = "rus", + sourceFile = "/b/82/second.srt", + ), + ) + val playerTracks = listOf( + playerTrack( + index = 1, + label = "Russian 2", + language = "ru", + id = "subs:Russian #02", + groupIndex = 0, + uri = "https://cdn.test/pd/subtitle/token/b/82/second.srt", + ), + playerTrack( + index = 2, + label = "Russian 1", + language = "ru", + id = "subs:Russian #01", + groupIndex = 1, + uri = "https://cdn.test/pd/subtitle/token/a/71/first.srt", + ), + ) + + val result = merger.merge(apiTracks, playerTracks) + + assertEquals(listOf(0, 1), result.drop(1).map { it.playerGroupIndex }) + assertEquals( + listOf("subs:Russian #02", "subs:Russian #01"), + result.drop(1).map { it.playerTrackId }, + ) + assertEquals( + listOf("/b/82/second.srt", "/a/71/first.srt"), + result.drop(1).map { it.sourceFile }, + ) } @Test @@ -125,7 +229,6 @@ internal class SubtitleTrackMergerTest { label = "Russian external", language = "rus", url = "https://old-cdn.test/subtitles/russian.vtt?token=expired", - embedded = false, ) val playerTrack = playerTrack( index = 1, @@ -137,18 +240,18 @@ internal class SubtitleTrackMergerTest { val result = merger.merge(listOf(offTrack(), apiTrack), listOf(playerTrack)) - assertEquals(listOf("Off", "Russian external"), result.map { it.label }) + assertEquals(listOf("Off", "russian.vtt"), result.map { it.label }) assertEquals(playerTrack.playerTrackId, result[1].playerTrackId) - assertEquals("rus", result[1].language) + assertEquals("ru", result[1].language) + assertEquals(apiTrack.url, result[1].url) } @Test - fun merge_keepsUnmatchedEmbeddedAndManifestTracks_withoutFalseLanguageMatch() { + fun merge_usesOnlyManifestTracks_whenApiIdentityDoesNotMatch() { val embeddedRussian = apiTrack( index = 1, label = "Russian embedded", language = "rus", - embedded = true, ) val manifestEnglish = playerTrack( index = 1, @@ -163,10 +266,23 @@ internal class SubtitleTrackMergerTest { listOf(manifestEnglish), ) - assertEquals(listOf("Off", "Russian embedded", "English HLS"), result.map { it.label }) - assertEquals(listOf("", "rus", "en"), result.map { it.language }) - assertEquals(null, result[1].playerTrackId) - assertEquals("hls-english", result[2].playerTrackId) + assertEquals(listOf("Off", "English HLS"), result.map { it.label }) + assertEquals(listOf("", "en"), result.map { it.language }) + assertEquals("hls-english", result[1].playerTrackId) + } + + @Test + fun merge_exposesOnlyOff_untilPlayerTracksAreDiscovered() { + val apiTracks = listOf( + offTrack(), + apiTrack(1, "Russian full", "rus"), + apiTrack(2, "Russian forced", "rus"), + ) + + val result = merger.merge(apiTracks, emptyList()) + + assertEquals(listOf("Off"), result.map { it.label }) + assertEquals(listOf(0), result.map { it.index }) } private fun offTrack() = SubtitleTrackUIState( @@ -181,15 +297,15 @@ internal class SubtitleTrackMergerTest { label: String, language: String, url: String = "", - embedded: Boolean, forced: Boolean? = null, + sourceFile: String? = null, ) = SubtitleTrackUIState( index = index, label = label, language = language, url = url, - isEmbedded = embedded, isForced = forced, + sourceFile = sourceFile, ) private fun playerTrack( @@ -199,14 +315,15 @@ internal class SubtitleTrackMergerTest { id: String, groupIndex: Int, forced: Boolean = false, + uri: String? = null, ) = SubtitleTrackUIState( index = index, label = label, language = language, url = "", - isEmbedded = true, isForced = forced, playerTrackId = id, + playerTrackUri = uri, playerGroupIndex = groupIndex, playerTrackIndex = 0, ) From 18fbef59fd4c50d669c5d72936575c7151688c70 Mon Sep 17 00:00:00 2001 From: Innokentii Enikeev Date: Fri, 28 Aug 2026 07:44:19 +0200 Subject: [PATCH 04/25] Use compact partial subtitle label --- app/src/main/res/values/player.xml | 2 +- .../ui/feature/player/component/AudioSubtitlesPanelTest.kt | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/app/src/main/res/values/player.xml b/app/src/main/res/values/player.xml index ae00a226..6c571c48 100644 --- a/app/src/main/res/values/player.xml +++ b/app/src/main/res/values/player.xml @@ -14,7 +14,7 @@ АУДИО СУБТИТРЫ Выкл. - форсированные + частичные Субтитры %1$d Аа Размер Маленький diff --git a/app/src/test/kotlin/com/kino/puber/ui/feature/player/component/AudioSubtitlesPanelTest.kt b/app/src/test/kotlin/com/kino/puber/ui/feature/player/component/AudioSubtitlesPanelTest.kt index 08a92830..1844f21d 100644 --- a/app/src/test/kotlin/com/kino/puber/ui/feature/player/component/AudioSubtitlesPanelTest.kt +++ b/app/src/test/kotlin/com/kino/puber/ui/feature/player/component/AudioSubtitlesPanelTest.kt @@ -10,14 +10,14 @@ internal class AudioSubtitlesPanelTest { fun subtitlePickerLabel_marksForcedTrack_withoutManifestNumbering() { val track = subtitleTrack(label = "rus", isForced = true) - assertEquals("rus · форсированные", track.subtitlePickerLabel("форсированные")) + assertEquals("rus · частичные", track.subtitlePickerLabel("частичные")) } @Test fun subtitlePickerLabel_keepsRegularTrackLanguageOnly() { val track = subtitleTrack(label = "rus", isForced = false) - assertEquals("rus", track.subtitlePickerLabel("форсированные")) + assertEquals("rus", track.subtitlePickerLabel("частичные")) } private fun subtitleTrack(label: String, isForced: Boolean) = SubtitleTrackUIState( From c68613738bcb110f2bb80f36e3ade45df7b08066 Mon Sep 17 00:00:00 2001 From: Innokentii Enikeev Date: Sat, 29 Aug 2026 10:37:55 +0200 Subject: [PATCH 05/25] Restrict HLS detection to the stream path Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01GEBLAtGvP5AHqHemWgLXAp --- .../feature/player/vm/PlaybackController.kt | 15 ++++++- .../ui/feature/player/vm/HlsStreamUrlTest.kt | 40 +++++++++++++++++++ 2 files changed, 53 insertions(+), 2 deletions(-) create mode 100644 app/src/test/kotlin/com/kino/puber/ui/feature/player/vm/HlsStreamUrlTest.kt diff --git a/app/src/main/java/com/kino/puber/ui/feature/player/vm/PlaybackController.kt b/app/src/main/java/com/kino/puber/ui/feature/player/vm/PlaybackController.kt index 3be6754c..0aa72a52 100644 --- a/app/src/main/java/com/kino/puber/ui/feature/player/vm/PlaybackController.kt +++ b/app/src/main/java/com/kino/puber/ui/feature/player/vm/PlaybackController.kt @@ -36,6 +36,7 @@ import com.kino.puber.ui.feature.player.model.AudioTrackUIState import com.kino.puber.ui.feature.player.model.BufferPreset import com.kino.puber.ui.feature.player.model.SubtitleTrackUIState import com.kino.puber.ui.feature.player.model.isOff +import java.net.URI import java.util.Locale import okhttp3.OkHttpClient @@ -767,6 +768,16 @@ internal class PlaybackController( } } -private fun String.isHlsStreamUrl(): Boolean { - return contains(".m3u8", ignoreCase = true) || contains("hls", ignoreCase = true) +// Matches only the URL path: a host such as "hls.cdn.example" or a query +// parameter must not turn a progressive stream into an HLS one, because the +// answer also decides whether API subtitles are side-loaded. +internal fun String.isHlsStreamUrl(): Boolean { + val path = runCatching { URI(this).path }.getOrNull() + ?: substringBefore('?').substringBefore('#').substringAfter("://").substringAfter('/', "") + if (path.isEmpty()) return false + if (path.endsWith(M3U8_EXTENSION, ignoreCase = true)) return true + return path.split('/').any { segment -> HLS_PATH_SEGMENT.matches(segment) } } + +private const val M3U8_EXTENSION = ".m3u8" +private val HLS_PATH_SEGMENT = Regex("""hls\d*""", RegexOption.IGNORE_CASE) diff --git a/app/src/test/kotlin/com/kino/puber/ui/feature/player/vm/HlsStreamUrlTest.kt b/app/src/test/kotlin/com/kino/puber/ui/feature/player/vm/HlsStreamUrlTest.kt new file mode 100644 index 00000000..7ea1fb17 --- /dev/null +++ b/app/src/test/kotlin/com/kino/puber/ui/feature/player/vm/HlsStreamUrlTest.kt @@ -0,0 +1,40 @@ +package com.kino.puber.ui.feature.player.vm + +import org.junit.jupiter.api.Assertions.assertFalse +import org.junit.jupiter.api.Assertions.assertTrue +import org.junit.jupiter.api.Test + +internal class HlsStreamUrlTest { + + @Test + fun isHlsStreamUrl_detectsPlaylistExtension_evenWithQueryAndFragment() { + assertTrue("https://cdn.example/video/master.m3u8".isHlsStreamUrl()) + assertTrue("https://cdn.example/video/master.M3U8?token=abc#t=10".isHlsStreamUrl()) + } + + @Test + fun isHlsStreamUrl_detectsHlsPathSegment() { + assertTrue("https://cdn.example/a/hls/index".isHlsStreamUrl()) + assertTrue("https://cdn.example/a/hls4/index".isHlsStreamUrl()) + } + + @Test + fun isHlsStreamUrl_ignoresHostAndQueryMatches() { + assertFalse("https://hls.cdn.example/video/file.mp4".isHlsStreamUrl()) + assertFalse("https://cdn.example/video/file.mp4?profile=hls".isHlsStreamUrl()) + assertFalse("https://cdn.example/video/file.mp4#hls".isHlsStreamUrl()) + } + + @Test + fun isHlsStreamUrl_ignoresPartialWordMatchesInPath() { + assertFalse("https://cdn.example/hlsx/file.mp4".isHlsStreamUrl()) + assertFalse("https://cdn.example/nothls/file.mp4".isHlsStreamUrl()) + } + + @Test + fun isHlsStreamUrl_handlesMalformedUrlsWithoutThrowing() { + assertTrue("https://cdn.example/a b/master.m3u8".isHlsStreamUrl()) + assertFalse("not a url".isHlsStreamUrl()) + assertFalse("".isHlsStreamUrl()) + } +} From 333eab945ea5f7928e6b8879221a2e3dcf147718 Mon Sep 17 00:00:00 2001 From: Innokentii Enikeev Date: Sat, 29 Aug 2026 10:38:04 +0200 Subject: [PATCH 06/25] Disambiguate same-language subtitle labels Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01GEBLAtGvP5AHqHemWgLXAp --- .../ui/feature/player/model/PlayerUIModels.kt | 2 + .../feature/player/vm/PlaybackController.kt | 1 + .../feature/player/vm/SubtitleTrackMerger.kt | 43 +++++++++++++-- app/src/main/res/values/player.xml | 1 + .../player/vm/SubtitleTrackMergerTest.kt | 54 ++++++++++++++++++- 5 files changed, 97 insertions(+), 4 deletions(-) diff --git a/app/src/main/java/com/kino/puber/ui/feature/player/model/PlayerUIModels.kt b/app/src/main/java/com/kino/puber/ui/feature/player/model/PlayerUIModels.kt index dc1569fb..2e47e5fc 100644 --- a/app/src/main/java/com/kino/puber/ui/feature/player/model/PlayerUIModels.kt +++ b/app/src/main/java/com/kino/puber/ui/feature/player/model/PlayerUIModels.kt @@ -17,6 +17,8 @@ internal data class SubtitleTrackUIState( val url: String, val isForced: Boolean? = null, val sourceFile: String? = null, + /** Raw manifest/container label, kept to disambiguate same-language variants. */ + val descriptiveLabel: String? = null, val playerTrackId: String? = null, val playerTrackUri: String? = null, val playerGroupIndex: Int? = null, diff --git a/app/src/main/java/com/kino/puber/ui/feature/player/vm/PlaybackController.kt b/app/src/main/java/com/kino/puber/ui/feature/player/vm/PlaybackController.kt index 0aa72a52..bee68d62 100644 --- a/app/src/main/java/com/kino/puber/ui/feature/player/vm/PlaybackController.kt +++ b/app/src/main/java/com/kino/puber/ui/feature/player/vm/PlaybackController.kt @@ -666,6 +666,7 @@ internal class PlaybackController( label = subtitleTrackDisplayLabel(language, fallbackLabel), language = language, url = "", + descriptiveLabel = format.label ?: identityFormat?.label, isForced = (format.selectionFlags or (identityFormat?.selectionFlags ?: 0)) and C.SELECTION_FLAG_FORCED != 0, playerTrackId = format.id ?: identityFormat?.id, diff --git a/app/src/main/java/com/kino/puber/ui/feature/player/vm/SubtitleTrackMerger.kt b/app/src/main/java/com/kino/puber/ui/feature/player/vm/SubtitleTrackMerger.kt index 59bc674e..03f0f7dc 100644 --- a/app/src/main/java/com/kino/puber/ui/feature/player/vm/SubtitleTrackMerger.kt +++ b/app/src/main/java/com/kino/puber/ui/feature/player/vm/SubtitleTrackMerger.kt @@ -4,7 +4,11 @@ import com.kino.puber.ui.feature.player.model.SubtitleTrackUIState import com.kino.puber.ui.feature.player.model.isOff import java.util.Locale -internal class SubtitleTrackMerger { +internal class SubtitleTrackMerger( + private val variantLabel: (label: String, ordinal: Int) -> String = { label, ordinal -> + "$label ($ordinal)" + }, +) { fun merge( apiTracks: List, @@ -31,11 +35,44 @@ internal class SubtitleTrackMerger { } ?: playerTrack } - return (listOf(offTrack) + enrichedPlayerTracks).mapIndexed { index, track -> - track.copy(index = index) + return (listOf(offTrack) + disambiguateLabels(enrichedPlayerTracks)) + .mapIndexed { index, track -> track.copy(index = index) } + } + + /** + * Two tracks that share a language and a forced flag collapse to the same picker row. + * Prefer the raw manifest labels when they tell the variants apart, and fall back to + * an explicit ordinal when they do not. + */ + private fun disambiguateLabels( + tracks: List, + ): List { + val collisions = tracks.groupBy(::pickerRowKey).filterValues { it.size > 1 } + if (collisions.isEmpty()) return tracks + + val descriptiveKeys = collisions.filterValues(::hasDistinctDescriptiveLabels).keys + val ordinals = mutableMapOf() + return tracks.map { track -> + val key = pickerRowKey(track) + when { + key !in collisions -> track + key in descriptiveKeys -> track.copy(label = track.descriptiveLabel.orEmpty().trim()) + else -> { + val ordinal = ordinals.merge(key, 1, Int::plus) ?: 1 + track.copy(label = variantLabel(track.label, ordinal)) + } + } } } + private fun hasDistinctDescriptiveLabels(group: List): Boolean { + val labels = group.mapNotNull { track -> track.descriptiveLabel?.trim()?.takeIf(String::isNotEmpty) } + return labels.size == group.size && labels.toSet().size == group.size + } + + private fun pickerRowKey(track: SubtitleTrackUIState): String = + "${track.label}\u0000${track.isForced == true}" + private fun findExactIdentityMatch( playerTrack: SubtitleTrackUIState, apiTracks: List, diff --git a/app/src/main/res/values/player.xml b/app/src/main/res/values/player.xml index 6c571c48..f9b06b21 100644 --- a/app/src/main/res/values/player.xml +++ b/app/src/main/res/values/player.xml @@ -15,6 +15,7 @@ СУБТИТРЫ Выкл. частичные + %1$s · вариант %2$d Субтитры %1$d Аа Размер Маленький diff --git a/app/src/test/kotlin/com/kino/puber/ui/feature/player/vm/SubtitleTrackMergerTest.kt b/app/src/test/kotlin/com/kino/puber/ui/feature/player/vm/SubtitleTrackMergerTest.kt index 4b5f1450..e50499e4 100644 --- a/app/src/test/kotlin/com/kino/puber/ui/feature/player/vm/SubtitleTrackMergerTest.kt +++ b/app/src/test/kotlin/com/kino/puber/ui/feature/player/vm/SubtitleTrackMergerTest.kt @@ -8,7 +8,7 @@ import org.junit.jupiter.api.Test internal class SubtitleTrackMergerTest { - private val merger = SubtitleTrackMerger() + private val merger = SubtitleTrackMerger(variantLabel = { label, ordinal -> "$label #$ordinal" }) @Test fun subtitleTrackDisplayLabel_hidesManifestNumberingAndUsesLowercaseIso3Language() { @@ -285,6 +285,56 @@ internal class SubtitleTrackMergerTest { assertEquals(listOf(0), result.map { it.index }) } + @Test + fun merge_usesDescriptiveLabels_forSameLanguageVariantsThatWouldCollide() { + val playerTracks = listOf( + playerTrack(1, "rus", "rus", "full", 0, descriptiveLabel = "Русские полные"), + playerTrack(2, "rus", "rus", "sdh", 1, descriptiveLabel = "Русские SDH"), + ) + + val result = merger.merge(listOf(offTrack()), playerTracks) + + assertEquals(listOf("Off", "Русские полные", "Русские SDH"), result.map { it.label }) + } + + @Test + fun merge_numbersSameLanguageVariants_whenDescriptiveLabelsCannotSeparateThem() { + val playerTracks = listOf( + playerTrack(1, "rus", "rus", "a", 0, descriptiveLabel = "RUS"), + playerTrack(2, "rus", "rus", "b", 1, descriptiveLabel = "RUS"), + playerTrack(3, "rus", "rus", "c", 2, descriptiveLabel = null), + ) + + val result = merger.merge(listOf(offTrack()), playerTracks) + + assertEquals(listOf("Off", "rus #1", "rus #2", "rus #3"), result.map { it.label }) + } + + @Test + fun merge_keepsForcedVariantUntouched_becauseThePickerAlreadyMarksIt() { + val playerTracks = listOf( + playerTrack(1, "rus", "rus", "full", 0, forced = false, descriptiveLabel = "RUS"), + playerTrack(2, "rus", "rus", "forced", 1, forced = true, descriptiveLabel = "RUS"), + ) + + val result = merger.merge(listOf(offTrack()), playerTracks) + + assertEquals(listOf("Off", "rus", "rus"), result.map { it.label }) + assertEquals(listOf(null, false, true), result.map { it.isForced }) + } + + @Test + fun merge_leavesDistinctLabelsAlone() { + val playerTracks = listOf( + playerTrack(1, "rus", "rus", "a", 0, descriptiveLabel = "RUS"), + playerTrack(2, "eng", "eng", "b", 1, descriptiveLabel = "ENG"), + ) + + val result = merger.merge(listOf(offTrack()), playerTracks) + + assertEquals(listOf("Off", "rus", "eng"), result.map { it.label }) + } + private fun offTrack() = SubtitleTrackUIState( index = 0, label = "Off", @@ -316,12 +366,14 @@ internal class SubtitleTrackMergerTest { groupIndex: Int, forced: Boolean = false, uri: String? = null, + descriptiveLabel: String? = null, ) = SubtitleTrackUIState( index = index, label = label, language = language, url = "", isForced = forced, + descriptiveLabel = descriptiveLabel, playerTrackId = id, playerTrackUri = uri, playerGroupIndex = groupIndex, From f45a7eb41fb301b25de2b0db94c075313b59a185 Mon Sep 17 00:00:00 2001 From: Innokentii Enikeev Date: Sat, 29 Aug 2026 10:38:04 +0200 Subject: [PATCH 07/25] Harden player track preference restore Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01GEBLAtGvP5AHqHemWgLXAp --- .../puber/ui/feature/player/vm/PlayerVM.kt | 109 +++++++++++++----- .../player/vm/PlayerVMSubtitleVariantTest.kt | 91 +++++++++++++++ 2 files changed, 174 insertions(+), 26 deletions(-) diff --git a/app/src/main/java/com/kino/puber/ui/feature/player/vm/PlayerVM.kt b/app/src/main/java/com/kino/puber/ui/feature/player/vm/PlayerVM.kt index 2004cf95..342db28a 100644 --- a/app/src/main/java/com/kino/puber/ui/feature/player/vm/PlayerVM.kt +++ b/app/src/main/java/com/kino/puber/ui/feature/player/vm/PlayerVM.kt @@ -139,6 +139,9 @@ internal class PlayerVM( private var dismissedSegmentType: SkipSegmentType? = null private var countdownDismissed = false private var tracksRestoredForCurrentMedia = false + private var audioRestoredForCurrentMedia = false + private var subtitleTracksDiscovered = false + private var subtitleRestoreAttempts = 0 private var episodeSwitchInProgress = false private var lastPositionMs: Long = 0L private var contentChanges = ContentChangeSet.empty() @@ -155,7 +158,11 @@ internal class PlayerVM( private val controlsStateMachine = ControlsStateMachine() private val progressTracker = ProgressTracker() private val audioTrackPreferenceResolver = AudioTrackPreferenceResolver() - private val subtitleTrackMerger = SubtitleTrackMerger() + private val subtitleTrackMerger = SubtitleTrackMerger( + variantLabel = { label, ordinal -> + resources.getString(R.string.player_subtitle_variant_label, label, ordinal) + }, + ) private val debugOverlayEnabled = interactor.isDebugOverlayEnabled() private val playbackCallback = object : PlaybackControl.Callback { @@ -197,6 +204,14 @@ internal class PlayerVM( it.playerGroupIndex != null && it.playerTrackIndex != null } val effectivePlayerTracks = subtitleTracks.ifEmpty { previousPlayerTracks } + // A tracks update may carry only one track type; never let the other list be cleared. + val effectiveAudioTracks = audioTracks.ifEmpty { currentContent.audioTracks } + val effectiveAudioIndex = if (audioTracks.isEmpty()) { + currentContent.selectedAudioTrackIndex + } else { + selectedIndex + } + subtitleTracksDiscovered = subtitleTracksDiscovered || effectivePlayerTracks.isNotEmpty() val mergedSubtitleTracks = subtitleTrackMerger.merge(apiSubtitleTracks, effectivePlayerTracks) val mergedSelectedIndex = previousSubtitle?.let { selectedTrack -> audioTrackPreferenceResolver.findSubtitleTrackIndex( @@ -210,15 +225,19 @@ internal class PlayerVM( }?.takeIf { it >= 0 } ?: 0 updateContent { copy( - audioTracks = audioTracks, - selectedAudioTrackIndex = selectedIndex, + audioTracks = effectiveAudioTracks, + selectedAudioTrackIndex = effectiveAudioIndex, subtitleTracks = mergedSubtitleTracks, selectedSubtitleIndex = mergedSelectedIndex, ) } if (!tracksRestoredForCurrentMedia) { tracksRestoredForCurrentMedia = true - if (!restoreTrackPreferences(hasDiscoveredSubtitleTracks = effectivePlayerTracks.isNotEmpty())) { + val restored = restoreTrackPreferences( + hasDiscoveredSubtitleTracks = effectivePlayerTracks.isNotEmpty(), + ) + if (!restored && subtitleRestoreAttempts < MAX_SUBTITLE_RESTORE_ATTEMPTS) { + subtitleRestoreAttempts++ tracksRestoredForCurrentMedia = false } } @@ -327,6 +346,12 @@ internal class PlayerVM( loadSkipSegments(item, resolved.seasonNumber, resolved.episodeNumber, token) } + private fun resetTrackRestoreState() { + audioRestoredForCurrentMedia = false + subtitleTracksDiscovered = false + subtitleRestoreAttempts = 0 + } + private fun showInitialContent( contentState: PlayerContentState, resumeDialog: ResumeDialogState?, @@ -386,36 +411,50 @@ internal class PlayerVM( playbackController.switchStream(streamUrl, media.subtitles) } + /** + * Returns false only while the subtitle preference is still waiting for the player to + * report its text tracks, which asks the caller for another attempt. + */ private fun restoreTrackPreferences(hasDiscoveredSubtitleTracks: Boolean): Boolean { - val preferredLabel = interactor.getPreferredAudioLabel(params.itemId) - val preferredLang = interactor.getPreferredAudioLang(params.itemId) - val subtitleLang = interactor.getPreferredSubtitleLang(params.itemId) - val subtitleUrl = interactor.getPreferredSubtitleUrl(params.itemId) val content = (stateValue as? PlayerViewState.Content)?.content ?: return false + restoreAudioTrackPreference(content) + return restoreSubtitlePreference(content, hasDiscoveredSubtitleTracks) + } + + // Applied at most once per media so retrying the subtitle restore never re-applies + // the stored audio track over a selection the user made in the meantime. + private fun restoreAudioTrackPreference(content: PlayerContentState) { + if (audioRestoredForCurrentMedia || content.audioTracks.isEmpty()) return + audioRestoredForCurrentMedia = true val audioIndex = audioTrackPreferenceResolver.findAudioTrackIndex( tracks = content.audioTracks, - preferredLabel = preferredLabel, - preferredLang = preferredLang, + preferredLabel = interactor.getPreferredAudioLabel(params.itemId), + preferredLang = interactor.getPreferredAudioLang(params.itemId), ) - if (audioIndex >= 0) { applyAudioTrackSelection(audioIndex, persist = false) } + } - val subtitleIndex = if (hasDiscoveredSubtitleTracks) { - audioTrackPreferenceResolver.findSubtitleTrackIndex( - tracks = content.subtitleTracks, - preferredLang = subtitleLang, - preferredUrl = subtitleUrl, - ) - } else { - -1 - } + private fun restoreSubtitlePreference( + content: PlayerContentState, + hasDiscoveredSubtitleTracks: Boolean, + ): Boolean { + val subtitleLang = interactor.getPreferredSubtitleLang(params.itemId) + val subtitleUrl = interactor.getPreferredSubtitleUrl(params.itemId) + val hasSubtitlePreference = !subtitleLang.isNullOrEmpty() || !subtitleUrl.isNullOrEmpty() + if (!hasSubtitlePreference) return true + if (!hasDiscoveredSubtitleTracks) return false + + val subtitleIndex = audioTrackPreferenceResolver.findSubtitleTrackIndex( + tracks = content.subtitleTracks, + preferredLang = subtitleLang, + preferredUrl = subtitleUrl, + ) if (subtitleIndex >= 0) { applySubtitleSelection(subtitleIndex, persist = false) } - val hasSubtitlePreference = !subtitleLang.isNullOrEmpty() || !subtitleUrl.isNullOrEmpty() - return subtitleIndex >= 0 || !hasSubtitlePreference || hasDiscoveredSubtitleTracks + return true } override fun onAction(action: UIAction) { @@ -658,6 +697,7 @@ internal class PlayerVM( val position = playbackController.currentPosition updateContent { copy(selectedBufferPresetIndex = index) } tracksRestoredForCurrentMedia = false + resetTrackRestoreState() initializePlayer(savedPosition = position) } @@ -669,6 +709,7 @@ internal class PlayerVM( val position = playbackController.currentPosition updateContent { copy(fastDnsEnabled = newValue) } tracksRestoredForCurrentMedia = false + resetTrackRestoreState() initializePlayer(savedPosition = position) } @@ -700,6 +741,7 @@ internal class PlayerVM( dismissedSegmentType = null countdownDismissed = false tracksRestoredForCurrentMedia = false + resetTrackRestoreState() updateViewState(PlayerViewState.Loading) startPreparingPlayback( @@ -1394,14 +1436,26 @@ internal class PlayerVM( val state = (stateValue as? PlayerViewState.Content)?.content ?: return val audioTrack = state.audioTracks.getOrNull(state.selectedAudioTrackIndex) val subtitle = state.subtitleTracks.getOrNull(state.selectedSubtitleIndex) + // Before the player reports its text tracks the picker holds nothing but "off", + // so persisting it would silently drop the preference we still have to restore. + val subtitleLang = if (subtitleTracksDiscovered) { + subtitle?.language?.takeIf { it.isNotEmpty() } + } else { + interactor.getPreferredSubtitleLang(params.itemId) + } + val subtitleUrl = if (subtitleTracksDiscovered) { + subtitle?.url?.takeIf { it.isNotEmpty() } + ?: subtitle?.playerTrackUri?.takeIf { it.isNotEmpty() } + ?: subtitle?.playerTrackId?.takeIf { it.isNotEmpty() } + } else { + interactor.getPreferredSubtitleUrl(params.itemId) + } interactor.saveTrackPreferences( itemId = params.itemId, audioLang = audioTrack?.language?.takeIf { it.isNotEmpty() }, audioLabel = audioTrack?.label?.takeIf { it.isNotEmpty() }, - subtitleLang = subtitle?.language?.takeIf { it.isNotEmpty() }, - subtitleUrl = subtitle?.url?.takeIf { it.isNotEmpty() } - ?: subtitle?.playerTrackUri?.takeIf { it.isNotEmpty() } - ?: subtitle?.playerTrackId?.takeIf { it.isNotEmpty() }, + subtitleLang = subtitleLang, + subtitleUrl = subtitleUrl, ) } @@ -1476,5 +1530,8 @@ internal class PlayerVM( const val EARLY_NEXT_EPISODE_OFFSET_MS = 30_000L const val SEEK_JUMP_THRESHOLD_MS = 2_000L const val SKIP_COUNTDOWN_SEC = 7 + // Bounds the wait for player text tracks so a stream that simply has none + // does not re-run the restore on every single track update. + const val MAX_SUBTITLE_RESTORE_ATTEMPTS = 10 } } diff --git a/app/src/test/kotlin/com/kino/puber/ui/feature/player/vm/PlayerVMSubtitleVariantTest.kt b/app/src/test/kotlin/com/kino/puber/ui/feature/player/vm/PlayerVMSubtitleVariantTest.kt index ee7f1049..e5337a2b 100644 --- a/app/src/test/kotlin/com/kino/puber/ui/feature/player/vm/PlayerVMSubtitleVariantTest.kt +++ b/app/src/test/kotlin/com/kino/puber/ui/feature/player/vm/PlayerVMSubtitleVariantTest.kt @@ -13,6 +13,8 @@ import org.junit.jupiter.api.extension.RegisterExtension internal class PlayerVMSubtitleVariantTest : PlayerVMTestFixture() { companion object { + private const val MAX_TRACK_UPDATES = 15 + @JvmField @RegisterExtension val mainDispatcher = MainDispatcherExtension() @@ -171,6 +173,95 @@ internal class PlayerVMSubtitleVariantTest : PlayerVMTestFixture() { assertEquals("manifest-eng-2", contentState(vm).subtitleTracks[2].playerTrackId) } + @Test + fun tracksUpdated_keepsAudioTracks_whenUpdateCarriesOnlyTextTracks() { + val vm = startedVM() + val audioTracks = listOf( + AudioTrackUIState(0, "English", "eng"), + AudioTrackUIState(1, "Russian", "rus"), + ) + callbackSlot.captured.onTracksUpdated(audioTracks, 1, testDiscoveredSubtitleTracks) + + callbackSlot.captured.onTracksUpdated(emptyList(), 0, testDiscoveredSubtitleTracks) + + assertEquals(audioTracks, contentState(vm).audioTracks) + assertEquals(1, contentState(vm).selectedAudioTrackIndex) + } + + @Test + fun tracksUpdated_stopsRetryingRestore_whenSubtitleTracksNeverAppear() { + every { interactor.getPreferredAudioLang(42) } returns "rus" + every { interactor.getPreferredSubtitleLang(42) } returns "rus" + every { interactor.getPreferredSubtitleUrl(42) } returns "" + val vm = startedVM() + val audioTracks = listOf( + AudioTrackUIState(0, "English", "eng"), + AudioTrackUIState(1, "Russian", "rus"), + ) + + repeat(MAX_TRACK_UPDATES) { + callbackSlot.captured.onTracksUpdated(audioTracks, 0, emptyList()) + } + + // The audio restore must run once, not once per deferred subtitle retry. + verify(exactly = 1) { playbackController.selectAudioTrack(any()) } + assertEquals(0, contentState(vm).selectedSubtitleIndex) + } + + @Test + fun tracksUpdated_appliesAudioRestoreOnce_andLetsLaterUserChoiceStand() { + every { interactor.getPreferredAudioLang(42) } returns "eng" + every { interactor.getPreferredSubtitleLang(42) } returns "rus" + every { interactor.getPreferredSubtitleUrl(42) } returns "" + val vm = startedVM() + val audioTracks = listOf( + AudioTrackUIState(0, "English", "eng"), + AudioTrackUIState(1, "Russian", "rus"), + ) + + callbackSlot.captured.onTracksUpdated(audioTracks, 0, emptyList()) + vm.onAction(PlayerAction.SelectAudioTrack(1)) + callbackSlot.captured.onTracksUpdated(audioTracks, 1, emptyList()) + + assertEquals(1, contentState(vm).selectedAudioTrackIndex) + // Restored once on the first update, never re-applied over the user's choice. + verify(exactly = 1) { playbackController.selectAudioTrack(0) } + } + + @Test + fun audioSelection_keepsStoredSubtitlePreference_whileSubtitleTracksAreUnknown() { + every { interactor.getPreferredSubtitleLang(42) } returns "rus" + every { interactor.getPreferredSubtitleUrl(42) } returns "https://api.test/subtitles/rus.srt" + val vm = startedVM() + val audioTracks = listOf(AudioTrackUIState(0, "English", "eng")) + callbackSlot.captured.onTracksUpdated(audioTracks, 0, emptyList()) + + vm.onAction(PlayerAction.SelectAudioTrack(0)) + + verify { + interactor.saveTrackPreferences( + 42, + "eng", + "English", + "rus", + "https://api.test/subtitles/rus.srt", + ) + } + } + + @Test + fun subtitleSelection_persistsOffChoice_onceSubtitleTracksAreKnown() { + every { interactor.getPreferredSubtitleLang(42) } returns "rus" + every { interactor.getPreferredSubtitleUrl(42) } returns "https://api.test/subtitles/rus.srt" + val vm = startedVM() + val audioTracks = listOf(AudioTrackUIState(0, "English", "eng")) + callbackSlot.captured.onTracksUpdated(audioTracks, 0, testDiscoveredSubtitleTracks) + + vm.onAction(PlayerAction.SelectSubtitle(0)) + + verify { interactor.saveTrackPreferences(42, "eng", "English", null, null) } + } + private fun manifestTrack( index: Int, label: String, From 5fd63edc1644467e39d8d1620e75255419365b88 Mon Sep 17 00:00:00 2001 From: Innokentii Enikeev Date: Sat, 29 Aug 2026 10:41:58 +0200 Subject: [PATCH 08/25] Drop unreachable subtitle code paths Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01GEBLAtGvP5AHqHemWgLXAp --- .../com/kino/puber/data/api/models/Models.kt | 3 --- .../ui/feature/player/vm/PlaybackController.kt | 4 ++-- .../feature/player/vm/SubtitleTrackMerger.kt | 4 +--- .../puber/data/api/models/SubtitleLinkTest.kt | 18 ++++++++++-------- 4 files changed, 13 insertions(+), 16 deletions(-) diff --git a/app/src/main/java/com/kino/puber/data/api/models/Models.kt b/app/src/main/java/com/kino/puber/data/api/models/Models.kt index 60894c21..edf43b34 100644 --- a/app/src/main/java/com/kino/puber/data/api/models/Models.kt +++ b/app/src/main/java/com/kino/puber/data/api/models/Models.kt @@ -387,9 +387,6 @@ data class SubtitleLink( url.substringBefore('?').substringBefore('#'), ) } - - val isForced: Boolean - get() = forcedState == true } private val FORCED_SUBTITLE_TOKEN = Regex( diff --git a/app/src/main/java/com/kino/puber/ui/feature/player/vm/PlaybackController.kt b/app/src/main/java/com/kino/puber/ui/feature/player/vm/PlaybackController.kt index bee68d62..3d34a970 100644 --- a/app/src/main/java/com/kino/puber/ui/feature/player/vm/PlaybackController.kt +++ b/app/src/main/java/com/kino/puber/ui/feature/player/vm/PlaybackController.kt @@ -701,9 +701,9 @@ internal class PlaybackController( stableKey: String, textGroups: List, ): TextTrackSelection? { + // Side-loaded configurations carry the stable key as both id and label, and + // manifest renditions carry their own id, so the raw API url is never a Format id. return findTextTrackBy(textGroups) { format -> - track.url.isNotEmpty() && format.id == track.url - } ?: findTextTrackBy(textGroups) { format -> track.playerTrackId != null && format.id == track.playerTrackId } ?: findTextTrackBy(textGroups) { format -> stableKey.isNotEmpty() && (format.id == stableKey || format.label == stableKey) diff --git a/app/src/main/java/com/kino/puber/ui/feature/player/vm/SubtitleTrackMerger.kt b/app/src/main/java/com/kino/puber/ui/feature/player/vm/SubtitleTrackMerger.kt index 03f0f7dc..ef103378 100644 --- a/app/src/main/java/com/kino/puber/ui/feature/player/vm/SubtitleTrackMerger.kt +++ b/app/src/main/java/com/kino/puber/ui/feature/player/vm/SubtitleTrackMerger.kt @@ -5,9 +5,7 @@ import com.kino.puber.ui.feature.player.model.isOff import java.util.Locale internal class SubtitleTrackMerger( - private val variantLabel: (label: String, ordinal: Int) -> String = { label, ordinal -> - "$label ($ordinal)" - }, + private val variantLabel: (label: String, ordinal: Int) -> String, ) { fun merge( diff --git a/app/src/test/kotlin/com/kino/puber/data/api/models/SubtitleLinkTest.kt b/app/src/test/kotlin/com/kino/puber/data/api/models/SubtitleLinkTest.kt index 04156e27..d815b8f9 100644 --- a/app/src/test/kotlin/com/kino/puber/data/api/models/SubtitleLinkTest.kt +++ b/app/src/test/kotlin/com/kino/puber/data/api/models/SubtitleLinkTest.kt @@ -1,6 +1,8 @@ package com.kino.puber.data.api.models +import org.junit.jupiter.api.Assertions.assertEquals import org.junit.jupiter.api.Assertions.assertFalse +import org.junit.jupiter.api.Assertions.assertNull import org.junit.jupiter.api.Assertions.assertTrue import org.junit.jupiter.api.Test @@ -28,17 +30,17 @@ internal class SubtitleLinkTest { } @Test - fun isForced_detectsForcedMarkerInPath_ignoringCaseAndQuery() { + fun forcedState_detectsForcedMarkerInPath_ignoringCaseAndQuery() { val subtitle = SubtitleLink( lang = "rus", url = "https://cdn.test/subtitles/RUS-FORCED.vtt?token=forced-value", ) - assertTrue(subtitle.isForced) + assertEquals(true, subtitle.forcedState) } @Test - fun isForced_doesNotUseQueryOrPartialWordAsMarker() { + fun forcedState_doesNotUseQueryOrPartialWordAsMarker() { val regularSubtitle = SubtitleLink( lang = "rus", url = "https://cdn.test/subtitles/russian.vtt?mode=forced", @@ -48,12 +50,12 @@ internal class SubtitleLinkTest { url = "https://cdn.test/subtitles/russian-unforced.vtt", ) - assertFalse(regularSubtitle.isForced) - assertFalse(unforcedSubtitle.isForced) + assertNull(regularSubtitle.forcedState) + assertNull(unforcedSubtitle.forcedState) } @Test - fun isForced_usesApiFlagInsteadOfUrlHeuristic() { + fun forcedState_usesApiFlagInsteadOfUrlHeuristic() { val forcedSubtitle = SubtitleLink( lang = "rus", url = "https://cdn.test/subtitles/russian.srt", @@ -66,7 +68,7 @@ internal class SubtitleLinkTest { forced = false, ) - assertTrue(forcedSubtitle.isForced) - assertFalse(regularSubtitle.isForced) + assertEquals(true, forcedSubtitle.forcedState) + assertEquals(false, regularSubtitle.forcedState) } } From 08f7e15c4fdc337ae2b99a18543971c1893015fd Mon Sep 17 00:00:00 2001 From: Innokentii Enikeev Date: Sat, 29 Aug 2026 10:52:24 +0200 Subject: [PATCH 09/25] Side-load API subtitles on HLS streams Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01GEBLAtGvP5AHqHemWgLXAp --- .../feature/player/vm/PlaybackController.kt | 73 ++++++++++------ .../feature/player/vm/SubtitleTrackMerger.kt | 45 +++++++--- .../player/vm/SubtitleTrackMergerTest.kt | 83 +++++++++++++++++++ 3 files changed, 165 insertions(+), 36 deletions(-) diff --git a/app/src/main/java/com/kino/puber/ui/feature/player/vm/PlaybackController.kt b/app/src/main/java/com/kino/puber/ui/feature/player/vm/PlaybackController.kt index 3d34a970..abb35268 100644 --- a/app/src/main/java/com/kino/puber/ui/feature/player/vm/PlaybackController.kt +++ b/app/src/main/java/com/kino/puber/ui/feature/player/vm/PlaybackController.kt @@ -24,6 +24,9 @@ import androidx.media3.exoplayer.hls.playlist.HlsMultivariantPlaylist import androidx.media3.exoplayer.mediacodec.MediaCodecRenderer import androidx.media3.exoplayer.source.BehindLiveWindowException import androidx.media3.exoplayer.source.DefaultMediaSourceFactory +import androidx.media3.exoplayer.source.MediaSource +import androidx.media3.exoplayer.source.MergingMediaSource +import androidx.media3.exoplayer.source.SingleSampleMediaSource import androidx.media3.exoplayer.trackselection.AdaptiveTrackSelection import androidx.media3.exoplayer.trackselection.DefaultTrackSelector import androidx.media3.exoplayer.upstream.DefaultBandwidthMeter @@ -372,27 +375,26 @@ internal class PlaybackController( private fun buildMediaItem(streamUrl: String, subtitles: List?): MediaItem { val builder = MediaItem.Builder().setUri(streamUrl) - val isHlsStream = streamUrl.isHlsStreamUrl() - if (isHlsStream) { + if (streamUrl.isHlsStreamUrl()) { builder.setMimeType(MimeTypes.APPLICATION_M3U8) } - // KinoPub HLS manifests already contain the API subtitle list as renditions. - // Adding the API URLs again creates a second set of Media3 text tracks. - if (!isHlsStream && !subtitles.isNullOrEmpty()) { - val subtitleConfigs = subtitles.mapNotNull { sub -> - if (!sub.shouldSideLoad) return@mapNotNull null - val subtitleUrl = sub.url - val stableKey = subtitleUrl.stableSubtitleKey() - MediaItem.SubtitleConfiguration.Builder(subtitleUrl.toUri()) - .setMimeType(subtitleMimeType(subtitleUrl)) - .setLanguage(sub.lang) - .setLabel(stableKey) - .setId(stableKey) - .build() - } - if (subtitleConfigs.isNotEmpty()) { - builder.setSubtitleConfigurations(subtitleConfigs) - } + // Every external subtitle is attached, including ones an HLS manifest may also + // publish as a rendition: the manifest contents are unknown until the source is + // prepared, and a rendition that turns out to duplicate an API entry is dropped + // from the picker afterwards by SubtitleTrackMerger. + val subtitleConfigs = subtitles.orEmpty().mapNotNull { sub -> + if (!sub.shouldSideLoad) return@mapNotNull null + val subtitleUrl = sub.url + val stableKey = subtitleUrl.stableSubtitleKey() + MediaItem.SubtitleConfiguration.Builder(subtitleUrl.toUri()) + .setMimeType(subtitleMimeType(subtitleUrl)) + .setLanguage(sub.lang) + .setLabel(stableKey) + .setId(stableKey) + .build() + } + if (subtitleConfigs.isNotEmpty()) { + builder.setSubtitleConfigurations(subtitleConfigs) } return builder.build() } @@ -430,15 +432,34 @@ internal class PlaybackController( @OptIn(UnstableApi::class) private fun setMediaSource(player: ExoPlayer, mediaItem: MediaItem, streamUrl: String) { val dsFactory = dataSourceFactory ?: return - if (streamUrl.isHlsStreamUrl()) { - val hlsSource = HlsMediaSource.Factory(dsFactory) - .setAllowChunklessPreparation(true) - .setLoadErrorHandlingPolicy(HlsErrorPolicy()) - .createMediaSource(mediaItem) - player.setMediaSource(hlsSource) - } else { + if (!streamUrl.isHlsStreamUrl()) { + // DefaultMediaSourceFactory side-loads the subtitle configurations itself. player.setMediaItem(mediaItem) + return + } + val hlsSource = HlsMediaSource.Factory(dsFactory) + .setAllowChunklessPreparation(true) + .setLoadErrorHandlingPolicy(HlsErrorPolicy()) + .createMediaSource(mediaItem) + player.setMediaSource(withSideLoadedSubtitles(hlsSource, mediaItem, dsFactory)) + } + + // HlsMediaSource.Factory ignores MediaItem subtitle configurations, so an API subtitle + // missing from the manifest would otherwise be unreachable on an HLS stream. + @OptIn(UnstableApi::class) + private fun withSideLoadedSubtitles( + source: MediaSource, + mediaItem: MediaItem, + dsFactory: DataSource.Factory, + ): MediaSource { + val subtitleConfigs = mediaItem.localConfiguration?.subtitleConfigurations.orEmpty() + if (subtitleConfigs.isEmpty()) return source + val subtitleSources = subtitleConfigs.map { config -> + SingleSampleMediaSource.Factory(dsFactory) + .setLoadErrorHandlingPolicy(HlsErrorPolicy()) + .createMediaSource(config, C.TIME_UNSET) } + return MergingMediaSource(source, *subtitleSources.toTypedArray()) } data class DebugInfo( diff --git a/app/src/main/java/com/kino/puber/ui/feature/player/vm/SubtitleTrackMerger.kt b/app/src/main/java/com/kino/puber/ui/feature/player/vm/SubtitleTrackMerger.kt index ef103378..ae3d42c0 100644 --- a/app/src/main/java/com/kino/puber/ui/feature/player/vm/SubtitleTrackMerger.kt +++ b/app/src/main/java/com/kino/puber/ui/feature/player/vm/SubtitleTrackMerger.kt @@ -24,17 +24,43 @@ internal class SubtitleTrackMerger( return listOf(offTrack.copy(index = 0)) } - val availableApiIndices = apiSubtitles.indices.toMutableSet() - val enrichedPlayerTracks = playerSubtitles.map { playerTrack -> - findExactIdentityMatch(playerTrack, apiSubtitles, availableApiIndices) - ?.let { apiIndex -> - availableApiIndices.remove(apiIndex) - playerTrack.withApiMetadata(apiSubtitles[apiIndex]) + val enrichedPlayerTracks = enrichPlayerTracks(playerSubtitles, apiSubtitles) + return (listOf(offTrack) + disambiguateLabels(enrichedPlayerTracks)) + .mapIndexed { index, track -> track.copy(index = index) } + } + + /** + * Every external subtitle is side-loaded because the manifest contents are unknown + * before preparation, so a subtitle the manifest also publishes shows up twice. The + * two tracks resolve to the same API entry, and the manifest rendition wins. + */ + private fun enrichPlayerTracks( + playerSubtitles: List, + apiSubtitles: List, + ): List { + val apiMatches = playerSubtitles.map { track -> findExactIdentityMatch(track, apiSubtitles) } + val redundant = mutableSetOf() + val owners = mutableMapOf() + apiMatches.withIndex() + .mapNotNull { (position, apiIndex) -> apiIndex?.let { it to position } } + .groupBy({ it.first }, { it.second }) + .forEach { (apiIndex, positions) -> + val fromManifest = positions.filter { playerSubtitles[it].playerTrackUri != null } + if (fromManifest.size == 1 && positions.size > 1) { + redundant += positions - fromManifest.toSet() + owners[apiIndex] = fromManifest.single() + } else { + owners[apiIndex] = positions.first() } + } + + return playerSubtitles.mapIndexedNotNull { position, playerTrack -> + if (position in redundant) return@mapIndexedNotNull null + apiMatches[position] + ?.takeIf { apiIndex -> owners[apiIndex] == position } + ?.let { apiIndex -> playerTrack.withApiMetadata(apiSubtitles[apiIndex]) } ?: playerTrack } - return (listOf(offTrack) + disambiguateLabels(enrichedPlayerTracks)) - .mapIndexed { index, track -> track.copy(index = index) } } /** @@ -74,14 +100,13 @@ internal class SubtitleTrackMerger( private fun findExactIdentityMatch( playerTrack: SubtitleTrackUIState, apiTracks: List, - availableApiIndices: Set, ): Int? { val playerIdentities = listOfNotNull( playerTrack.playerTrackUri, playerTrack.playerTrackId, ).filter { it.isNotEmpty() } if (playerIdentities.isEmpty()) return null - return availableApiIndices.filter { apiIndex -> + return apiTracks.indices.filter { apiIndex -> val apiTrack = apiTracks[apiIndex] val apiIdentities = listOfNotNull(apiTrack.sourceFile, apiTrack.url) .filter { it.isNotEmpty() } diff --git a/app/src/test/kotlin/com/kino/puber/ui/feature/player/vm/SubtitleTrackMergerTest.kt b/app/src/test/kotlin/com/kino/puber/ui/feature/player/vm/SubtitleTrackMergerTest.kt index e50499e4..5562808b 100644 --- a/app/src/test/kotlin/com/kino/puber/ui/feature/player/vm/SubtitleTrackMergerTest.kt +++ b/app/src/test/kotlin/com/kino/puber/ui/feature/player/vm/SubtitleTrackMergerTest.kt @@ -335,6 +335,89 @@ internal class SubtitleTrackMergerTest { assertEquals(listOf("Off", "rus", "eng"), result.map { it.label }) } + @Test + fun merge_hidesSideLoadedCopy_whenManifestPublishesTheSameSubtitle() { + val apiTracks = listOf( + offTrack(), + apiTrack(1, "rus", "rus", url = "https://api.test/subtitles/rus.srt"), + ) + val playerTracks = listOf( + playerTrack( + index = 1, + label = "rus", + language = "rus", + id = "hls-rus", + groupIndex = 0, + uri = "https://cdn.test/subtitles/rus.srt", + ), + // The side-loaded copy Media3 built from the same API url. + playerTrack(2, "rus", "rus", "rus.srt", 1), + ) + + val result = merger.merge(apiTracks, playerTracks) + + assertEquals(listOf("Off", "rus"), result.map { it.label }) + assertEquals("hls-rus", result[1].playerTrackId) + assertEquals("https://api.test/subtitles/rus.srt", result[1].url) + } + + @Test + fun merge_keepsSideLoadedTrack_whenTheManifestDoesNotPublishIt() { + val apiTracks = listOf( + offTrack(), + apiTrack(1, "rus", "rus", url = "https://api.test/subtitles/rus.srt"), + apiTrack(2, "eng", "eng", url = "https://api.test/subtitles/eng.srt"), + ) + val playerTracks = listOf( + playerTrack( + index = 1, + label = "rus", + language = "rus", + id = "hls-rus", + groupIndex = 0, + uri = "https://cdn.test/subtitles/rus.srt", + ), + playerTrack(2, "eng", "eng", "eng.srt", 1), + ) + + val result = merger.merge(apiTracks, playerTracks) + + assertEquals(listOf("Off", "rus", "eng"), result.map { it.label }) + assertEquals("https://api.test/subtitles/eng.srt", result[2].url) + } + + @Test + fun merge_keepsBothManifestRenditions_whenTheyResolveToOneApiEntry() { + val apiTracks = listOf( + offTrack(), + apiTrack(1, "rus", "rus", url = "https://api.test/subtitles/rus.srt"), + ) + val playerTracks = listOf( + playerTrack( + index = 1, + label = "rus", + language = "rus", + id = "a", + groupIndex = 0, + uri = "https://cdn.test/subtitles/rus.srt", + descriptiveLabel = "RUS A", + ), + playerTrack( + index = 2, + label = "rus", + language = "rus", + id = "b", + groupIndex = 1, + uri = "https://cdn.test/subtitles/rus.srt", + descriptiveLabel = "RUS B", + ), + ) + + val result = merger.merge(apiTracks, playerTracks) + + assertEquals(listOf("Off", "RUS A", "RUS B"), result.map { it.label }) + } + private fun offTrack() = SubtitleTrackUIState( index = 0, label = "Off", From 64524da21267f679f29f7d7b5f146235333e7088 Mon Sep 17 00:00:00 2001 From: Innokentii Enikeev Date: Sat, 29 Aug 2026 10:59:40 +0200 Subject: [PATCH 10/25] Select subtitle tracks by merge-stable group id Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01GEBLAtGvP5AHqHemWgLXAp --- .../ui/feature/player/model/PlayerUIModels.kt | 3 + .../feature/player/vm/PlaybackController.kt | 88 +++------- .../player/vm/SubtitleTrackSelector.kt | 103 +++++++++++ .../player/vm/SubtitleTrackSelectorTest.kt | 164 ++++++++++++++++++ 4 files changed, 292 insertions(+), 66 deletions(-) create mode 100644 app/src/main/java/com/kino/puber/ui/feature/player/vm/SubtitleTrackSelector.kt create mode 100644 app/src/test/kotlin/com/kino/puber/ui/feature/player/vm/SubtitleTrackSelectorTest.kt diff --git a/app/src/main/java/com/kino/puber/ui/feature/player/model/PlayerUIModels.kt b/app/src/main/java/com/kino/puber/ui/feature/player/model/PlayerUIModels.kt index 2e47e5fc..b5ed4356 100644 --- a/app/src/main/java/com/kino/puber/ui/feature/player/model/PlayerUIModels.kt +++ b/app/src/main/java/com/kino/puber/ui/feature/player/model/PlayerUIModels.kt @@ -20,6 +20,8 @@ internal data class SubtitleTrackUIState( /** Raw manifest/container label, kept to disambiguate same-language variants. */ val descriptiveLabel: String? = null, val playerTrackId: String? = null, + /** Media3 `TrackGroup.id`, unique per group once side-loaded subtitles are merged. */ + val playerTrackGroupId: String? = null, val playerTrackUri: String? = null, val playerGroupIndex: Int? = null, val playerTrackIndex: Int? = null, @@ -29,6 +31,7 @@ internal val SubtitleTrackUIState.isOff: Boolean get() = language.isEmpty() && url.isEmpty() && playerTrackId == null && + playerTrackGroupId == null && playerTrackUri == null && playerGroupIndex == null && playerTrackIndex == null diff --git a/app/src/main/java/com/kino/puber/ui/feature/player/vm/PlaybackController.kt b/app/src/main/java/com/kino/puber/ui/feature/player/vm/PlaybackController.kt index abb35268..9a1e0e88 100644 --- a/app/src/main/java/com/kino/puber/ui/feature/player/vm/PlaybackController.kt +++ b/app/src/main/java/com/kino/puber/ui/feature/player/vm/PlaybackController.kt @@ -95,6 +95,7 @@ internal class PlaybackController( private val ac3FallbackPolicy = Ac3FallbackPolicy() private val callbackGate = PlaybackCallbackGate() private val mediaItemFactory = PlaybackMediaItemFactory() + private val subtitleTrackSelector = SubtitleTrackSelector() @OptIn(UnstableApi::class) private val bandwidthMeter = DefaultBandwidthMeter.Builder(context).build() @@ -647,6 +648,7 @@ internal class PlaybackController( ), groupIndex = groupIndex, trackIndex = trackIndex, + groupId = group.mediaTrackGroup.id, ) } } @@ -676,6 +678,7 @@ internal class PlaybackController( hlsRendition: HlsMultivariantPlaylist.Rendition?, groupIndex: Int, trackIndex: Int, + groupId: String, ): SubtitleTrackUIState { val identityFormat = hlsRendition?.format val language = format.language ?: identityFormat?.language.orEmpty() @@ -691,6 +694,7 @@ internal class PlaybackController( isForced = (format.selectionFlags or (identityFormat?.selectionFlags ?: 0)) and C.SELECTION_FLAG_FORCED != 0, playerTrackId = format.id ?: identityFormat?.id, + playerTrackGroupId = groupId, playerTrackUri = hlsRendition?.url?.toString(), playerGroupIndex = groupIndex, playerTrackIndex = trackIndex, @@ -703,82 +707,34 @@ internal class PlaybackController( private fun applySubtitleTrackSelection(track: SubtitleTrackUIState) { val player = exoPlayer ?: return - val stableKey = track.url.stableSubtitleKey() val textGroups = player.currentTracks.groups.filter { it.type == C.TRACK_TYPE_TEXT } - val target = findTextTrack(track, stableKey, textGroups) - if (target == null) return - val builder = player.trackSelectionParameters + val target = subtitleTrackSelector.select(track, textGroups.toPlayerTextTracks()) + ?: return + val targetGroup = textGroups.getOrNull(target.groupIndex) ?: return + player.trackSelectionParameters = player.trackSelectionParameters .buildUpon() .clearOverridesOfType(C.TRACK_TYPE_TEXT) .setTrackTypeDisabled(C.TRACK_TYPE_TEXT, false) .setOverrideForType( - TrackSelectionOverride(target.group.mediaTrackGroup, target.trackIndex), + TrackSelectionOverride(targetGroup.mediaTrackGroup, target.trackIndex), ) - player.trackSelectionParameters = builder.build() - } - - private fun findTextTrack( - track: SubtitleTrackUIState, - stableKey: String, - textGroups: List, - ): TextTrackSelection? { - // Side-loaded configurations carry the stable key as both id and label, and - // manifest renditions carry their own id, so the raw API url is never a Format id. - return findTextTrackBy(textGroups) { format -> - track.playerTrackId != null && format.id == track.playerTrackId - } ?: findTextTrackBy(textGroups) { format -> - stableKey.isNotEmpty() && (format.id == stableKey || format.label == stableKey) - } ?: findTextTrackByPlayerCoordinates(textGroups, track) - ?: findUnambiguousTextTrackByLanguage(textGroups, track.language) - } - - private fun findTextTrackByPlayerCoordinates( - textGroups: List, - track: SubtitleTrackUIState, - ): TextTrackSelection? { - val group = track.playerGroupIndex?.let(textGroups::getOrNull) - val trackIndex = track.playerTrackIndex - return if (group != null && trackIndex != null && trackIndex in 0 until group.length) { - TextTrackSelection(group = group, trackIndex = trackIndex) - } else { - null - } + .build() } - private fun findUnambiguousTextTrackByLanguage( - textGroups: List, - language: String, - ): TextTrackSelection? { - if (language.isEmpty()) return null - val matches = textGroups.flatMap { group -> - (0 until group.length).mapNotNull { trackIndex -> - group.getTrackFormat(trackIndex).takeIf { format -> - format.language?.let { sameSubtitleLanguage(it, language) } == true - }?.let { - TextTrackSelection(group = group, trackIndex = trackIndex) - } + private fun List.toPlayerTextTracks(): List = + flatMapIndexed { groupIndex, group -> + (0 until group.length).map { trackIndex -> + val format = group.getTrackFormat(trackIndex) + PlayerTextTrack( + groupId = group.mediaTrackGroup.id, + groupIndex = groupIndex, + trackIndex = trackIndex, + formatId = format.id, + formatLabel = format.label, + language = format.language, + ) } } - return matches.singleOrNull() - } - - private fun findTextTrackBy( - textGroups: List, - predicate: (Format) -> Boolean, - ): TextTrackSelection? { - return textGroups.flatMap { group -> - (0 until group.length).mapNotNull { trackIndex -> - group.getTrackFormat(trackIndex).takeIf(predicate)?.let { - TextTrackSelection(group = group, trackIndex = trackIndex) - } - } - }.singleOrNull() - } - - private data class TextTrackSelection( - val group: Tracks.Group, - val trackIndex: Int, - ) private companion object { const val MIN_DURATION_FOR_QUALITY_INCREASE_MS = 10_000 diff --git a/app/src/main/java/com/kino/puber/ui/feature/player/vm/SubtitleTrackSelector.kt b/app/src/main/java/com/kino/puber/ui/feature/player/vm/SubtitleTrackSelector.kt new file mode 100644 index 00000000..1df3fd10 --- /dev/null +++ b/app/src/main/java/com/kino/puber/ui/feature/player/vm/SubtitleTrackSelector.kt @@ -0,0 +1,103 @@ +package com.kino.puber.ui.feature.player.vm + +import com.kino.puber.ui.feature.player.model.SubtitleTrackUIState + +/** + * A text track exposed by the player, flattened out of the Media3 track groups. + * + * [groupId] is `TrackGroup.id`. MergingMediaSource rewrites the ids of its children to + * `":"`, so once side-loaded subtitles are merged into an HLS + * source the id is unique per track group and stable across track updates. Plain sources + * may leave it blank, which is why every match below is uniqueness-checked. + */ +internal data class PlayerTextTrack( + val groupId: String, + val groupIndex: Int, + val trackIndex: Int, + val formatId: String? = null, + val formatLabel: String? = null, + val language: String? = null, +) + +/** + * Resolves the player text track a picker row points at. + * + * The rules never guess: every candidate set is reduced with `singleOrNull`, so an + * ambiguous match yields nothing and falls through to the next rule rather than + * selecting an arbitrary track. + */ +internal class SubtitleTrackSelector { + + fun select( + track: SubtitleTrackUIState, + candidates: List, + ): PlayerTextTrack? { + if (candidates.isEmpty()) return null + return matchByTrackGroupId(track, candidates) + ?: matchByFormatId(track, candidates) + ?: matchByStableKey(track, candidates) + ?: matchByCoordinates(track, candidates) + ?: matchByLanguage(track, candidates) + } + + /** Exact and order independent: survives track groups being added or reordered. */ + private fun matchByTrackGroupId( + track: SubtitleTrackUIState, + candidates: List, + ): PlayerTextTrack? { + val groupId = track.playerTrackGroupId?.takeIf { it.isNotEmpty() } ?: return null + val trackIndex = track.playerTrackIndex ?: return null + return candidates + .filter { it.groupId == groupId && it.trackIndex == trackIndex } + .singleOrNull() + } + + private fun matchByFormatId( + track: SubtitleTrackUIState, + candidates: List, + ): PlayerTextTrack? { + val formatId = track.playerTrackId?.takeIf { it.isNotEmpty() } ?: return null + return candidates.filter { it.formatId == formatId }.singleOrNull() + } + + /** + * The stable key identifies the side-loaded copy Media3 built from the API url, which + * carries it as both format id and label. A row backed by a manifest rendition must + * never resolve here: it holds the same API url after merging, and matching on it + * would select the hidden side-loaded duplicate instead of the rendition. + */ + private fun matchByStableKey( + track: SubtitleTrackUIState, + candidates: List, + ): PlayerTextTrack? { + if (track.playerTrackUri != null) return null + val stableKey = track.url.stableSubtitleKey().takeIf { it.isNotEmpty() } ?: return null + return candidates + .filter { it.formatId == stableKey || it.formatLabel == stableKey } + .singleOrNull() + } + + /** Positional fallback for tracks the manifest exposes without any usable identity. */ + private fun matchByCoordinates( + track: SubtitleTrackUIState, + candidates: List, + ): PlayerTextTrack? { + val groupIndex = track.playerGroupIndex ?: return null + val trackIndex = track.playerTrackIndex ?: return null + return candidates + .filter { it.groupIndex == groupIndex && it.trackIndex == trackIndex } + .singleOrNull() + } + + private fun matchByLanguage( + track: SubtitleTrackUIState, + candidates: List, + ): PlayerTextTrack? { + if (track.language.isEmpty()) return null + return candidates + .filter { candidate -> + candidate.language?.let { sameSubtitleLanguage(it, track.language) } == true + } + .singleOrNull() + } +} diff --git a/app/src/test/kotlin/com/kino/puber/ui/feature/player/vm/SubtitleTrackSelectorTest.kt b/app/src/test/kotlin/com/kino/puber/ui/feature/player/vm/SubtitleTrackSelectorTest.kt new file mode 100644 index 00000000..4d8041ca --- /dev/null +++ b/app/src/test/kotlin/com/kino/puber/ui/feature/player/vm/SubtitleTrackSelectorTest.kt @@ -0,0 +1,164 @@ +package com.kino.puber.ui.feature.player.vm + +import com.kino.puber.ui.feature.player.model.SubtitleTrackUIState +import org.junit.jupiter.api.Assertions.assertEquals +import org.junit.jupiter.api.Assertions.assertNull +import org.junit.jupiter.api.Test + +internal class SubtitleTrackSelectorTest { + + private val selector = SubtitleTrackSelector() + + /** + * The regression that made merged HLS playback unusable before: the picker row is + * backed by a manifest rendition, but after merging it also carries the API url, whose + * stable key is the id of the hidden side-loaded copy of the same subtitle. + */ + @Test + fun select_prefersManifestRendition_overSideLoadedCopyOfTheSameSubtitle() { + val manifestRow = manifestTrack( + groupId = "0:rus-rendition", + trackIndex = 0, + formatId = "rus-rendition", + url = "https://api.test/subtitles/29725.srt", + ) + val candidates = listOf( + PlayerTextTrack("0:rus-rendition", 0, 0, formatId = "rus-rendition", language = "rus"), + PlayerTextTrack("1:", 1, 0, formatId = "29725.srt", formatLabel = "29725.srt", language = "rus"), + ) + + assertEquals(candidates[0], selector.select(manifestRow, candidates)) + } + + /** + * The same collision with a rendition the manifest publishes without an ID: there is no + * format id to match on, so the old rules fell through to the stable key and selected + * the side-loaded duplicate. The track group id resolves it unambiguously. + */ + @Test + fun select_prefersIdLessManifestRendition_overSideLoadedCopy() { + val manifestRow = manifestTrack( + groupId = "0:rus", + trackIndex = 0, + formatId = null, + url = "https://api.test/subtitles/29725.srt", + ) + val candidates = listOf( + PlayerTextTrack("0:rus", 0, 0, formatLabel = "Русские", language = "rus"), + PlayerTextTrack("1:", 1, 0, formatId = "29725.srt", formatLabel = "29725.srt", language = "rus"), + ) + + assertEquals(candidates[0], selector.select(manifestRow, candidates)) + } + + @Test + fun select_resolvesSideLoadedRow_byStableKey() { + val sideLoadedRow = SubtitleTrackUIState( + index = 1, + label = "eng", + language = "eng", + url = "https://api.test/subtitles/29726.srt", + playerTrackGroupId = "1:", + playerTrackId = "29726.srt", + playerGroupIndex = 1, + playerTrackIndex = 0, + ) + val candidates = listOf( + PlayerTextTrack("0:rus-rendition", 0, 0, formatId = "rus-rendition", language = "rus"), + PlayerTextTrack("1:", 1, 0, formatId = "29726.srt", formatLabel = "29726.srt", language = "eng"), + ) + + assertEquals(candidates[1], selector.select(sideLoadedRow, candidates)) + } + + @Test + fun select_survivesTrackGroupsBeingReordered() { + val row = manifestTrack( + groupId = "0:rus-rendition", + trackIndex = 0, + formatId = "rus-rendition", + url = "", + groupIndex = 0, + ) + // A later tracks update exposes the same groups in a different order. + val candidates = listOf( + PlayerTextTrack("2:eng-rendition", 0, 0, formatId = "eng-rendition", language = "eng"), + PlayerTextTrack("0:rus-rendition", 1, 0, formatId = "rus-rendition", language = "rus"), + ) + + assertEquals(candidates[1], selector.select(row, candidates)) + } + + @Test + fun select_picksCorrectVariant_whenGroupIdsAreBlankAndLanguagesCollide() { + val row = manifestTrack( + groupId = "", + trackIndex = 0, + formatId = "forced-rus", + url = "", + groupIndex = 1, + ) + val candidates = listOf( + PlayerTextTrack("", 0, 0, formatId = "full-rus", language = "rus"), + PlayerTextTrack("", 1, 0, formatId = "forced-rus", language = "rus"), + ) + + assertEquals(candidates[1], selector.select(row, candidates)) + } + + @Test + fun select_fallsBackToCoordinates_whenTheManifestExposesNoIdentity() { + val row = manifestTrack(groupId = "", trackIndex = 0, formatId = null, url = "", groupIndex = 1) + val candidates = listOf( + PlayerTextTrack("", 0, 0, language = "rus"), + PlayerTextTrack("", 1, 0, language = "rus"), + ) + + assertEquals(candidates[1], selector.select(row, candidates)) + } + + @Test + fun select_returnsNull_ratherThanGuessing_whenNothingIdentifiesTheTrack() { + val row = SubtitleTrackUIState(index = 1, label = "rus", language = "", url = "") + val candidates = listOf( + PlayerTextTrack("", 0, 0, language = "rus"), + PlayerTextTrack("", 1, 0, language = "rus"), + ) + + assertNull(selector.select(row, candidates)) + } + + @Test + fun select_matchesByLanguage_asLastResort() { + val row = SubtitleTrackUIState(index = 1, label = "ukr", language = "ukr", url = "") + val candidates = listOf( + PlayerTextTrack("", 0, 0, language = "rus"), + PlayerTextTrack("", 1, 0, language = "uk"), + ) + + assertEquals(candidates[1], selector.select(row, candidates)) + } + + @Test + fun select_returnsNull_whenThePlayerExposesNoTextTracks() { + assertNull(selector.select(manifestTrack("0:a", 0, "a", ""), emptyList())) + } + + private fun manifestTrack( + groupId: String, + trackIndex: Int, + formatId: String?, + url: String, + groupIndex: Int = 0, + ) = SubtitleTrackUIState( + index = 1, + label = "rus", + language = "rus", + url = url, + playerTrackGroupId = groupId, + playerTrackId = formatId, + playerTrackUri = "https://cdn.test/rendition.m3u8", + playerGroupIndex = groupIndex, + playerTrackIndex = trackIndex, + ) +} From 6e636a08e4b8977152a94f4532f7b20e707ea43a Mon Sep 17 00:00:00 2001 From: Innokentii Enikeev Date: Sat, 29 Aug 2026 11:41:46 +0200 Subject: [PATCH 11/25] Present subtitles by language name and group variants Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01GEBLAtGvP5AHqHemWgLXAp --- .../player/component/AudioSubtitlesPanel.kt | 9 +- .../feature/player/vm/PlaybackController.kt | 6 +- .../puber/ui/feature/player/vm/PlayerVM.kt | 13 +- .../ui/feature/player/vm/SubtitleLabeler.kt | 88 +++++++++++++ .../ui/feature/player/vm/SubtitleLanguage.kt | 49 +++++++ .../feature/player/vm/SubtitleTrackMerger.kt | 86 +++++-------- app/src/main/res/values/player.xml | 2 + .../component/AudioSubtitlesPanelTest.kt | 30 ----- .../feature/player/vm/SubtitleLabelerTest.kt | 120 ++++++++++++++++++ .../player/vm/SubtitleTrackMergerTest.kt | 61 +++++---- 10 files changed, 336 insertions(+), 128 deletions(-) create mode 100644 app/src/main/java/com/kino/puber/ui/feature/player/vm/SubtitleLabeler.kt create mode 100644 app/src/main/java/com/kino/puber/ui/feature/player/vm/SubtitleLanguage.kt delete mode 100644 app/src/test/kotlin/com/kino/puber/ui/feature/player/component/AudioSubtitlesPanelTest.kt create mode 100644 app/src/test/kotlin/com/kino/puber/ui/feature/player/vm/SubtitleLabelerTest.kt diff --git a/app/src/main/java/com/kino/puber/ui/feature/player/component/AudioSubtitlesPanel.kt b/app/src/main/java/com/kino/puber/ui/feature/player/component/AudioSubtitlesPanel.kt index b45ca72f..26485d61 100644 --- a/app/src/main/java/com/kino/puber/ui/feature/player/component/AudioSubtitlesPanel.kt +++ b/app/src/main/java/com/kino/puber/ui/feature/player/component/AudioSubtitlesPanel.kt @@ -205,10 +205,7 @@ private fun RowScope.SubtitleColumn( panelFocusRequester: FocusRequester?, onSubtitleSelected: (Int) -> Unit, ) { - val forcedLabel = stringResource(R.string.player_subtitle_forced) - val labels = remember(subtitleTracks, forcedLabel) { - subtitleTracks.map { it.subtitlePickerLabel(forcedLabel) } - } + val labels = remember(subtitleTracks) { subtitleTracks.map { it.label } } SettingsPanelColumn( header = stringResource(R.string.player_panel_subtitles), items = labels, @@ -220,10 +217,6 @@ private fun RowScope.SubtitleColumn( ) } -internal fun SubtitleTrackUIState.subtitlePickerLabel(forcedLabel: String): String { - return if (isForced == true) "$label · $forcedLabel" else label -} - @Composable private fun BoxScope.SubtitleSizeButton( onClick: () -> Unit, diff --git a/app/src/main/java/com/kino/puber/ui/feature/player/vm/PlaybackController.kt b/app/src/main/java/com/kino/puber/ui/feature/player/vm/PlaybackController.kt index 9a1e0e88..673b3b92 100644 --- a/app/src/main/java/com/kino/puber/ui/feature/player/vm/PlaybackController.kt +++ b/app/src/main/java/com/kino/puber/ui/feature/player/vm/PlaybackController.kt @@ -682,12 +682,10 @@ internal class PlaybackController( ): SubtitleTrackUIState { val identityFormat = hlsRendition?.format val language = format.language ?: identityFormat?.language.orEmpty() - val fallbackLabel = format.label - ?: identityFormat?.label - ?: context.getString(R.string.player_subtitle_unknown, index) + // SubtitleLabeler builds every visible label once the full track list is known. return SubtitleTrackUIState( index = index, - label = subtitleTrackDisplayLabel(language, fallbackLabel), + label = "", language = language, url = "", descriptiveLabel = format.label ?: identityFormat?.label, diff --git a/app/src/main/java/com/kino/puber/ui/feature/player/vm/PlayerVM.kt b/app/src/main/java/com/kino/puber/ui/feature/player/vm/PlayerVM.kt index 342db28a..9ab82afc 100644 --- a/app/src/main/java/com/kino/puber/ui/feature/player/vm/PlayerVM.kt +++ b/app/src/main/java/com/kino/puber/ui/feature/player/vm/PlayerVM.kt @@ -159,9 +159,16 @@ internal class PlayerVM( private val progressTracker = ProgressTracker() private val audioTrackPreferenceResolver = AudioTrackPreferenceResolver() private val subtitleTrackMerger = SubtitleTrackMerger( - variantLabel = { label, ordinal -> - resources.getString(R.string.player_subtitle_variant_label, label, ordinal) - }, + labeler = SubtitleLabeler( + displayLanguageTag = resources.getString(R.string.player_subtitle_language_locale), + forcedQualifier = resources.getString(R.string.player_subtitle_forced), + variantLabel = { label, ordinal -> + resources.getString(R.string.player_subtitle_variant_label, label, ordinal) + }, + unknownLabel = { position -> + resources.getString(R.string.player_subtitle_unknown, position) + }, + ), ) private val debugOverlayEnabled = interactor.isDebugOverlayEnabled() diff --git a/app/src/main/java/com/kino/puber/ui/feature/player/vm/SubtitleLabeler.kt b/app/src/main/java/com/kino/puber/ui/feature/player/vm/SubtitleLabeler.kt new file mode 100644 index 00000000..ba7704d0 --- /dev/null +++ b/app/src/main/java/com/kino/puber/ui/feature/player/vm/SubtitleLabeler.kt @@ -0,0 +1,88 @@ +package com.kino.puber.ui.feature.player.vm + +import com.kino.puber.ui.feature.player.model.SubtitleTrackUIState +import java.util.Locale + +/** + * Builds the strings the subtitle picker shows. + * + * Every row is named after its language. A qualifier is appended only when two rows would + * otherwise read identically, and raw manifest strings only reach the UI when they are + * genuinely descriptive: CDN naming such as `RUS #03` and the subtitle file names Media3 + * uses as side-loaded track labels are rejected. + */ +internal class SubtitleLabeler( + displayLanguageTag: String, + private val forcedQualifier: String, + private val variantLabel: (label: String, ordinal: Int) -> String, + private val unknownLabel: (position: Int) -> String, +) { + private val displayLocale: Locale = Locale.forLanguageTag(displayLanguageTag) + + fun apply(tracks: List): List { + val labeled = tracks.mapIndexed { position, track -> + track.copy(label = withForcedQualifier(baseLabel(track, position), track)) + } + return disambiguate(labeled) + } + + private fun baseLabel(track: SubtitleTrackUIState, position: Int): String = + subtitleLanguageDisplayName(track.language, displayLocale) + ?: track.readableDescriptiveLabel() + ?: unknownLabel(position + 1) + + private fun withForcedQualifier(label: String, track: SubtitleTrackUIState): String = + if (track.isForced == true) "$label$QUALIFIER_SEPARATOR$forcedQualifier" else label + + /** + * Rows that still read the same are separated by their manifest labels when every one + * of them has a distinct readable label, and by an ordinal otherwise. + */ + private fun disambiguate(tracks: List): List { + val collisions = tracks.groupBy { it.label }.filterValues { it.size > 1 } + if (collisions.isEmpty()) return tracks + + val describable = collisions.filterValues(::hasDistinctReadableLabels).keys + val ordinals = mutableMapOf() + return tracks.map { track -> + when { + track.label !in collisions -> track + track.label in describable -> + track.copy(label = withForcedQualifier(track.readableDescriptiveLabel()!!, track)) + else -> { + val ordinal = ordinals.merge(track.label, 1, Int::plus) ?: 1 + track.copy(label = variantLabel(track.label, ordinal)) + } + } + } + } + + private fun hasDistinctReadableLabels(group: List): Boolean { + val labels = group.mapNotNull { it.readableDescriptiveLabel() } + return labels.size == group.size && labels.toSet().size == group.size + } + + private companion object { + const val QUALIFIER_SEPARATOR = " · " + } +} + +/** + * A manifest label is worth showing only when it reads as a name. Anything that is really + * a file name or a language code with a channel number is not. + */ +internal fun SubtitleTrackUIState.readableDescriptiveLabel(): String? { + val candidate = descriptiveLabel?.trim()?.takeIf { it.isNotEmpty() } ?: return null + val isFileName = SUBTITLE_FILE_NAME.containsMatchIn(candidate) || + candidate == sourceFile || + candidate == url.stableSubtitleKey() + val isLanguageCode = candidate.count(Char::isLetter) <= SHORTEST_DESCRIPTIVE_LABEL + return candidate.takeUnless { isFileName || isLanguageCode } +} + +private const val SHORTEST_DESCRIPTIVE_LABEL = 3 + +private val SUBTITLE_FILE_NAME = Regex( + pattern = """\.(srt|vtt|webvtt|ass|ssa|ttml|xml)$""", + option = RegexOption.IGNORE_CASE, +) diff --git a/app/src/main/java/com/kino/puber/ui/feature/player/vm/SubtitleLanguage.kt b/app/src/main/java/com/kino/puber/ui/feature/player/vm/SubtitleLanguage.kt new file mode 100644 index 00000000..a53de275 --- /dev/null +++ b/app/src/main/java/com/kino/puber/ui/feature/player/vm/SubtitleLanguage.kt @@ -0,0 +1,49 @@ +package com.kino.puber.ui.feature.player.vm + +import java.util.Locale + +internal fun sameSubtitleLanguage(first: String, second: String): Boolean { + if (first.isBlank() || second.isBlank()) return false + return canonicalSubtitleLanguage(first) == canonicalSubtitleLanguage(second) +} + +private fun canonicalSubtitleLanguage(language: String): String { + val normalized = language + .trim() + .lowercase(Locale.ROOT) + .substringBefore('-') + .substringBefore('_') + return runCatching { Locale.forLanguageTag(normalized).isO3Language } + .getOrNull() + ?.takeIf { it.isNotBlank() && it != "und" } + ?: normalized +} + +/** + * Localized name of a subtitle language, or null when the code cannot be resolved. + * + * `Locale.forLanguageTag("rus")` does not resolve to a known locale, so its display name + * comes back untranslated. Resolving the three-letter code against the available locales + * first is what makes "rus" read as "Русский" rather than "Russian". + */ +internal fun subtitleLanguageDisplayName(language: String, displayLocale: Locale): String? { + if (language.isBlank()) return null + val canonical = canonicalSubtitleLanguage(language) + if (canonical.isEmpty()) return null + val locale = iso3Locales[canonical] ?: Locale.forLanguageTag(canonical) + val name = runCatching { locale.getDisplayLanguage(displayLocale) }.getOrNull().orEmpty() + return name + .takeIf { it.isNotBlank() && !it.equals(canonical, ignoreCase = true) } + ?.replaceFirstChar { it.titlecase(displayLocale) } +} + +private val iso3Locales: Map by lazy { + Locale.getAvailableLocales() + .filter { it.language.isNotEmpty() } + .mapNotNull { locale -> + runCatching { locale.isO3Language }.getOrNull() + ?.takeIf { it.isNotBlank() } + ?.let { iso3 -> iso3 to Locale.forLanguageTag(locale.language) } + } + .toMap() +} diff --git a/app/src/main/java/com/kino/puber/ui/feature/player/vm/SubtitleTrackMerger.kt b/app/src/main/java/com/kino/puber/ui/feature/player/vm/SubtitleTrackMerger.kt index ae3d42c0..3d7a80fc 100644 --- a/app/src/main/java/com/kino/puber/ui/feature/player/vm/SubtitleTrackMerger.kt +++ b/app/src/main/java/com/kino/puber/ui/feature/player/vm/SubtitleTrackMerger.kt @@ -5,7 +5,7 @@ import com.kino.puber.ui.feature.player.model.isOff import java.util.Locale internal class SubtitleTrackMerger( - private val variantLabel: (label: String, ordinal: Int) -> String, + private val labeler: SubtitleLabeler, ) { fun merge( @@ -25,10 +25,37 @@ internal class SubtitleTrackMerger( } val enrichedPlayerTracks = enrichPlayerTracks(playerSubtitles, apiSubtitles) - return (listOf(offTrack) + disambiguateLabels(enrichedPlayerTracks)) + val orderedPlayerTracks = orderByLanguage(enrichedPlayerTracks) + return (listOf(offTrack) + labeler.apply(orderedPlayerTracks)) .mapIndexed { index, track -> track.copy(index = index) } } + /** + * Player order follows the manifest, which is arbitrary from the viewer's side. Group + * the rows by language in the order each language first appears, keeping every variant + * of a language together and its partial variant last. + */ + private fun orderByLanguage( + tracks: List, + ): List { + val languageOrder = mutableMapOf() + tracks.forEach { track -> + languageOrder.getOrPut(orderingLanguage(track)) { languageOrder.size } + } + return tracks.withIndex() + .sortedWith( + compareBy( + { languageOrder.getValue(orderingLanguage(it.value)) }, + { it.value.isForced == true }, + { it.index }, + ), + ) + .map { it.value } + } + + private fun orderingLanguage(track: SubtitleTrackUIState): String = + track.language.trim().lowercase(Locale.ROOT) + /** * Every external subtitle is side-loaded because the manifest contents are unknown * before preparation, so a subtitle the manifest also publishes shows up twice. The @@ -63,40 +90,6 @@ internal class SubtitleTrackMerger( } } - /** - * Two tracks that share a language and a forced flag collapse to the same picker row. - * Prefer the raw manifest labels when they tell the variants apart, and fall back to - * an explicit ordinal when they do not. - */ - private fun disambiguateLabels( - tracks: List, - ): List { - val collisions = tracks.groupBy(::pickerRowKey).filterValues { it.size > 1 } - if (collisions.isEmpty()) return tracks - - val descriptiveKeys = collisions.filterValues(::hasDistinctDescriptiveLabels).keys - val ordinals = mutableMapOf() - return tracks.map { track -> - val key = pickerRowKey(track) - when { - key !in collisions -> track - key in descriptiveKeys -> track.copy(label = track.descriptiveLabel.orEmpty().trim()) - else -> { - val ordinal = ordinals.merge(key, 1, Int::plus) ?: 1 - track.copy(label = variantLabel(track.label, ordinal)) - } - } - } - } - - private fun hasDistinctDescriptiveLabels(group: List): Boolean { - val labels = group.mapNotNull { track -> track.descriptiveLabel?.trim()?.takeIf(String::isNotEmpty) } - return labels.size == group.size && labels.toSet().size == group.size - } - - private fun pickerRowKey(track: SubtitleTrackUIState): String = - "${track.label}\u0000${track.isForced == true}" - private fun findExactIdentityMatch( playerTrack: SubtitleTrackUIState, apiTracks: List, @@ -126,24 +119,3 @@ internal class SubtitleTrackMerger( isForced = apiTrack.isForced ?: isForced, ) } - -internal fun sameSubtitleLanguage(first: String, second: String): Boolean { - if (first.isBlank() || second.isBlank()) return false - return canonicalSubtitleLanguage(first) == canonicalSubtitleLanguage(second) -} - -internal fun subtitleTrackDisplayLabel(language: String, fallbackLabel: String): String { - return canonicalSubtitleLanguage(language).ifEmpty { fallbackLabel } -} - -private fun canonicalSubtitleLanguage(language: String): String { - val normalized = language - .trim() - .lowercase(Locale.ROOT) - .substringBefore('-') - .substringBefore('_') - return runCatching { Locale.forLanguageTag(normalized).isO3Language } - .getOrNull() - ?.takeIf { it.isNotBlank() && it != "und" } - ?: normalized -} diff --git a/app/src/main/res/values/player.xml b/app/src/main/res/values/player.xml index f9b06b21..88e64b7a 100644 --- a/app/src/main/res/values/player.xml +++ b/app/src/main/res/values/player.xml @@ -14,6 +14,8 @@ АУДИО СУБТИТРЫ Выкл. + + ru частичные %1$s · вариант %2$d Субтитры %1$d diff --git a/app/src/test/kotlin/com/kino/puber/ui/feature/player/component/AudioSubtitlesPanelTest.kt b/app/src/test/kotlin/com/kino/puber/ui/feature/player/component/AudioSubtitlesPanelTest.kt deleted file mode 100644 index 1844f21d..00000000 --- a/app/src/test/kotlin/com/kino/puber/ui/feature/player/component/AudioSubtitlesPanelTest.kt +++ /dev/null @@ -1,30 +0,0 @@ -package com.kino.puber.ui.feature.player.component - -import com.kino.puber.ui.feature.player.model.SubtitleTrackUIState -import org.junit.jupiter.api.Assertions.assertEquals -import org.junit.jupiter.api.Test - -internal class AudioSubtitlesPanelTest { - - @Test - fun subtitlePickerLabel_marksForcedTrack_withoutManifestNumbering() { - val track = subtitleTrack(label = "rus", isForced = true) - - assertEquals("rus · частичные", track.subtitlePickerLabel("частичные")) - } - - @Test - fun subtitlePickerLabel_keepsRegularTrackLanguageOnly() { - val track = subtitleTrack(label = "rus", isForced = false) - - assertEquals("rus", track.subtitlePickerLabel("частичные")) - } - - private fun subtitleTrack(label: String, isForced: Boolean) = SubtitleTrackUIState( - index = 1, - label = label, - language = "rus", - url = "", - isForced = isForced, - ) -} diff --git a/app/src/test/kotlin/com/kino/puber/ui/feature/player/vm/SubtitleLabelerTest.kt b/app/src/test/kotlin/com/kino/puber/ui/feature/player/vm/SubtitleLabelerTest.kt new file mode 100644 index 00000000..17eb94ab --- /dev/null +++ b/app/src/test/kotlin/com/kino/puber/ui/feature/player/vm/SubtitleLabelerTest.kt @@ -0,0 +1,120 @@ +package com.kino.puber.ui.feature.player.vm + +import com.kino.puber.ui.feature.player.model.SubtitleTrackUIState +import org.junit.jupiter.api.Assertions.assertEquals +import org.junit.jupiter.api.Test + +internal class SubtitleLabelerTest { + + private val labeler = SubtitleLabeler( + displayLanguageTag = "ru", + forcedQualifier = "частичные", + variantLabel = { label, ordinal -> "$label · вариант $ordinal" }, + unknownLabel = { position -> "Субтитры $position" }, + ) + + @Test + fun apply_namesRowsByLocalizedLanguage_forTwoAndThreeLetterCodes() { + val labels = labeler.apply( + listOf(track("rus"), track("en"), track("spa"), track("uk")), + ).map { it.label } + + assertEquals(listOf("Русский", "Английский", "Испанский", "Украинский"), labels) + } + + @Test + fun apply_marksPartialTrack() { + val labels = labeler.apply(listOf(track("rus"), track("rus", forced = true))) + .map { it.label } + + assertEquals(listOf("Русский", "Русский · частичные"), labels) + } + + @Test + fun apply_neverShowsSubtitleFileNames() { + val labels = labeler.apply( + listOf( + track("spa", descriptive = "29725.srt"), + track("spa", descriptive = "29727.srt"), + ), + ).map { it.label } + + assertEquals(listOf("Испанский · вариант 1", "Испанский · вариант 2"), labels) + } + + @Test + fun apply_neverShowsCdnChannelNumbering() { + val labels = labeler.apply( + listOf( + track("rus", descriptive = "RUS #03"), + track("rus", descriptive = "RUS #05"), + ), + ).map { it.label } + + assertEquals(listOf("Русский · вариант 1", "Русский · вариант 2"), labels) + } + + @Test + fun apply_usesManifestLabels_whenTheyReadAsNamesAndSeparateTheRows() { + val labels = labeler.apply( + listOf( + track("rus", descriptive = "Русские полные"), + track("rus", descriptive = "Русские SDH"), + ), + ).map { it.label } + + assertEquals(listOf("Русские полные", "Русские SDH"), labels) + } + + @Test + fun apply_keepsPartialMarker_whenAManifestLabelSeparatesTheRows() { + val labels = labeler.apply( + listOf( + track("rus", descriptive = "Русские полные", forced = true), + track("rus", descriptive = "Русские SDH", forced = true), + ), + ).map { it.label } + + assertEquals( + listOf("Русские полные · частичные", "Русские SDH · частичные"), + labels, + ) + } + + @Test + fun apply_doesNotQualifyRowsThatAlreadyReadDifferently() { + val labels = labeler.apply( + listOf(track("rus", descriptive = "RUS #03"), track("eng", descriptive = "ENG #01")), + ).map { it.label } + + assertEquals(listOf("Русский", "Английский"), labels) + } + + @Test + fun apply_fallsBackToPositionalName_whenNothingIdentifiesTheLanguage() { + val labels = labeler.apply(listOf(track(""), track("", descriptive = "Комментарии режиссёра"))) + .map { it.label } + + assertEquals(listOf("Субтитры 1", "Комментарии режиссёра"), labels) + } + + @Test + fun apply_leavesUnknownLanguageCodeVisible_ratherThanInventingAName() { + val labels = labeler.apply(listOf(track("qqq"))).map { it.label } + + assertEquals(listOf("Субтитры 1"), labels) + } + + private fun track( + language: String, + descriptive: String? = null, + forced: Boolean = false, + ) = SubtitleTrackUIState( + index = 0, + label = "", + language = language, + url = "", + isForced = forced, + descriptiveLabel = descriptive, + ) +} diff --git a/app/src/test/kotlin/com/kino/puber/ui/feature/player/vm/SubtitleTrackMergerTest.kt b/app/src/test/kotlin/com/kino/puber/ui/feature/player/vm/SubtitleTrackMergerTest.kt index 5562808b..cb0bfefe 100644 --- a/app/src/test/kotlin/com/kino/puber/ui/feature/player/vm/SubtitleTrackMergerTest.kt +++ b/app/src/test/kotlin/com/kino/puber/ui/feature/player/vm/SubtitleTrackMergerTest.kt @@ -8,14 +8,14 @@ import org.junit.jupiter.api.Test internal class SubtitleTrackMergerTest { - private val merger = SubtitleTrackMerger(variantLabel = { label, ordinal -> "$label #$ordinal" }) + private val merger = SubtitleTrackMerger(labeler = testLabeler()) - @Test - fun subtitleTrackDisplayLabel_hidesManifestNumberingAndUsesLowercaseIso3Language() { - assertEquals("rus", subtitleTrackDisplayLabel("RU", "RUS #03")) - assertEquals("spa", subtitleTrackDisplayLabel("es-ES", "SPA #01")) - assertEquals("Unknown", subtitleTrackDisplayLabel("", "Unknown")) - } + private fun testLabeler() = SubtitleLabeler( + displayLanguageTag = "ru", + forcedQualifier = "частичные", + variantLabel = { label, ordinal -> "$label · вариант $ordinal" }, + unknownLabel = { position -> "Субтитры $position" }, + ) @Test fun merge_usesPlayerTracksAsBackbone_andEnrichesExactIdentity() { @@ -54,7 +54,7 @@ internal class SubtitleTrackMergerTest { val result = merger.merge(apiTracks, playerTracks) assertEquals( - listOf("Off", "external-rus.srt", "Русские полные"), + listOf("Off", "Русский · вариант 1", "Русский · вариант 2"), result.map { it.label }, ) assertEquals("external-rus.srt", result[1].playerTrackId) @@ -77,11 +77,11 @@ internal class SubtitleTrackMergerTest { val result = merger.merge(apiTracks, playerTracks) - assertEquals(listOf("Off", "Russian forced HLS", "Russian full HLS"), result.map { it.label }) - assertEquals("forced", result[1].playerTrackId) - assertTrue(result[1].isForced!!) - assertEquals("full", result[2].playerTrackId) - assertFalse(result[2].isForced!!) + assertEquals(listOf("Off", "Русский", "Русский · частичные"), result.map { it.label }) + assertEquals("full", result[1].playerTrackId) + assertFalse(result[1].isForced!!) + assertEquals("forced", result[2].playerTrackId) + assertTrue(result[2].isForced!!) } @Test @@ -119,7 +119,7 @@ internal class SubtitleTrackMergerTest { val result = merger.merge(apiTracks, playerTracks) - assertEquals(listOf("Off", "Russian HLS"), result.map { it.label }) + assertEquals(listOf("Off", "Русский"), result.map { it.label }) assertEquals("", result[1].url) assertEquals("hls-russian", result[1].playerTrackId) } @@ -148,7 +148,7 @@ internal class SubtitleTrackMergerTest { } @Test - fun merge_keepsManifestOrder_forSameLanguageVariants() { + fun merge_groupsVariantsByLanguage_keepingManifestOrderWithinEachLanguage() { val apiTracks = listOf( offTrack(), apiTrack(1, "rus #1", "rus"), @@ -166,11 +166,17 @@ internal class SubtitleTrackMergerTest { val result = merger.merge(apiTracks, playerTracks) assertEquals( - listOf("Off", "Russian 1", "Spanish", "Russian 2", "Russian 3"), + listOf( + "Off", + "Русский · вариант 1", + "Русский · вариант 2", + "Русский · вариант 3", + "Испанский", + ), result.map { it.label }, ) - assertEquals(listOf(null, 0, 1, 2, 3), result.map { it.playerGroupIndex }) - assertEquals("rus-3", result[4].playerTrackId) + assertEquals(listOf(null, 0, 2, 3, 1), result.map { it.playerGroupIndex }) + assertEquals(listOf("rus-1", "rus-2", "rus-3", "spa"), result.drop(1).map { it.playerTrackId }) } @Test @@ -240,7 +246,7 @@ internal class SubtitleTrackMergerTest { val result = merger.merge(listOf(offTrack(), apiTrack), listOf(playerTrack)) - assertEquals(listOf("Off", "russian.vtt"), result.map { it.label }) + assertEquals(listOf("Off", "Русский"), result.map { it.label }) assertEquals(playerTrack.playerTrackId, result[1].playerTrackId) assertEquals("ru", result[1].language) assertEquals(apiTrack.url, result[1].url) @@ -266,7 +272,7 @@ internal class SubtitleTrackMergerTest { listOf(manifestEnglish), ) - assertEquals(listOf("Off", "English HLS"), result.map { it.label }) + assertEquals(listOf("Off", "Английский"), result.map { it.label }) assertEquals(listOf("", "en"), result.map { it.language }) assertEquals("hls-english", result[1].playerTrackId) } @@ -307,11 +313,14 @@ internal class SubtitleTrackMergerTest { val result = merger.merge(listOf(offTrack()), playerTracks) - assertEquals(listOf("Off", "rus #1", "rus #2", "rus #3"), result.map { it.label }) + assertEquals( + listOf("Off", "Русский · вариант 1", "Русский · вариант 2", "Русский · вариант 3"), + result.map { it.label }, + ) } @Test - fun merge_keepsForcedVariantUntouched_becauseThePickerAlreadyMarksIt() { + fun merge_marksPartialVariant_andOrdersItLastWithinItsLanguage() { val playerTracks = listOf( playerTrack(1, "rus", "rus", "full", 0, forced = false, descriptiveLabel = "RUS"), playerTrack(2, "rus", "rus", "forced", 1, forced = true, descriptiveLabel = "RUS"), @@ -319,7 +328,7 @@ internal class SubtitleTrackMergerTest { val result = merger.merge(listOf(offTrack()), playerTracks) - assertEquals(listOf("Off", "rus", "rus"), result.map { it.label }) + assertEquals(listOf("Off", "Русский", "Русский · частичные"), result.map { it.label }) assertEquals(listOf(null, false, true), result.map { it.isForced }) } @@ -332,7 +341,7 @@ internal class SubtitleTrackMergerTest { val result = merger.merge(listOf(offTrack()), playerTracks) - assertEquals(listOf("Off", "rus", "eng"), result.map { it.label }) + assertEquals(listOf("Off", "Русский", "Английский"), result.map { it.label }) } @Test @@ -356,7 +365,7 @@ internal class SubtitleTrackMergerTest { val result = merger.merge(apiTracks, playerTracks) - assertEquals(listOf("Off", "rus"), result.map { it.label }) + assertEquals(listOf("Off", "Русский"), result.map { it.label }) assertEquals("hls-rus", result[1].playerTrackId) assertEquals("https://api.test/subtitles/rus.srt", result[1].url) } @@ -382,7 +391,7 @@ internal class SubtitleTrackMergerTest { val result = merger.merge(apiTracks, playerTracks) - assertEquals(listOf("Off", "rus", "eng"), result.map { it.label }) + assertEquals(listOf("Off", "Русский", "Английский"), result.map { it.label }) assertEquals("https://api.test/subtitles/eng.srt", result[2].url) } From 1a9e70caddc9355814db45d44909e8b9a85c3b70 Mon Sep 17 00:00:00 2001 From: Innokentii Enikeev Date: Sat, 29 Aug 2026 12:48:33 +0200 Subject: [PATCH 12/25] Drop side-loaded subtitles the manifest already covers Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01GEBLAtGvP5AHqHemWgLXAp --- .../ui/feature/player/vm/SubtitleLanguage.kt | 2 +- .../feature/player/vm/SubtitleTrackMerger.kt | 49 +++++++++++-------- .../player/vm/SubtitleTrackMergerTest.kt | 46 ++++++++++++++++- 3 files changed, 75 insertions(+), 22 deletions(-) diff --git a/app/src/main/java/com/kino/puber/ui/feature/player/vm/SubtitleLanguage.kt b/app/src/main/java/com/kino/puber/ui/feature/player/vm/SubtitleLanguage.kt index a53de275..23eb1bd9 100644 --- a/app/src/main/java/com/kino/puber/ui/feature/player/vm/SubtitleLanguage.kt +++ b/app/src/main/java/com/kino/puber/ui/feature/player/vm/SubtitleLanguage.kt @@ -7,7 +7,7 @@ internal fun sameSubtitleLanguage(first: String, second: String): Boolean { return canonicalSubtitleLanguage(first) == canonicalSubtitleLanguage(second) } -private fun canonicalSubtitleLanguage(language: String): String { +internal fun canonicalSubtitleLanguage(language: String): String { val normalized = language .trim() .lowercase(Locale.ROOT) diff --git a/app/src/main/java/com/kino/puber/ui/feature/player/vm/SubtitleTrackMerger.kt b/app/src/main/java/com/kino/puber/ui/feature/player/vm/SubtitleTrackMerger.kt index 3d7a80fc..906d69b6 100644 --- a/app/src/main/java/com/kino/puber/ui/feature/player/vm/SubtitleTrackMerger.kt +++ b/app/src/main/java/com/kino/puber/ui/feature/player/vm/SubtitleTrackMerger.kt @@ -2,7 +2,6 @@ package com.kino.puber.ui.feature.player.vm import com.kino.puber.ui.feature.player.model.SubtitleTrackUIState import com.kino.puber.ui.feature.player.model.isOff -import java.util.Locale internal class SubtitleTrackMerger( private val labeler: SubtitleLabeler, @@ -54,37 +53,34 @@ internal class SubtitleTrackMerger( } private fun orderingLanguage(track: SubtitleTrackUIState): String = - track.language.trim().lowercase(Locale.ROOT) + canonicalSubtitleLanguage(track.language) /** * Every external subtitle is side-loaded because the manifest contents are unknown * before preparation, so a subtitle the manifest also publishes shows up twice. The * two tracks resolve to the same API entry, and the manifest rendition wins. */ + /** + * Every external subtitle is side-loaded because the manifest contents are unknown + * before preparation. Once they are known, a side-loaded subtitle whose language the + * manifest already publishes is redundant: the renditions are segmented with the + * stream and are what the KinoPub web player offers. + */ private fun enrichPlayerTracks( playerSubtitles: List, apiSubtitles: List, ): List { + val manifestLanguages = playerSubtitles + .filter { it.isFromManifest } + .mapTo(mutableSetOf()) { canonicalSubtitleLanguage(it.language) } val apiMatches = playerSubtitles.map { track -> findExactIdentityMatch(track, apiSubtitles) } - val redundant = mutableSetOf() - val owners = mutableMapOf() - apiMatches.withIndex() - .mapNotNull { (position, apiIndex) -> apiIndex?.let { it to position } } - .groupBy({ it.first }, { it.second }) - .forEach { (apiIndex, positions) -> - val fromManifest = positions.filter { playerSubtitles[it].playerTrackUri != null } - if (fromManifest.size == 1 && positions.size > 1) { - redundant += positions - fromManifest.toSet() - owners[apiIndex] = fromManifest.single() - } else { - owners[apiIndex] = positions.first() - } - } - + val claimedApiIndices = mutableSetOf() return playerSubtitles.mapIndexedNotNull { position, playerTrack -> - if (position in redundant) return@mapIndexedNotNull null + if (playerTrack.isRedundantSideLoad(manifestLanguages)) { + return@mapIndexedNotNull null + } apiMatches[position] - ?.takeIf { apiIndex -> owners[apiIndex] == position } + ?.takeIf(claimedApiIndices::add) ?.let { apiIndex -> playerTrack.withApiMetadata(apiSubtitles[apiIndex]) } ?: playerTrack } @@ -96,7 +92,7 @@ internal class SubtitleTrackMerger( ): Int? { val playerIdentities = listOfNotNull( playerTrack.playerTrackUri, - playerTrack.playerTrackId, + playerTrack.playerTrackId?.withoutMergedSourcePrefix(), ).filter { it.isNotEmpty() } if (playerIdentities.isEmpty()) return null return apiTracks.indices.filter { apiIndex -> @@ -119,3 +115,16 @@ internal class SubtitleTrackMerger( isForced = apiTrack.isForced ?: isForced, ) } + +private val SubtitleTrackUIState.isFromManifest: Boolean + get() = playerTrackUri != null + +private fun SubtitleTrackUIState.isRedundantSideLoad(manifestLanguages: Set): Boolean = + !isFromManifest && + language.isNotBlank() && + canonicalSubtitleLanguage(language) in manifestLanguages + +// MergingMediaSource rewrites child track ids to ":". +private fun String.withoutMergedSourcePrefix(): String = MERGED_SOURCE_PREFIX.replace(this, "") + +private val MERGED_SOURCE_PREFIX = Regex("""^\d+:""") diff --git a/app/src/test/kotlin/com/kino/puber/ui/feature/player/vm/SubtitleTrackMergerTest.kt b/app/src/test/kotlin/com/kino/puber/ui/feature/player/vm/SubtitleTrackMergerTest.kt index cb0bfefe..32f407fc 100644 --- a/app/src/test/kotlin/com/kino/puber/ui/feature/player/vm/SubtitleTrackMergerTest.kt +++ b/app/src/test/kotlin/com/kino/puber/ui/feature/player/vm/SubtitleTrackMergerTest.kt @@ -344,8 +344,52 @@ internal class SubtitleTrackMergerTest { assertEquals(listOf("Off", "Русский", "Английский"), result.map { it.label }) } + /** + * Real KinoPub data: the API lists two subtitles, the manifest publishes three + * renditions covering both languages, and Media3 prefixes side-loaded track ids with + * the merged child index. The renditions win and the side-loaded copies disappear. + */ @Test - fun merge_hidesSideLoadedCopy_whenManifestPublishesTheSameSubtitle() { + fun merge_hidesSideLoadedCopies_whenManifestCoversTheirLanguages() { + val apiTracks = listOf( + offTrack(), + apiTrack(1, "eng", "eng", url = "https://api.test/pd/xyz", sourceFile = "/9/24/2871466.srt"), + apiTrack(2, "rus", "rus", url = "https://api.test/pd/abc", sourceFile = "/1/92/2871463.srt"), + ) + val playerTracks = listOf( + playerTrack(1, "", "ru", "0:", 0, uri = "https://cdn.test/hls/rus01.m3u8", descriptiveLabel = "RUS #01"), + playerTrack(2, "", "ru", "0:", 1, uri = "https://cdn.test/hls/rus02.m3u8", descriptiveLabel = "RUS #02"), + playerTrack(3, "", "en", "0:", 2, uri = "https://cdn.test/hls/eng03.m3u8", descriptiveLabel = "ENG #03"), + playerTrack(4, "", "en", "1:9/24/2871466.srt", 3), + playerTrack(5, "", "ru", "2:1/92/2871463.srt", 4), + ) + + val result = merger.merge(apiTracks, playerTracks) + + assertEquals( + listOf("Off", "Русский · вариант 1", "Русский · вариант 2", "Английский"), + result.map { it.label }, + ) + } + + @Test + fun merge_matchesSideLoadedTrack_ignoringTheMergedChildIndexPrefix() { + val apiTracks = listOf( + offTrack(), + apiTrack(1, "ukr", "ukr", url = "https://api.test/pd/abc", sourceFile = "/1/92/2871463.srt"), + ) + val playerTracks = listOf( + playerTrack(1, "", "uk", "2:1/92/2871463.srt", 0), + ) + + val result = merger.merge(apiTracks, playerTracks) + + assertEquals(listOf("Off", "Украинский"), result.map { it.label }) + assertEquals("https://api.test/pd/abc", result[1].url) + } + + @Test + fun merge_hidesSideLoadedCopy_whenManifestCoversItsLanguage() { val apiTracks = listOf( offTrack(), apiTrack(1, "rus", "rus", url = "https://api.test/subtitles/rus.srt"), From e5090969da3028e97fa2f0b5cecea31c14c8f581 Mon Sep 17 00:00:00 2001 From: Innokentii Enikeev Date: Sat, 29 Aug 2026 12:57:27 +0200 Subject: [PATCH 13/25] Hide side-loaded subtitles only when the manifest covers them Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01GEBLAtGvP5AHqHemWgLXAp --- .../feature/player/vm/SubtitleTrackMerger.kt | 41 +++++++++++-------- .../player/vm/SubtitleTrackMergerTest.kt | 27 ++++++++++++ 2 files changed, 51 insertions(+), 17 deletions(-) diff --git a/app/src/main/java/com/kino/puber/ui/feature/player/vm/SubtitleTrackMerger.kt b/app/src/main/java/com/kino/puber/ui/feature/player/vm/SubtitleTrackMerger.kt index 906d69b6..39d3f8ac 100644 --- a/app/src/main/java/com/kino/puber/ui/feature/player/vm/SubtitleTrackMerger.kt +++ b/app/src/main/java/com/kino/puber/ui/feature/player/vm/SubtitleTrackMerger.kt @@ -57,26 +57,24 @@ internal class SubtitleTrackMerger( /** * Every external subtitle is side-loaded because the manifest contents are unknown - * before preparation, so a subtitle the manifest also publishes shows up twice. The - * two tracks resolve to the same API entry, and the manifest rendition wins. - */ - /** - * Every external subtitle is side-loaded because the manifest contents are unknown - * before preparation. Once they are known, a side-loaded subtitle whose language the - * manifest already publishes is redundant: the renditions are segmented with the - * stream and are what the KinoPub web player offers. + * before preparation, so a subtitle the manifest also publishes shows up twice. + * + * The two sets share no identifier — a rendition is addressed by its HLS playlist URL, + * an API subtitle by its file path — so coverage is decided per language by count: the + * side-loaded copies of a language are hidden only when the manifest offers at least as + * many renditions of it. Anything short of that keeps the side-loaded tracks, so a + * subtitle is never hidden behind a rendition that cannot stand in for it. */ private fun enrichPlayerTracks( playerSubtitles: List, apiSubtitles: List, ): List { - val manifestLanguages = playerSubtitles - .filter { it.isFromManifest } - .mapTo(mutableSetOf()) { canonicalSubtitleLanguage(it.language) } + val coveredLanguages = coveredLanguages(playerSubtitles) val apiMatches = playerSubtitles.map { track -> findExactIdentityMatch(track, apiSubtitles) } val claimedApiIndices = mutableSetOf() return playerSubtitles.mapIndexedNotNull { position, playerTrack -> - if (playerTrack.isRedundantSideLoad(manifestLanguages)) { + val language = canonicalSubtitleLanguage(playerTrack.language) + if (!playerTrack.isFromManifest && language in coveredLanguages) { return@mapIndexedNotNull null } apiMatches[position] @@ -86,6 +84,20 @@ internal class SubtitleTrackMerger( } } + private fun coveredLanguages(playerSubtitles: List): Set { + val (manifest, sideLoaded) = playerSubtitles.partition { it.isFromManifest } + val manifestCounts = manifest.countByLanguage() + return sideLoaded.countByLanguage() + .filterKeys { it.isNotBlank() } + .filter { (language, sideLoadedCount) -> + manifestCounts.getOrDefault(language, 0) >= sideLoadedCount + } + .keys + } + + private fun List.countByLanguage(): Map = + groupingBy { canonicalSubtitleLanguage(it.language) }.eachCount() + private fun findExactIdentityMatch( playerTrack: SubtitleTrackUIState, apiTracks: List, @@ -119,11 +131,6 @@ internal class SubtitleTrackMerger( private val SubtitleTrackUIState.isFromManifest: Boolean get() = playerTrackUri != null -private fun SubtitleTrackUIState.isRedundantSideLoad(manifestLanguages: Set): Boolean = - !isFromManifest && - language.isNotBlank() && - canonicalSubtitleLanguage(language) in manifestLanguages - // MergingMediaSource rewrites child track ids to ":". private fun String.withoutMergedSourcePrefix(): String = MERGED_SOURCE_PREFIX.replace(this, "") diff --git a/app/src/test/kotlin/com/kino/puber/ui/feature/player/vm/SubtitleTrackMergerTest.kt b/app/src/test/kotlin/com/kino/puber/ui/feature/player/vm/SubtitleTrackMergerTest.kt index 32f407fc..88a6558c 100644 --- a/app/src/test/kotlin/com/kino/puber/ui/feature/player/vm/SubtitleTrackMergerTest.kt +++ b/app/src/test/kotlin/com/kino/puber/ui/feature/player/vm/SubtitleTrackMergerTest.kt @@ -372,6 +372,33 @@ internal class SubtitleTrackMergerTest { ) } + /** + * Renditions and API subtitles share no identifier, so coverage cannot be proven per + * track. When the manifest offers fewer renditions of a language than the API has + * subtitles for it, nothing is hidden — a duplicate row is better than a lost subtitle. + */ + @Test + fun merge_keepsSideLoadedTracks_whenTheManifestHasFewerRenditionsOfTheirLanguage() { + val apiTracks = listOf( + offTrack(), + apiTrack(1, "rus", "rus", url = "https://api.test/pd/a", sourceFile = "/1/11/1.srt"), + apiTrack(2, "rus", "rus", url = "https://api.test/pd/b", sourceFile = "/2/22/2.srt"), + ) + val playerTracks = listOf( + playerTrack(1, "", "ru", "0:", 0, uri = "https://cdn.test/hls/rus01.m3u8", descriptiveLabel = "RUS #01"), + playerTrack(2, "", "ru", "1:1/11/1.srt", 1), + playerTrack(3, "", "ru", "2:2/22/2.srt", 2), + ) + + val result = merger.merge(apiTracks, playerTracks) + + assertEquals(4, result.size) + assertEquals( + listOf("https://api.test/pd/a", "https://api.test/pd/b"), + result.drop(2).map { it.url }, + ) + } + @Test fun merge_matchesSideLoadedTrack_ignoringTheMergedChildIndexPrefix() { val apiTracks = listOf( From b449cf2d2904bdf444285b8d701f37aad772890f Mon Sep 17 00:00:00 2001 From: Innokentii Enikeev Date: Sat, 29 Aug 2026 13:17:26 +0200 Subject: [PATCH 14/25] Treat the HLS manifest as the subtitle list when it has one Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01GEBLAtGvP5AHqHemWgLXAp --- .../feature/player/vm/SubtitleTrackMerger.kt | 29 +++++-------------- .../player/vm/SubtitleTrackMergerTest.kt | 18 +++++------- 2 files changed, 15 insertions(+), 32 deletions(-) diff --git a/app/src/main/java/com/kino/puber/ui/feature/player/vm/SubtitleTrackMerger.kt b/app/src/main/java/com/kino/puber/ui/feature/player/vm/SubtitleTrackMerger.kt index 39d3f8ac..aaa291cc 100644 --- a/app/src/main/java/com/kino/puber/ui/feature/player/vm/SubtitleTrackMerger.kt +++ b/app/src/main/java/com/kino/puber/ui/feature/player/vm/SubtitleTrackMerger.kt @@ -59,22 +59,21 @@ internal class SubtitleTrackMerger( * Every external subtitle is side-loaded because the manifest contents are unknown * before preparation, so a subtitle the manifest also publishes shows up twice. * - * The two sets share no identifier — a rendition is addressed by its HLS playlist URL, - * an API subtitle by its file path — so coverage is decided per language by count: the - * side-loaded copies of a language are hidden only when the manifest offers at least as - * many renditions of it. Anything short of that keeps the side-loaded tracks, so a - * subtitle is never hidden behind a rendition that cannot stand in for it. + * Renditions and API subtitles share no identifier — a rendition is addressed by its + * HLS playlist URL, an API subtitle by its file path — so the duplicate cannot be + * identified per track. A manifest that publishes subtitles at all is treated as the + * complete list and the side-loaded copies are hidden; the side-loaded tracks carry + * playback only when the manifest offers no subtitles of its own. */ private fun enrichPlayerTracks( playerSubtitles: List, apiSubtitles: List, ): List { - val coveredLanguages = coveredLanguages(playerSubtitles) + val manifestPublishesSubtitles = playerSubtitles.any { it.isFromManifest } val apiMatches = playerSubtitles.map { track -> findExactIdentityMatch(track, apiSubtitles) } val claimedApiIndices = mutableSetOf() return playerSubtitles.mapIndexedNotNull { position, playerTrack -> - val language = canonicalSubtitleLanguage(playerTrack.language) - if (!playerTrack.isFromManifest && language in coveredLanguages) { + if (manifestPublishesSubtitles && !playerTrack.isFromManifest) { return@mapIndexedNotNull null } apiMatches[position] @@ -84,20 +83,6 @@ internal class SubtitleTrackMerger( } } - private fun coveredLanguages(playerSubtitles: List): Set { - val (manifest, sideLoaded) = playerSubtitles.partition { it.isFromManifest } - val manifestCounts = manifest.countByLanguage() - return sideLoaded.countByLanguage() - .filterKeys { it.isNotBlank() } - .filter { (language, sideLoadedCount) -> - manifestCounts.getOrDefault(language, 0) >= sideLoadedCount - } - .keys - } - - private fun List.countByLanguage(): Map = - groupingBy { canonicalSubtitleLanguage(it.language) }.eachCount() - private fun findExactIdentityMatch( playerTrack: SubtitleTrackUIState, apiTracks: List, diff --git a/app/src/test/kotlin/com/kino/puber/ui/feature/player/vm/SubtitleTrackMergerTest.kt b/app/src/test/kotlin/com/kino/puber/ui/feature/player/vm/SubtitleTrackMergerTest.kt index 88a6558c..284f4c9e 100644 --- a/app/src/test/kotlin/com/kino/puber/ui/feature/player/vm/SubtitleTrackMergerTest.kt +++ b/app/src/test/kotlin/com/kino/puber/ui/feature/player/vm/SubtitleTrackMergerTest.kt @@ -378,24 +378,23 @@ internal class SubtitleTrackMergerTest { * subtitles for it, nothing is hidden — a duplicate row is better than a lost subtitle. */ @Test - fun merge_keepsSideLoadedTracks_whenTheManifestHasFewerRenditionsOfTheirLanguage() { + fun merge_carriesSideLoadedTracks_whenTheManifestPublishesNoSubtitles() { val apiTracks = listOf( offTrack(), apiTrack(1, "rus", "rus", url = "https://api.test/pd/a", sourceFile = "/1/11/1.srt"), - apiTrack(2, "rus", "rus", url = "https://api.test/pd/b", sourceFile = "/2/22/2.srt"), + apiTrack(2, "eng", "eng", url = "https://api.test/pd/b", sourceFile = "/2/22/2.srt"), ) val playerTracks = listOf( - playerTrack(1, "", "ru", "0:", 0, uri = "https://cdn.test/hls/rus01.m3u8", descriptiveLabel = "RUS #01"), - playerTrack(2, "", "ru", "1:1/11/1.srt", 1), - playerTrack(3, "", "ru", "2:2/22/2.srt", 2), + playerTrack(1, "", "ru", "1:1/11/1.srt", 0), + playerTrack(2, "", "en", "2:2/22/2.srt", 1), ) val result = merger.merge(apiTracks, playerTracks) - assertEquals(4, result.size) + assertEquals(listOf("Off", "Русский", "Английский"), result.map { it.label }) assertEquals( listOf("https://api.test/pd/a", "https://api.test/pd/b"), - result.drop(2).map { it.url }, + result.drop(1).map { it.url }, ) } @@ -442,7 +441,7 @@ internal class SubtitleTrackMergerTest { } @Test - fun merge_keepsSideLoadedTrack_whenTheManifestDoesNotPublishIt() { + fun merge_hidesSideLoadedTrack_evenWhenItsLanguageIsMissingFromTheManifest() { val apiTracks = listOf( offTrack(), apiTrack(1, "rus", "rus", url = "https://api.test/subtitles/rus.srt"), @@ -462,8 +461,7 @@ internal class SubtitleTrackMergerTest { val result = merger.merge(apiTracks, playerTracks) - assertEquals(listOf("Off", "Русский", "Английский"), result.map { it.label }) - assertEquals("https://api.test/subtitles/eng.srt", result[2].url) + assertEquals(listOf("Off", "Русский"), result.map { it.label }) } @Test From a43d54affd23afb28d16f1b7774187d6acdccf8c Mon Sep 17 00:00:00 2001 From: Innokentii Enikeev Date: Sat, 29 Aug 2026 13:30:58 +0200 Subject: [PATCH 15/25] Remove dead subtitle track state and drive-by changes SubtitleTrackUIState.index lost its last production reader when the positional text-track lookup was deleted, so drop the field and the re-indexing pass it forced on every merge. Also fold the duplicated subtitle file-extension regex into one, fold the tracksRestored flag into resetTrackRestoreState, drop the unconsumed `selected` semantics from SettingsPanelItem, and mark the deliberate SingleSampleMediaSource deprecation instead of leaving a new build warning. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01GEBLAtGvP5AHqHemWgLXAp --- .../feature/player/component/PlayerPreview.kt | 10 +- .../player/component/SettingsPanelColumn.kt | 5 +- .../ui/feature/player/model/PlayerUIMapper.kt | 4 +- .../ui/feature/player/model/PlayerUIModels.kt | 1 - .../feature/player/vm/PlaybackController.kt | 14 +-- .../puber/ui/feature/player/vm/PlayerVM.kt | 4 +- .../ui/feature/player/vm/SubtitleLabeler.kt | 7 +- .../player/vm/SubtitleTrackIdentity.kt | 2 +- .../feature/player/vm/SubtitleTrackMerger.kt | 6 +- .../model/PlayerUIMapperSubtitleTest.kt | 4 +- .../vm/AudioTrackPreferenceResolverTest.kt | 1 - .../player/vm/PlayerVMSubtitleVariantTest.kt | 16 +-- .../feature/player/vm/PlayerVMTestFixture.kt | 6 +- .../feature/player/vm/SubtitleLabelerTest.kt | 1 - .../player/vm/SubtitleTrackMergerTest.kt | 113 +++++++----------- .../player/vm/SubtitleTrackSelectorTest.kt | 6 +- 16 files changed, 71 insertions(+), 129 deletions(-) diff --git a/app/src/main/java/com/kino/puber/ui/feature/player/component/PlayerPreview.kt b/app/src/main/java/com/kino/puber/ui/feature/player/component/PlayerPreview.kt index a04a62c9..6291405b 100644 --- a/app/src/main/java/com/kino/puber/ui/feature/player/component/PlayerPreview.kt +++ b/app/src/main/java/com/kino/puber/ui/feature/player/component/PlayerPreview.kt @@ -39,11 +39,11 @@ private val previewAudioTracks = listOf( ) private val previewSubtitleTracks = listOf( - SubtitleTrackUIState(0, "Выкл.", "", ""), - SubtitleTrackUIState(1, "Русские", "ru", "https://example.com/ru.vtt"), - SubtitleTrackUIState(2, "Русские для слабослышащих", "ru", "https://example.com/ru-sdh.vtt"), - SubtitleTrackUIState(3, "English", "en", "https://example.com/en.vtt"), - SubtitleTrackUIState(4, "Узбекские · Созданы нейросетью", "uz", "https://example.com/uz.vtt"), + SubtitleTrackUIState("Выкл.", "", ""), + SubtitleTrackUIState("Русские", "ru", "https://example.com/ru.vtt"), + SubtitleTrackUIState("Русские для слабослышащих", "ru", "https://example.com/ru-sdh.vtt"), + SubtitleTrackUIState("English", "en", "https://example.com/en.vtt"), + SubtitleTrackUIState("Узбекские · Созданы нейросетью", "uz", "https://example.com/uz.vtt"), ) private val previewSoundModes = listOf( diff --git a/app/src/main/java/com/kino/puber/ui/feature/player/component/SettingsPanelColumn.kt b/app/src/main/java/com/kino/puber/ui/feature/player/component/SettingsPanelColumn.kt index 279ba62f..90b1250d 100644 --- a/app/src/main/java/com/kino/puber/ui/feature/player/component/SettingsPanelColumn.kt +++ b/app/src/main/java/com/kino/puber/ui/feature/player/component/SettingsPanelColumn.kt @@ -19,8 +19,6 @@ import androidx.compose.ui.focus.FocusRequester import androidx.compose.ui.focus.focusRequester import androidx.compose.ui.graphics.Color import androidx.compose.ui.platform.testTag -import androidx.compose.ui.semantics.selected -import androidx.compose.ui.semantics.semantics import androidx.compose.ui.unit.dp import androidx.tv.material3.ClickableSurfaceDefaults import androidx.tv.material3.Icon @@ -92,8 +90,7 @@ private fun SettingsPanelItem( onClick = onClick, modifier = Modifier .then(testTag?.let { Modifier.testTag(it) } ?: Modifier) - .then(focusRequester?.let { Modifier.focusRequester(it) } ?: Modifier) - .semantics { this.selected = selected }, + .then(focusRequester?.let { Modifier.focusRequester(it) } ?: Modifier), colors = ClickableSurfaceDefaults.colors( containerColor = Color.Transparent, focusedContainerColor = colors.primary.copy(alpha = 0.2f), diff --git a/app/src/main/java/com/kino/puber/ui/feature/player/model/PlayerUIMapper.kt b/app/src/main/java/com/kino/puber/ui/feature/player/model/PlayerUIMapper.kt index f9b0766e..935e95fc 100644 --- a/app/src/main/java/com/kino/puber/ui/feature/player/model/PlayerUIMapper.kt +++ b/app/src/main/java/com/kino/puber/ui/feature/player/model/PlayerUIMapper.kt @@ -40,16 +40,14 @@ internal class PlayerUIMapper( fun mapSubtitleTracks(subtitles: List?): List { val result = mutableListOf( SubtitleTrackUIState( - index = 0, label = context.getString(R.string.player_subtitles_off), language = "", url = "", ) ) - subtitles?.forEachIndexed { index, sub -> + subtitles?.forEach { sub -> result.add( SubtitleTrackUIState( - index = index + 1, label = sub.lang, language = sub.lang, url = sub.url, diff --git a/app/src/main/java/com/kino/puber/ui/feature/player/model/PlayerUIModels.kt b/app/src/main/java/com/kino/puber/ui/feature/player/model/PlayerUIModels.kt index b5ed4356..28d9b4d8 100644 --- a/app/src/main/java/com/kino/puber/ui/feature/player/model/PlayerUIModels.kt +++ b/app/src/main/java/com/kino/puber/ui/feature/player/model/PlayerUIModels.kt @@ -11,7 +11,6 @@ internal data class AudioTrackUIState( @Immutable internal data class SubtitleTrackUIState( - val index: Int, val label: String, val language: String, val url: String, diff --git a/app/src/main/java/com/kino/puber/ui/feature/player/vm/PlaybackController.kt b/app/src/main/java/com/kino/puber/ui/feature/player/vm/PlaybackController.kt index 673b3b92..4322ba1d 100644 --- a/app/src/main/java/com/kino/puber/ui/feature/player/vm/PlaybackController.kt +++ b/app/src/main/java/com/kino/puber/ui/feature/player/vm/PlaybackController.kt @@ -26,7 +26,6 @@ import androidx.media3.exoplayer.source.BehindLiveWindowException import androidx.media3.exoplayer.source.DefaultMediaSourceFactory import androidx.media3.exoplayer.source.MediaSource import androidx.media3.exoplayer.source.MergingMediaSource -import androidx.media3.exoplayer.source.SingleSampleMediaSource import androidx.media3.exoplayer.trackselection.AdaptiveTrackSelection import androidx.media3.exoplayer.trackselection.DefaultTrackSelector import androidx.media3.exoplayer.upstream.DefaultBandwidthMeter @@ -447,6 +446,9 @@ internal class PlaybackController( // HlsMediaSource.Factory ignores MediaItem subtitle configurations, so an API subtitle // missing from the manifest would otherwise be unreachable on an HLS stream. + // DefaultMediaSourceFactory does this wrapping itself but cannot be told to enable + // chunkless HLS preparation, so the deprecated source is assembled by hand. + @Suppress("DEPRECATION") @OptIn(UnstableApi::class) private fun withSideLoadedSubtitles( source: MediaSource, @@ -456,7 +458,7 @@ internal class PlaybackController( val subtitleConfigs = mediaItem.localConfiguration?.subtitleConfigurations.orEmpty() if (subtitleConfigs.isEmpty()) return source val subtitleSources = subtitleConfigs.map { config -> - SingleSampleMediaSource.Factory(dsFactory) + androidx.media3.exoplayer.source.SingleSampleMediaSource.Factory(dsFactory) .setLoadErrorHandlingPolicy(HlsErrorPolicy()) .createMediaSource(config, C.TIME_UNSET) } @@ -631,18 +633,16 @@ internal class PlaybackController( textGroups: List, hlsSubtitles: List, ): List { - var subtitleIndex = 0 + var flatIndex = 0 val textTrackCount = textGroups.sumOf { it.length } return textGroups.flatMapIndexed { groupIndex, group -> (0 until group.length).map { trackIndex -> val format = group.getTrackFormat(trackIndex) - subtitleIndex += 1 buildSubtitleTrack( - index = subtitleIndex, format = format, hlsRendition = findHlsRendition( format = format, - renditionIndex = subtitleIndex - 1, + renditionIndex = flatIndex++, textTrackCount = textTrackCount, hlsSubtitles = hlsSubtitles, ), @@ -673,7 +673,6 @@ internal class PlaybackController( } private fun buildSubtitleTrack( - index: Int, format: Format, hlsRendition: HlsMultivariantPlaylist.Rendition?, groupIndex: Int, @@ -684,7 +683,6 @@ internal class PlaybackController( val language = format.language ?: identityFormat?.language.orEmpty() // SubtitleLabeler builds every visible label once the full track list is known. return SubtitleTrackUIState( - index = index, label = "", language = language, url = "", diff --git a/app/src/main/java/com/kino/puber/ui/feature/player/vm/PlayerVM.kt b/app/src/main/java/com/kino/puber/ui/feature/player/vm/PlayerVM.kt index 9ab82afc..45a3d707 100644 --- a/app/src/main/java/com/kino/puber/ui/feature/player/vm/PlayerVM.kt +++ b/app/src/main/java/com/kino/puber/ui/feature/player/vm/PlayerVM.kt @@ -354,6 +354,7 @@ internal class PlayerVM( } private fun resetTrackRestoreState() { + tracksRestoredForCurrentMedia = false audioRestoredForCurrentMedia = false subtitleTracksDiscovered = false subtitleRestoreAttempts = 0 @@ -703,7 +704,6 @@ internal class PlayerVM( val position = playbackController.currentPosition updateContent { copy(selectedBufferPresetIndex = index) } - tracksRestoredForCurrentMedia = false resetTrackRestoreState() initializePlayer(savedPosition = position) } @@ -715,7 +715,6 @@ internal class PlayerVM( val position = playbackController.currentPosition updateContent { copy(fastDnsEnabled = newValue) } - tracksRestoredForCurrentMedia = false resetTrackRestoreState() initializePlayer(savedPosition = position) } @@ -747,7 +746,6 @@ internal class PlayerVM( creditsSegment = null dismissedSegmentType = null countdownDismissed = false - tracksRestoredForCurrentMedia = false resetTrackRestoreState() updateViewState(PlayerViewState.Loading) diff --git a/app/src/main/java/com/kino/puber/ui/feature/player/vm/SubtitleLabeler.kt b/app/src/main/java/com/kino/puber/ui/feature/player/vm/SubtitleLabeler.kt index ba7704d0..4a4ac080 100644 --- a/app/src/main/java/com/kino/puber/ui/feature/player/vm/SubtitleLabeler.kt +++ b/app/src/main/java/com/kino/puber/ui/feature/player/vm/SubtitleLabeler.kt @@ -73,7 +73,7 @@ internal class SubtitleLabeler( */ internal fun SubtitleTrackUIState.readableDescriptiveLabel(): String? { val candidate = descriptiveLabel?.trim()?.takeIf { it.isNotEmpty() } ?: return null - val isFileName = SUBTITLE_FILE_NAME.containsMatchIn(candidate) || + val isFileName = SUBTITLE_FILE_EXTENSION.containsMatchIn(candidate) || candidate == sourceFile || candidate == url.stableSubtitleKey() val isLanguageCode = candidate.count(Char::isLetter) <= SHORTEST_DESCRIPTIVE_LABEL @@ -81,8 +81,3 @@ internal fun SubtitleTrackUIState.readableDescriptiveLabel(): String? { } private const val SHORTEST_DESCRIPTIVE_LABEL = 3 - -private val SUBTITLE_FILE_NAME = Regex( - pattern = """\.(srt|vtt|webvtt|ass|ssa|ttml|xml)$""", - option = RegexOption.IGNORE_CASE, -) diff --git a/app/src/main/java/com/kino/puber/ui/feature/player/vm/SubtitleTrackIdentity.kt b/app/src/main/java/com/kino/puber/ui/feature/player/vm/SubtitleTrackIdentity.kt index c3b92017..f879e274 100644 --- a/app/src/main/java/com/kino/puber/ui/feature/player/vm/SubtitleTrackIdentity.kt +++ b/app/src/main/java/com/kino/puber/ui/feature/player/vm/SubtitleTrackIdentity.kt @@ -33,7 +33,7 @@ private fun String.subtitleIdentityPathOrNull(): String? { } } -private val SUBTITLE_FILE_EXTENSION = Regex( +internal val SUBTITLE_FILE_EXTENSION = Regex( pattern = """\.(srt|vtt|webvtt|ass|ssa|ttml|xml)$""", option = RegexOption.IGNORE_CASE, ) diff --git a/app/src/main/java/com/kino/puber/ui/feature/player/vm/SubtitleTrackMerger.kt b/app/src/main/java/com/kino/puber/ui/feature/player/vm/SubtitleTrackMerger.kt index aaa291cc..bfedb8b4 100644 --- a/app/src/main/java/com/kino/puber/ui/feature/player/vm/SubtitleTrackMerger.kt +++ b/app/src/main/java/com/kino/puber/ui/feature/player/vm/SubtitleTrackMerger.kt @@ -12,7 +12,6 @@ internal class SubtitleTrackMerger( playerTracks: List, ): List { val offTrack = apiTracks.firstOrNull { it.isOff } ?: SubtitleTrackUIState( - index = 0, label = "", language = "", url = "", @@ -20,13 +19,12 @@ internal class SubtitleTrackMerger( val apiSubtitles = apiTracks.filterNot { it.isOff } val playerSubtitles = playerTracks.filterNot { it.isOff } if (playerSubtitles.isEmpty()) { - return listOf(offTrack.copy(index = 0)) + return listOf(offTrack) } val enrichedPlayerTracks = enrichPlayerTracks(playerSubtitles, apiSubtitles) val orderedPlayerTracks = orderByLanguage(enrichedPlayerTracks) - return (listOf(offTrack) + labeler.apply(orderedPlayerTracks)) - .mapIndexed { index, track -> track.copy(index = index) } + return listOf(offTrack) + labeler.apply(orderedPlayerTracks) } /** diff --git a/app/src/test/kotlin/com/kino/puber/ui/feature/player/model/PlayerUIMapperSubtitleTest.kt b/app/src/test/kotlin/com/kino/puber/ui/feature/player/model/PlayerUIMapperSubtitleTest.kt index 5370d8a8..2840caca 100644 --- a/app/src/test/kotlin/com/kino/puber/ui/feature/player/model/PlayerUIMapperSubtitleTest.kt +++ b/app/src/test/kotlin/com/kino/puber/ui/feature/player/model/PlayerUIMapperSubtitleTest.kt @@ -19,7 +19,6 @@ internal class PlayerUIMapperSubtitleTest { fun mapSubtitleTracks_returnsOnlyOffTrack_whenInputIsEmpty() { val result = mapper.mapSubtitleTracks(emptyList()) - assertEquals(listOf(0), result.map { it.index }) assertEquals(listOf(""), result.map { it.language }) assertEquals(listOf(""), result.map { it.url }) } @@ -42,7 +41,6 @@ internal class PlayerUIMapperSubtitleTest { ) ) - assertEquals(listOf(0, 1, 2), result.map { it.index }) assertEquals(listOf("", "rus", "eng"), result.map { it.language }) assertEquals(listOf("", embeddedUrl, externalUrl), result.map { it.url }) assertEquals(listOf(null, "/a/71/russian.vtt", null), result.map { it.sourceFile }) @@ -50,7 +48,7 @@ internal class PlayerUIMapperSubtitleTest { } @Test - fun mapSubtitleTracks_keepsSameLanguageVariantsDistinct() { + fun mapSubtitleTracks_leavesLabellingToTheMerger_forSameLanguageVariants() { val result = mapper.mapSubtitleTracks( listOf( SubtitleLink( diff --git a/app/src/test/kotlin/com/kino/puber/ui/feature/player/vm/AudioTrackPreferenceResolverTest.kt b/app/src/test/kotlin/com/kino/puber/ui/feature/player/vm/AudioTrackPreferenceResolverTest.kt index 84008a80..703e4e28 100644 --- a/app/src/test/kotlin/com/kino/puber/ui/feature/player/vm/AudioTrackPreferenceResolverTest.kt +++ b/app/src/test/kotlin/com/kino/puber/ui/feature/player/vm/AudioTrackPreferenceResolverTest.kt @@ -276,7 +276,6 @@ internal class AudioTrackPreferenceResolverTest { groupIndex: Int? = playerTrackId?.let { index - 1 }, trackIndex: Int? = groupIndex?.let { 0 }, ) = SubtitleTrackUIState( - index = index, label = "Track $index", language = language, url = url, diff --git a/app/src/test/kotlin/com/kino/puber/ui/feature/player/vm/PlayerVMSubtitleVariantTest.kt b/app/src/test/kotlin/com/kino/puber/ui/feature/player/vm/PlayerVMSubtitleVariantTest.kt index e5337a2b..2d49b14c 100644 --- a/app/src/test/kotlin/com/kino/puber/ui/feature/player/vm/PlayerVMSubtitleVariantTest.kt +++ b/app/src/test/kotlin/com/kino/puber/ui/feature/player/vm/PlayerVMSubtitleVariantTest.kt @@ -25,7 +25,6 @@ internal class PlayerVMSubtitleVariantTest : PlayerVMTestFixture() { val vm = startedVM() val audioTracks = listOf(AudioTrackUIState(0, "English", "eng")) val manifestTrack = testSubtitleTracks.first().copy( - index = 1, label = "Ukrainian HLS", language = "uk", playerTrackId = "hls-ukrainian", @@ -59,7 +58,6 @@ internal class PlayerVMSubtitleVariantTest : PlayerVMTestFixture() { val vm = startedVM() val audioTracks = listOf(AudioTrackUIState(0, "English", "eng")) val manifestTrack = testSubtitleTracks.first().copy( - index = 1, label = "Ukrainian HLS", language = "uk", playerTrackId = "hls-ukrainian", @@ -104,7 +102,6 @@ internal class PlayerVMSubtitleVariantTest : PlayerVMTestFixture() { val audioTracks = listOf(AudioTrackUIState(0, "English", "eng")) val manifestTracks = listOf( testSubtitleTracks.first().copy( - index = 1, label = "Russian full", language = "ru", isForced = false, @@ -113,7 +110,6 @@ internal class PlayerVMSubtitleVariantTest : PlayerVMTestFixture() { playerTrackIndex = 0, ), testSubtitleTracks.first().copy( - index = 2, label = "Russian forced", language = "ru", isForced = true, @@ -139,9 +135,9 @@ internal class PlayerVMSubtitleVariantTest : PlayerVMTestFixture() { val vm = startedVM() val audioTracks = listOf(AudioTrackUIState(0, "English", "eng")) val manifestTracks = listOf( - manifestTrack(1, "English 1", "manifest-eng-1", groupIndex = 5), - manifestTrack(2, "English 2", "manifest-eng-2", groupIndex = 6), - manifestTrack(3, "English 3", "manifest-eng-3", groupIndex = 7), + manifestTrack("English 1", "manifest-eng-1", groupIndex = 5), + manifestTrack("English 2", "manifest-eng-2", groupIndex = 6), + manifestTrack("English 3", "manifest-eng-3", groupIndex = 7), ) callbackSlot.captured.onTracksUpdated(audioTracks, 0, manifestTracks) vm.onAction(PlayerAction.SelectSubtitle(2)) @@ -161,8 +157,8 @@ internal class PlayerVMSubtitleVariantTest : PlayerVMTestFixture() { val vm = startedVM() val audioTracks = listOf(AudioTrackUIState(0, "English", "eng")) val manifestTracks = listOf( - manifestTrack(1, "English 1", "manifest-eng-1", groupIndex = 5), - manifestTrack(2, "English 2", "manifest-eng-2", groupIndex = 6), + manifestTrack("English 1", "manifest-eng-1", groupIndex = 5), + manifestTrack("English 2", "manifest-eng-2", groupIndex = 6), ) callbackSlot.captured.onTracksUpdated(audioTracks, 0, manifestTracks) vm.onAction(PlayerAction.SelectSubtitle(2)) @@ -263,12 +259,10 @@ internal class PlayerVMSubtitleVariantTest : PlayerVMTestFixture() { } private fun manifestTrack( - index: Int, label: String, id: String, groupIndex: Int, ) = SubtitleTrackUIState( - index = index, label = label, language = "en", url = "", diff --git a/app/src/test/kotlin/com/kino/puber/ui/feature/player/vm/PlayerVMTestFixture.kt b/app/src/test/kotlin/com/kino/puber/ui/feature/player/vm/PlayerVMTestFixture.kt index c23518d8..657c20da 100644 --- a/app/src/test/kotlin/com/kino/puber/ui/feature/player/vm/PlayerVMTestFixture.kt +++ b/app/src/test/kotlin/com/kino/puber/ui/feature/player/vm/PlayerVMTestFixture.kt @@ -190,10 +190,9 @@ internal abstract class PlayerVMTestFixture { ) protected val testSubtitleTracks = listOf( - SubtitleTrackUIState(index = 0, label = "Off", language = "", url = ""), - SubtitleTrackUIState(index = 1, label = "Russian", language = "rus", url = "https://test/subtitles/rus.vtt"), + SubtitleTrackUIState(label = "Off", language = "", url = ""), + SubtitleTrackUIState(label = "Russian", language = "rus", url = "https://test/subtitles/rus.vtt"), SubtitleTrackUIState( - index = 2, label = "Russian forced", language = "rus", url = "https://test/subtitles/rus-forced.vtt", @@ -202,7 +201,6 @@ internal abstract class PlayerVMTestFixture { protected val testDiscoveredSubtitleTracks = testSubtitleTracks.drop(1).mapIndexed { index, track -> track.copy( - index = index + 1, url = "", playerTrackId = track.url, playerTrackUri = track.url, diff --git a/app/src/test/kotlin/com/kino/puber/ui/feature/player/vm/SubtitleLabelerTest.kt b/app/src/test/kotlin/com/kino/puber/ui/feature/player/vm/SubtitleLabelerTest.kt index 17eb94ab..cf20acb5 100644 --- a/app/src/test/kotlin/com/kino/puber/ui/feature/player/vm/SubtitleLabelerTest.kt +++ b/app/src/test/kotlin/com/kino/puber/ui/feature/player/vm/SubtitleLabelerTest.kt @@ -110,7 +110,6 @@ internal class SubtitleLabelerTest { descriptive: String? = null, forced: Boolean = false, ) = SubtitleTrackUIState( - index = 0, label = "", language = language, url = "", diff --git a/app/src/test/kotlin/com/kino/puber/ui/feature/player/vm/SubtitleTrackMergerTest.kt b/app/src/test/kotlin/com/kino/puber/ui/feature/player/vm/SubtitleTrackMergerTest.kt index 284f4c9e..9c45a9f7 100644 --- a/app/src/test/kotlin/com/kino/puber/ui/feature/player/vm/SubtitleTrackMergerTest.kt +++ b/app/src/test/kotlin/com/kino/puber/ui/feature/player/vm/SubtitleTrackMergerTest.kt @@ -22,13 +22,11 @@ internal class SubtitleTrackMergerTest { val apiTracks = listOf( offTrack(), apiTrack( - index = 1, label = "Russian embedded", language = "rus", url = "https://api.test/subtitles/embedded-rus.srt", ), apiTrack( - index = 2, label = "Russian external", language = "rus", url = "https://api.test/subtitles/external-rus.srt", @@ -36,14 +34,12 @@ internal class SubtitleTrackMergerTest { ) val playerTracks = listOf( playerTrack( - index = 1, label = "external-rus.srt", language = "rus", id = "external-rus.srt", groupIndex = 0, ), playerTrack( - index = 2, label = "Русские полные", language = "ru", id = "hls-russian-full", @@ -67,12 +63,12 @@ internal class SubtitleTrackMergerTest { fun merge_doesNotPairSameLanguageVariants_withoutExactIdentity() { val apiTracks = listOf( offTrack(), - apiTrack(1, "Russian full", "rus", forced = false), - apiTrack(2, "Russian forced", "rus", forced = true), + apiTrack("Russian full", "rus", forced = false), + apiTrack("Russian forced", "rus", forced = true), ) val playerTracks = listOf( - playerTrack(1, "Russian forced HLS", "ru", "forced", 0, forced = true), - playerTrack(2, "Russian full HLS", "ru", "full", 1, forced = false), + playerTrack("Russian forced HLS", "ru", "forced", 0, forced = true), + playerTrack("Russian full HLS", "ru", "full", 1, forced = false), ) val result = merger.merge(apiTracks, playerTracks) @@ -87,7 +83,6 @@ internal class SubtitleTrackMergerTest { @Test fun merge_appendsManifestOnlyTracks_withoutChangingTheirLanguage() { val playerTrack = playerTrack( - index = 7, label = "Українські", language = "uk", id = "hls-ukrainian", @@ -97,7 +92,6 @@ internal class SubtitleTrackMergerTest { val result = merger.merge(listOf(offTrack()), listOf(playerTrack)) assertEquals(listOf("", "uk"), result.map { it.language }) - assertEquals(listOf(0, 1), result.map { it.index }) assertEquals("hls-ukrainian", result[1].playerTrackId) assertEquals(2, result[1].playerGroupIndex) } @@ -107,14 +101,13 @@ internal class SubtitleTrackMergerTest { val apiTracks = listOf( offTrack(), apiTrack( - index = 1, label = "Russian external", language = "rus", url = "https://api.test/subtitles/external.srt", ), ) val playerTracks = listOf( - playerTrack(1, "Russian HLS", "ru", "hls-russian", 0), + playerTrack("Russian HLS", "ru", "hls-russian", 0), ) val result = merger.merge(apiTracks, playerTracks) @@ -127,13 +120,11 @@ internal class SubtitleTrackMergerTest { @Test fun merge_doesNotTreatDisplayLabelAsTrackIdentity() { val apiTrack = apiTrack( - index = 1, label = "Spanish API", language = "spa", url = "https://api.test/subtitles/spanish.srt", ) val playerTrack = playerTrack( - index = 1, label = "spanish.srt", language = "rus", id = "hls-russian", @@ -151,16 +142,16 @@ internal class SubtitleTrackMergerTest { fun merge_groupsVariantsByLanguage_keepingManifestOrderWithinEachLanguage() { val apiTracks = listOf( offTrack(), - apiTrack(1, "rus #1", "rus"), - apiTrack(2, "rus #2", "rus"), - apiTrack(3, "rus #3", "rus"), - apiTrack(4, "spa", "spa"), + apiTrack("rus #1", "rus"), + apiTrack("rus #2", "rus"), + apiTrack("rus #3", "rus"), + apiTrack("spa", "spa"), ) val playerTracks = listOf( - playerTrack(1, "Russian 1", "ru", "rus-1", 0), - playerTrack(2, "Spanish", "es", "spa", 1), - playerTrack(3, "Russian 2", "ru", "rus-2", 2), - playerTrack(4, "Russian 3", "ru", "rus-3", 3), + playerTrack("Russian 1", "ru", "rus-1", 0), + playerTrack("Spanish", "es", "spa", 1), + playerTrack("Russian 2", "ru", "rus-2", 2), + playerTrack("Russian 3", "ru", "rus-3", 3), ) val result = merger.merge(apiTracks, playerTracks) @@ -184,13 +175,11 @@ internal class SubtitleTrackMergerTest { val apiTracks = listOf( offTrack(), apiTrack( - index = 1, label = "rus #1", language = "rus", sourceFile = "/a/71/first.srt", ), apiTrack( - index = 2, label = "rus #2", language = "rus", sourceFile = "/b/82/second.srt", @@ -198,7 +187,6 @@ internal class SubtitleTrackMergerTest { ) val playerTracks = listOf( playerTrack( - index = 1, label = "Russian 2", language = "ru", id = "subs:Russian #02", @@ -206,7 +194,6 @@ internal class SubtitleTrackMergerTest { uri = "https://cdn.test/pd/subtitle/token/b/82/second.srt", ), playerTrack( - index = 2, label = "Russian 1", language = "ru", id = "subs:Russian #01", @@ -231,13 +218,11 @@ internal class SubtitleTrackMergerTest { @Test fun merge_matchesSideLoadedTrackByStableUrl_whenHostAndTokenChange() { val apiTrack = apiTrack( - index = 1, label = "Russian external", language = "rus", url = "https://old-cdn.test/subtitles/russian.vtt?token=expired", ) val playerTrack = playerTrack( - index = 1, label = "russian.vtt", language = "ru", id = "https://new-cdn.test/subtitles/russian.vtt?token=fresh", @@ -255,12 +240,10 @@ internal class SubtitleTrackMergerTest { @Test fun merge_usesOnlyManifestTracks_whenApiIdentityDoesNotMatch() { val embeddedRussian = apiTrack( - index = 1, label = "Russian embedded", language = "rus", ) val manifestEnglish = playerTrack( - index = 1, label = "English HLS", language = "en", id = "hls-english", @@ -281,21 +264,20 @@ internal class SubtitleTrackMergerTest { fun merge_exposesOnlyOff_untilPlayerTracksAreDiscovered() { val apiTracks = listOf( offTrack(), - apiTrack(1, "Russian full", "rus"), - apiTrack(2, "Russian forced", "rus"), + apiTrack("Russian full", "rus"), + apiTrack("Russian forced", "rus"), ) val result = merger.merge(apiTracks, emptyList()) assertEquals(listOf("Off"), result.map { it.label }) - assertEquals(listOf(0), result.map { it.index }) } @Test fun merge_usesDescriptiveLabels_forSameLanguageVariantsThatWouldCollide() { val playerTracks = listOf( - playerTrack(1, "rus", "rus", "full", 0, descriptiveLabel = "Русские полные"), - playerTrack(2, "rus", "rus", "sdh", 1, descriptiveLabel = "Русские SDH"), + playerTrack("rus", "rus", "full", 0, descriptiveLabel = "Русские полные"), + playerTrack("rus", "rus", "sdh", 1, descriptiveLabel = "Русские SDH"), ) val result = merger.merge(listOf(offTrack()), playerTracks) @@ -306,9 +288,9 @@ internal class SubtitleTrackMergerTest { @Test fun merge_numbersSameLanguageVariants_whenDescriptiveLabelsCannotSeparateThem() { val playerTracks = listOf( - playerTrack(1, "rus", "rus", "a", 0, descriptiveLabel = "RUS"), - playerTrack(2, "rus", "rus", "b", 1, descriptiveLabel = "RUS"), - playerTrack(3, "rus", "rus", "c", 2, descriptiveLabel = null), + playerTrack("rus", "rus", "a", 0, descriptiveLabel = "RUS"), + playerTrack("rus", "rus", "b", 1, descriptiveLabel = "RUS"), + playerTrack("rus", "rus", "c", 2, descriptiveLabel = null), ) val result = merger.merge(listOf(offTrack()), playerTracks) @@ -322,8 +304,8 @@ internal class SubtitleTrackMergerTest { @Test fun merge_marksPartialVariant_andOrdersItLastWithinItsLanguage() { val playerTracks = listOf( - playerTrack(1, "rus", "rus", "full", 0, forced = false, descriptiveLabel = "RUS"), - playerTrack(2, "rus", "rus", "forced", 1, forced = true, descriptiveLabel = "RUS"), + playerTrack("rus", "rus", "full", 0, forced = false, descriptiveLabel = "RUS"), + playerTrack("rus", "rus", "forced", 1, forced = true, descriptiveLabel = "RUS"), ) val result = merger.merge(listOf(offTrack()), playerTracks) @@ -335,8 +317,8 @@ internal class SubtitleTrackMergerTest { @Test fun merge_leavesDistinctLabelsAlone() { val playerTracks = listOf( - playerTrack(1, "rus", "rus", "a", 0, descriptiveLabel = "RUS"), - playerTrack(2, "eng", "eng", "b", 1, descriptiveLabel = "ENG"), + playerTrack("rus", "rus", "a", 0, descriptiveLabel = "RUS"), + playerTrack("eng", "eng", "b", 1, descriptiveLabel = "ENG"), ) val result = merger.merge(listOf(offTrack()), playerTracks) @@ -353,15 +335,15 @@ internal class SubtitleTrackMergerTest { fun merge_hidesSideLoadedCopies_whenManifestCoversTheirLanguages() { val apiTracks = listOf( offTrack(), - apiTrack(1, "eng", "eng", url = "https://api.test/pd/xyz", sourceFile = "/9/24/2871466.srt"), - apiTrack(2, "rus", "rus", url = "https://api.test/pd/abc", sourceFile = "/1/92/2871463.srt"), + apiTrack("eng", "eng", url = "https://api.test/pd/xyz", sourceFile = "/9/24/2871466.srt"), + apiTrack("rus", "rus", url = "https://api.test/pd/abc", sourceFile = "/1/92/2871463.srt"), ) val playerTracks = listOf( - playerTrack(1, "", "ru", "0:", 0, uri = "https://cdn.test/hls/rus01.m3u8", descriptiveLabel = "RUS #01"), - playerTrack(2, "", "ru", "0:", 1, uri = "https://cdn.test/hls/rus02.m3u8", descriptiveLabel = "RUS #02"), - playerTrack(3, "", "en", "0:", 2, uri = "https://cdn.test/hls/eng03.m3u8", descriptiveLabel = "ENG #03"), - playerTrack(4, "", "en", "1:9/24/2871466.srt", 3), - playerTrack(5, "", "ru", "2:1/92/2871463.srt", 4), + playerTrack("", "ru", "0:", 0, uri = "https://cdn.test/hls/rus01.m3u8", descriptiveLabel = "RUS #01"), + playerTrack("", "ru", "0:", 1, uri = "https://cdn.test/hls/rus02.m3u8", descriptiveLabel = "RUS #02"), + playerTrack("", "en", "0:", 2, uri = "https://cdn.test/hls/eng03.m3u8", descriptiveLabel = "ENG #03"), + playerTrack("", "en", "1:9/24/2871466.srt", 3), + playerTrack("", "ru", "2:1/92/2871463.srt", 4), ) val result = merger.merge(apiTracks, playerTracks) @@ -381,12 +363,12 @@ internal class SubtitleTrackMergerTest { fun merge_carriesSideLoadedTracks_whenTheManifestPublishesNoSubtitles() { val apiTracks = listOf( offTrack(), - apiTrack(1, "rus", "rus", url = "https://api.test/pd/a", sourceFile = "/1/11/1.srt"), - apiTrack(2, "eng", "eng", url = "https://api.test/pd/b", sourceFile = "/2/22/2.srt"), + apiTrack("rus", "rus", url = "https://api.test/pd/a", sourceFile = "/1/11/1.srt"), + apiTrack("eng", "eng", url = "https://api.test/pd/b", sourceFile = "/2/22/2.srt"), ) val playerTracks = listOf( - playerTrack(1, "", "ru", "1:1/11/1.srt", 0), - playerTrack(2, "", "en", "2:2/22/2.srt", 1), + playerTrack("", "ru", "1:1/11/1.srt", 0), + playerTrack("", "en", "2:2/22/2.srt", 1), ) val result = merger.merge(apiTracks, playerTracks) @@ -402,10 +384,10 @@ internal class SubtitleTrackMergerTest { fun merge_matchesSideLoadedTrack_ignoringTheMergedChildIndexPrefix() { val apiTracks = listOf( offTrack(), - apiTrack(1, "ukr", "ukr", url = "https://api.test/pd/abc", sourceFile = "/1/92/2871463.srt"), + apiTrack("ukr", "ukr", url = "https://api.test/pd/abc", sourceFile = "/1/92/2871463.srt"), ) val playerTracks = listOf( - playerTrack(1, "", "uk", "2:1/92/2871463.srt", 0), + playerTrack("", "uk", "2:1/92/2871463.srt", 0), ) val result = merger.merge(apiTracks, playerTracks) @@ -418,11 +400,10 @@ internal class SubtitleTrackMergerTest { fun merge_hidesSideLoadedCopy_whenManifestCoversItsLanguage() { val apiTracks = listOf( offTrack(), - apiTrack(1, "rus", "rus", url = "https://api.test/subtitles/rus.srt"), + apiTrack("rus", "rus", url = "https://api.test/subtitles/rus.srt"), ) val playerTracks = listOf( playerTrack( - index = 1, label = "rus", language = "rus", id = "hls-rus", @@ -430,7 +411,7 @@ internal class SubtitleTrackMergerTest { uri = "https://cdn.test/subtitles/rus.srt", ), // The side-loaded copy Media3 built from the same API url. - playerTrack(2, "rus", "rus", "rus.srt", 1), + playerTrack("rus", "rus", "rus.srt", 1), ) val result = merger.merge(apiTracks, playerTracks) @@ -444,19 +425,18 @@ internal class SubtitleTrackMergerTest { fun merge_hidesSideLoadedTrack_evenWhenItsLanguageIsMissingFromTheManifest() { val apiTracks = listOf( offTrack(), - apiTrack(1, "rus", "rus", url = "https://api.test/subtitles/rus.srt"), - apiTrack(2, "eng", "eng", url = "https://api.test/subtitles/eng.srt"), + apiTrack("rus", "rus", url = "https://api.test/subtitles/rus.srt"), + apiTrack("eng", "eng", url = "https://api.test/subtitles/eng.srt"), ) val playerTracks = listOf( playerTrack( - index = 1, label = "rus", language = "rus", id = "hls-rus", groupIndex = 0, uri = "https://cdn.test/subtitles/rus.srt", ), - playerTrack(2, "eng", "eng", "eng.srt", 1), + playerTrack("eng", "eng", "eng.srt", 1), ) val result = merger.merge(apiTracks, playerTracks) @@ -468,11 +448,10 @@ internal class SubtitleTrackMergerTest { fun merge_keepsBothManifestRenditions_whenTheyResolveToOneApiEntry() { val apiTracks = listOf( offTrack(), - apiTrack(1, "rus", "rus", url = "https://api.test/subtitles/rus.srt"), + apiTrack("rus", "rus", url = "https://api.test/subtitles/rus.srt"), ) val playerTracks = listOf( playerTrack( - index = 1, label = "rus", language = "rus", id = "a", @@ -481,7 +460,6 @@ internal class SubtitleTrackMergerTest { descriptiveLabel = "RUS A", ), playerTrack( - index = 2, label = "rus", language = "rus", id = "b", @@ -497,21 +475,18 @@ internal class SubtitleTrackMergerTest { } private fun offTrack() = SubtitleTrackUIState( - index = 0, label = "Off", language = "", url = "", ) private fun apiTrack( - index: Int, label: String, language: String, url: String = "", forced: Boolean? = null, sourceFile: String? = null, ) = SubtitleTrackUIState( - index = index, label = label, language = language, url = url, @@ -520,7 +495,6 @@ internal class SubtitleTrackMergerTest { ) private fun playerTrack( - index: Int, label: String, language: String, id: String, @@ -529,7 +503,6 @@ internal class SubtitleTrackMergerTest { uri: String? = null, descriptiveLabel: String? = null, ) = SubtitleTrackUIState( - index = index, label = label, language = language, url = "", diff --git a/app/src/test/kotlin/com/kino/puber/ui/feature/player/vm/SubtitleTrackSelectorTest.kt b/app/src/test/kotlin/com/kino/puber/ui/feature/player/vm/SubtitleTrackSelectorTest.kt index 4d8041ca..4241b6df 100644 --- a/app/src/test/kotlin/com/kino/puber/ui/feature/player/vm/SubtitleTrackSelectorTest.kt +++ b/app/src/test/kotlin/com/kino/puber/ui/feature/player/vm/SubtitleTrackSelectorTest.kt @@ -54,7 +54,6 @@ internal class SubtitleTrackSelectorTest { @Test fun select_resolvesSideLoadedRow_byStableKey() { val sideLoadedRow = SubtitleTrackUIState( - index = 1, label = "eng", language = "eng", url = "https://api.test/subtitles/29726.srt", @@ -119,7 +118,7 @@ internal class SubtitleTrackSelectorTest { @Test fun select_returnsNull_ratherThanGuessing_whenNothingIdentifiesTheTrack() { - val row = SubtitleTrackUIState(index = 1, label = "rus", language = "", url = "") + val row = SubtitleTrackUIState(label = "rus", language = "", url = "") val candidates = listOf( PlayerTextTrack("", 0, 0, language = "rus"), PlayerTextTrack("", 1, 0, language = "rus"), @@ -130,7 +129,7 @@ internal class SubtitleTrackSelectorTest { @Test fun select_matchesByLanguage_asLastResort() { - val row = SubtitleTrackUIState(index = 1, label = "ukr", language = "ukr", url = "") + val row = SubtitleTrackUIState(label = "ukr", language = "ukr", url = "") val candidates = listOf( PlayerTextTrack("", 0, 0, language = "rus"), PlayerTextTrack("", 1, 0, language = "uk"), @@ -151,7 +150,6 @@ internal class SubtitleTrackSelectorTest { url: String, groupIndex: Int = 0, ) = SubtitleTrackUIState( - index = 1, label = "rus", language = "rus", url = url, From 0495a3a2feb6bb76b47a5edcfe2b2d563492b448 Mon Sep 17 00:00:00 2001 From: Innokentii Enikeev Date: Sat, 29 Aug 2026 13:37:21 +0200 Subject: [PATCH 16/25] Cut redundant subtitle tests and the forced-URL guess Twelve tests either restated another test's assertion or asserted that a mapper does nothing. Labelling cases move to SubtitleLabelerTest, which owns them; the merger keeps only the cases where merging itself decides the outcome. select_resolvesSideLoadedRow_byStableKey never reached the stable-key rule (its group id matched first) - reshaped so that rule has coverage for the first time. SubtitleLink.forcedState guessed "forced" from a token in the subtitle URL. It was written when the model had no forced flag; the flag was added later and the comment rewritten to claim older responses omit it, which nothing in the repo supports. Drop the guess and read the API field directly. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01GEBLAtGvP5AHqHemWgLXAp --- .../com/kino/puber/data/api/models/Models.kt | 13 -- .../ui/feature/player/model/PlayerUIMapper.kt | 2 +- .../puber/data/api/models/SubtitleLinkTest.kt | 67 +--------- .../model/PlayerUIMapperSubtitleTest.kt | 30 ----- .../vm/AudioTrackPreferenceResolverTest.kt | 18 --- .../player/vm/PlayerVMSubtitleVariantTest.kt | 18 --- .../feature/player/vm/SubtitleLabelerTest.kt | 14 +- .../player/vm/SubtitleTrackMergerTest.kt | 126 +----------------- .../player/vm/SubtitleTrackSelectorTest.kt | 39 ++---- 9 files changed, 21 insertions(+), 306 deletions(-) diff --git a/app/src/main/java/com/kino/puber/data/api/models/Models.kt b/app/src/main/java/com/kino/puber/data/api/models/Models.kt index edf43b34..973a42f3 100644 --- a/app/src/main/java/com/kino/puber/data/api/models/Models.kt +++ b/app/src/main/java/com/kino/puber/data/api/models/Models.kt @@ -379,21 +379,8 @@ data class SubtitleLink( ) { val shouldSideLoad: Boolean get() = embed != true - - // Older API responses may omit the dedicated forced flag. - val forcedState: Boolean? - get() = forced ?: true.takeIf { - FORCED_SUBTITLE_TOKEN.containsMatchIn( - url.substringBefore('?').substringBefore('#'), - ) - } } -private val FORCED_SUBTITLE_TOKEN = Regex( - pattern = """(^|[/_.-])forced([/_.-]|$)""", - option = RegexOption.IGNORE_CASE, -) - @Serializable data class TVChannel( val id: Int, val title: String, val stream: String? = null, val epg: List? = null, diff --git a/app/src/main/java/com/kino/puber/ui/feature/player/model/PlayerUIMapper.kt b/app/src/main/java/com/kino/puber/ui/feature/player/model/PlayerUIMapper.kt index 935e95fc..2855e274 100644 --- a/app/src/main/java/com/kino/puber/ui/feature/player/model/PlayerUIMapper.kt +++ b/app/src/main/java/com/kino/puber/ui/feature/player/model/PlayerUIMapper.kt @@ -51,7 +51,7 @@ internal class PlayerUIMapper( label = sub.lang, language = sub.lang, url = sub.url, - isForced = sub.forcedState, + isForced = sub.forced, sourceFile = sub.file, ) ) diff --git a/app/src/test/kotlin/com/kino/puber/data/api/models/SubtitleLinkTest.kt b/app/src/test/kotlin/com/kino/puber/data/api/models/SubtitleLinkTest.kt index d815b8f9..fc08065f 100644 --- a/app/src/test/kotlin/com/kino/puber/data/api/models/SubtitleLinkTest.kt +++ b/app/src/test/kotlin/com/kino/puber/data/api/models/SubtitleLinkTest.kt @@ -1,74 +1,17 @@ package com.kino.puber.data.api.models -import org.junit.jupiter.api.Assertions.assertEquals import org.junit.jupiter.api.Assertions.assertFalse -import org.junit.jupiter.api.Assertions.assertNull import org.junit.jupiter.api.Assertions.assertTrue import org.junit.jupiter.api.Test internal class SubtitleLinkTest { @Test - fun shouldSideLoad_includesExternalSubtitle() { - val externalSubtitle = SubtitleLink( - lang = "eng", - url = "https://cdn.test/subtitle.srt", - ) + fun shouldSideLoad_coversEverythingButEmbeddedSubtitles() { + val url = "https://cdn.test/subtitle.srt" - assertTrue(externalSubtitle.shouldSideLoad) - } - - @Test - fun shouldSideLoad_excludesEmbeddedSubtitle() { - val subtitle = SubtitleLink( - lang = "eng", - url = "https://cdn.test/subtitle.srt", - embed = true, - ) - - assertFalse(subtitle.shouldSideLoad) - } - - @Test - fun forcedState_detectsForcedMarkerInPath_ignoringCaseAndQuery() { - val subtitle = SubtitleLink( - lang = "rus", - url = "https://cdn.test/subtitles/RUS-FORCED.vtt?token=forced-value", - ) - - assertEquals(true, subtitle.forcedState) - } - - @Test - fun forcedState_doesNotUseQueryOrPartialWordAsMarker() { - val regularSubtitle = SubtitleLink( - lang = "rus", - url = "https://cdn.test/subtitles/russian.vtt?mode=forced", - ) - val unforcedSubtitle = SubtitleLink( - lang = "rus", - url = "https://cdn.test/subtitles/russian-unforced.vtt", - ) - - assertNull(regularSubtitle.forcedState) - assertNull(unforcedSubtitle.forcedState) - } - - @Test - fun forcedState_usesApiFlagInsteadOfUrlHeuristic() { - val forcedSubtitle = SubtitleLink( - lang = "rus", - url = "https://cdn.test/subtitles/russian.srt", - forced = true, - file = "/a/71/29725.srt", - ) - val regularSubtitle = SubtitleLink( - lang = "rus", - url = "https://cdn.test/subtitles/russian-forced.srt", - forced = false, - ) - - assertEquals(true, forcedSubtitle.forcedState) - assertEquals(false, regularSubtitle.forcedState) + assertTrue(SubtitleLink(lang = "eng", url = url).shouldSideLoad) + assertTrue(SubtitleLink(lang = "eng", url = url, embed = false).shouldSideLoad) + assertFalse(SubtitleLink(lang = "eng", url = url, embed = true).shouldSideLoad) } } diff --git a/app/src/test/kotlin/com/kino/puber/ui/feature/player/model/PlayerUIMapperSubtitleTest.kt b/app/src/test/kotlin/com/kino/puber/ui/feature/player/model/PlayerUIMapperSubtitleTest.kt index 2840caca..117ff017 100644 --- a/app/src/test/kotlin/com/kino/puber/ui/feature/player/model/PlayerUIMapperSubtitleTest.kt +++ b/app/src/test/kotlin/com/kino/puber/ui/feature/player/model/PlayerUIMapperSubtitleTest.kt @@ -47,34 +47,4 @@ internal class PlayerUIMapperSubtitleTest { assertEquals(listOf(null, false, null), result.map { it.isForced }) } - @Test - fun mapSubtitleTracks_leavesLabellingToTheMerger_forSameLanguageVariants() { - val result = mapper.mapSubtitleTracks( - listOf( - SubtitleLink( - lang = "rus", - url = "https://cdn.test/subtitles/russian-full.vtt", - embed = true, - forced = false, - ), - SubtitleLink( - lang = "rus", - url = "https://cdn.test/subtitles/russian-forced.vtt", - embed = true, - forced = true, - ), - ) - ) - - assertEquals(listOf("rus", "rus"), result.drop(1).map { it.label }) - assertEquals(listOf("rus", "rus"), result.drop(1).map { it.language }) - assertEquals( - listOf( - "https://cdn.test/subtitles/russian-full.vtt", - "https://cdn.test/subtitles/russian-forced.vtt", - ), - result.drop(1).map { it.url }, - ) - assertEquals(listOf(false, true), result.drop(1).map { it.isForced }) - } } diff --git a/app/src/test/kotlin/com/kino/puber/ui/feature/player/vm/AudioTrackPreferenceResolverTest.kt b/app/src/test/kotlin/com/kino/puber/ui/feature/player/vm/AudioTrackPreferenceResolverTest.kt index 703e4e28..f5a770d9 100644 --- a/app/src/test/kotlin/com/kino/puber/ui/feature/player/vm/AudioTrackPreferenceResolverTest.kt +++ b/app/src/test/kotlin/com/kino/puber/ui/feature/player/vm/AudioTrackPreferenceResolverTest.kt @@ -94,24 +94,6 @@ internal class AudioTrackPreferenceResolverTest { assertEquals(0, result) } - @Test - fun findSubtitleTrackIndex_prefersCurrentPlayerIdentity_overLanguageFallback() { - val tracks = listOf( - subtitleTrack(index = 0, language = "", url = ""), - subtitleTrack(index = 1, language = "rus", url = "", playerTrackId = "full"), - subtitleTrack(index = 2, language = "rus", url = "", playerTrackId = "forced"), - ) - - val result = resolver.findSubtitleTrackIndex( - tracks = tracks, - preferredLang = "rus", - preferredUrl = "", - preferredPlayerTrackId = "forced", - ) - - assertEquals(2, result) - } - @Test fun findSubtitleTrackIndex_usesPlayerCoordinates_whenSelectedFormatLosesItsId() { val tracks = listOf( diff --git a/app/src/test/kotlin/com/kino/puber/ui/feature/player/vm/PlayerVMSubtitleVariantTest.kt b/app/src/test/kotlin/com/kino/puber/ui/feature/player/vm/PlayerVMSubtitleVariantTest.kt index 2d49b14c..eaa135b2 100644 --- a/app/src/test/kotlin/com/kino/puber/ui/feature/player/vm/PlayerVMSubtitleVariantTest.kt +++ b/app/src/test/kotlin/com/kino/puber/ui/feature/player/vm/PlayerVMSubtitleVariantTest.kt @@ -76,24 +76,6 @@ internal class PlayerVMSubtitleVariantTest : PlayerVMTestFixture() { verify { playbackController.selectSubtitle(selectedTrack) } } - @Test - fun tracksUpdated_defersUrlPreference_untilPlayerTrackAppears() { - every { interactor.getPreferredSubtitleLang(42) } returns "rus" - every { interactor.getPreferredSubtitleUrl(42) } returns - "https://test/subtitles/rus-forced.vtt" - val vm = startedVM() - val audioTracks = listOf(AudioTrackUIState(0, "English", "eng")) - - callbackSlot.captured.onTracksUpdated(audioTracks, 0, emptyList()) - verify(exactly = 0) { playbackController.selectSubtitle(any()) } - - callbackSlot.captured.onTracksUpdated(audioTracks, 0, testDiscoveredSubtitleTracks) - - val selectedTrack = contentState(vm).subtitleTracks[2] - assertEquals(2, contentState(vm).selectedSubtitleIndex) - verify { playbackController.selectSubtitle(selectedTrack) } - } - @Test fun tracksUpdated_restoresForcedManifestSubtitleBySavedIdentity() { every { interactor.getPreferredSubtitleLang(42) } returns "rus" diff --git a/app/src/test/kotlin/com/kino/puber/ui/feature/player/vm/SubtitleLabelerTest.kt b/app/src/test/kotlin/com/kino/puber/ui/feature/player/vm/SubtitleLabelerTest.kt index cf20acb5..2779d383 100644 --- a/app/src/test/kotlin/com/kino/puber/ui/feature/player/vm/SubtitleLabelerTest.kt +++ b/app/src/test/kotlin/com/kino/puber/ui/feature/player/vm/SubtitleLabelerTest.kt @@ -92,17 +92,11 @@ internal class SubtitleLabelerTest { @Test fun apply_fallsBackToPositionalName_whenNothingIdentifiesTheLanguage() { - val labels = labeler.apply(listOf(track(""), track("", descriptive = "Комментарии режиссёра"))) - .map { it.label } - - assertEquals(listOf("Субтитры 1", "Комментарии режиссёра"), labels) - } - - @Test - fun apply_leavesUnknownLanguageCodeVisible_ratherThanInventingAName() { - val labels = labeler.apply(listOf(track("qqq"))).map { it.label } + val labels = labeler.apply( + listOf(track(""), track("", descriptive = "Комментарии режиссёра"), track("qqq")), + ).map { it.label } - assertEquals(listOf("Субтитры 1"), labels) + assertEquals(listOf("Субтитры 1", "Комментарии режиссёра", "Субтитры 3"), labels) } private fun track( diff --git a/app/src/test/kotlin/com/kino/puber/ui/feature/player/vm/SubtitleTrackMergerTest.kt b/app/src/test/kotlin/com/kino/puber/ui/feature/player/vm/SubtitleTrackMergerTest.kt index 9c45a9f7..e906ff0b 100644 --- a/app/src/test/kotlin/com/kino/puber/ui/feature/player/vm/SubtitleTrackMergerTest.kt +++ b/app/src/test/kotlin/com/kino/puber/ui/feature/player/vm/SubtitleTrackMergerTest.kt @@ -80,22 +80,6 @@ internal class SubtitleTrackMergerTest { assertTrue(result[2].isForced!!) } - @Test - fun merge_appendsManifestOnlyTracks_withoutChangingTheirLanguage() { - val playerTrack = playerTrack( - label = "Українські", - language = "uk", - id = "hls-ukrainian", - groupIndex = 2, - ) - - val result = merger.merge(listOf(offTrack()), listOf(playerTrack)) - - assertEquals(listOf("", "uk"), result.map { it.language }) - assertEquals("hls-ukrainian", result[1].playerTrackId) - assertEquals(2, result[1].playerGroupIndex) - } - @Test fun merge_dropsUnmatchedApiTrack_whenPlayerTracksAreAvailable() { val apiTracks = listOf( @@ -237,29 +221,6 @@ internal class SubtitleTrackMergerTest { assertEquals(apiTrack.url, result[1].url) } - @Test - fun merge_usesOnlyManifestTracks_whenApiIdentityDoesNotMatch() { - val embeddedRussian = apiTrack( - label = "Russian embedded", - language = "rus", - ) - val manifestEnglish = playerTrack( - label = "English HLS", - language = "en", - id = "hls-english", - groupIndex = 0, - ) - - val result = merger.merge( - listOf(offTrack(), embeddedRussian), - listOf(manifestEnglish), - ) - - assertEquals(listOf("Off", "Английский"), result.map { it.label }) - assertEquals(listOf("", "en"), result.map { it.language }) - assertEquals("hls-english", result[1].playerTrackId) - } - @Test fun merge_exposesOnlyOff_untilPlayerTracksAreDiscovered() { val apiTracks = listOf( @@ -273,34 +234,6 @@ internal class SubtitleTrackMergerTest { assertEquals(listOf("Off"), result.map { it.label }) } - @Test - fun merge_usesDescriptiveLabels_forSameLanguageVariantsThatWouldCollide() { - val playerTracks = listOf( - playerTrack("rus", "rus", "full", 0, descriptiveLabel = "Русские полные"), - playerTrack("rus", "rus", "sdh", 1, descriptiveLabel = "Русские SDH"), - ) - - val result = merger.merge(listOf(offTrack()), playerTracks) - - assertEquals(listOf("Off", "Русские полные", "Русские SDH"), result.map { it.label }) - } - - @Test - fun merge_numbersSameLanguageVariants_whenDescriptiveLabelsCannotSeparateThem() { - val playerTracks = listOf( - playerTrack("rus", "rus", "a", 0, descriptiveLabel = "RUS"), - playerTrack("rus", "rus", "b", 1, descriptiveLabel = "RUS"), - playerTrack("rus", "rus", "c", 2, descriptiveLabel = null), - ) - - val result = merger.merge(listOf(offTrack()), playerTracks) - - assertEquals( - listOf("Off", "Русский · вариант 1", "Русский · вариант 2", "Русский · вариант 3"), - result.map { it.label }, - ) - } - @Test fun merge_marksPartialVariant_andOrdersItLastWithinItsLanguage() { val playerTracks = listOf( @@ -314,18 +247,6 @@ internal class SubtitleTrackMergerTest { assertEquals(listOf(null, false, true), result.map { it.isForced }) } - @Test - fun merge_leavesDistinctLabelsAlone() { - val playerTracks = listOf( - playerTrack("rus", "rus", "a", 0, descriptiveLabel = "RUS"), - playerTrack("eng", "eng", "b", 1, descriptiveLabel = "ENG"), - ) - - val result = merger.merge(listOf(offTrack()), playerTracks) - - assertEquals(listOf("Off", "Русский", "Английский"), result.map { it.label }) - } - /** * Real KinoPub data: the API lists two subtitles, the manifest publishes three * renditions covering both languages, and Media3 prefixes side-loaded track ids with @@ -355,9 +276,9 @@ internal class SubtitleTrackMergerTest { } /** - * Renditions and API subtitles share no identifier, so coverage cannot be proven per - * track. When the manifest offers fewer renditions of a language than the API has - * subtitles for it, nothing is hidden — a duplicate row is better than a lost subtitle. + * The complement of the case above: with no renditions in the manifest the side-loaded + * tracks are the only subtitles there are, so they stay and pick up their API metadata + * through the merged child index prefix Media3 puts on their ids. */ @Test fun merge_carriesSideLoadedTracks_whenTheManifestPublishesNoSubtitles() { @@ -380,47 +301,6 @@ internal class SubtitleTrackMergerTest { ) } - @Test - fun merge_matchesSideLoadedTrack_ignoringTheMergedChildIndexPrefix() { - val apiTracks = listOf( - offTrack(), - apiTrack("ukr", "ukr", url = "https://api.test/pd/abc", sourceFile = "/1/92/2871463.srt"), - ) - val playerTracks = listOf( - playerTrack("", "uk", "2:1/92/2871463.srt", 0), - ) - - val result = merger.merge(apiTracks, playerTracks) - - assertEquals(listOf("Off", "Украинский"), result.map { it.label }) - assertEquals("https://api.test/pd/abc", result[1].url) - } - - @Test - fun merge_hidesSideLoadedCopy_whenManifestCoversItsLanguage() { - val apiTracks = listOf( - offTrack(), - apiTrack("rus", "rus", url = "https://api.test/subtitles/rus.srt"), - ) - val playerTracks = listOf( - playerTrack( - label = "rus", - language = "rus", - id = "hls-rus", - groupIndex = 0, - uri = "https://cdn.test/subtitles/rus.srt", - ), - // The side-loaded copy Media3 built from the same API url. - playerTrack("rus", "rus", "rus.srt", 1), - ) - - val result = merger.merge(apiTracks, playerTracks) - - assertEquals(listOf("Off", "Русский"), result.map { it.label }) - assertEquals("hls-rus", result[1].playerTrackId) - assertEquals("https://api.test/subtitles/rus.srt", result[1].url) - } - @Test fun merge_hidesSideLoadedTrack_evenWhenItsLanguageIsMissingFromTheManifest() { val apiTracks = listOf( diff --git a/app/src/test/kotlin/com/kino/puber/ui/feature/player/vm/SubtitleTrackSelectorTest.kt b/app/src/test/kotlin/com/kino/puber/ui/feature/player/vm/SubtitleTrackSelectorTest.kt index 4241b6df..032cbc38 100644 --- a/app/src/test/kotlin/com/kino/puber/ui/feature/player/vm/SubtitleTrackSelectorTest.kt +++ b/app/src/test/kotlin/com/kino/puber/ui/feature/player/vm/SubtitleTrackSelectorTest.kt @@ -31,40 +31,22 @@ internal class SubtitleTrackSelectorTest { } /** - * The same collision with a rendition the manifest publishes without an ID: there is no - * format id to match on, so the old rules fell through to the stable key and selected - * the side-loaded duplicate. The track group id resolves it unambiguously. + * MergingMediaSource prefixes child format ids but not labels, so a side-loaded row + * whose captured id no longer matches still resolves through the label Media3 copied + * from the subtitle configuration. */ @Test - fun select_prefersIdLessManifestRendition_overSideLoadedCopy() { - val manifestRow = manifestTrack( - groupId = "0:rus", - trackIndex = 0, - formatId = null, - url = "https://api.test/subtitles/29725.srt", - ) - val candidates = listOf( - PlayerTextTrack("0:rus", 0, 0, formatLabel = "Русские", language = "rus"), - PlayerTextTrack("1:", 1, 0, formatId = "29725.srt", formatLabel = "29725.srt", language = "rus"), - ) - - assertEquals(candidates[0], selector.select(manifestRow, candidates)) - } - - @Test - fun select_resolvesSideLoadedRow_byStableKey() { + fun select_resolvesSideLoadedRow_byStableKey_whenItsFormatIdNoLongerMatches() { val sideLoadedRow = SubtitleTrackUIState( label = "eng", language = "eng", url = "https://api.test/subtitles/29726.srt", - playerTrackGroupId = "1:", - playerTrackId = "29726.srt", - playerGroupIndex = 1, - playerTrackIndex = 0, + playerTrackGroupId = "", + playerTrackId = "1:29726.srt", ) val candidates = listOf( - PlayerTextTrack("0:rus-rendition", 0, 0, formatId = "rus-rendition", language = "rus"), - PlayerTextTrack("1:", 1, 0, formatId = "29726.srt", formatLabel = "29726.srt", language = "eng"), + PlayerTextTrack("", 0, 0, formatId = "rus-rendition", language = "rus"), + PlayerTextTrack("", 1, 0, formatId = "2:29726.srt", formatLabel = "29726.srt", language = "eng"), ) assertEquals(candidates[1], selector.select(sideLoadedRow, candidates)) @@ -138,11 +120,6 @@ internal class SubtitleTrackSelectorTest { assertEquals(candidates[1], selector.select(row, candidates)) } - @Test - fun select_returnsNull_whenThePlayerExposesNoTextTracks() { - assertNull(selector.select(manifestTrack("0:a", 0, "a", ""), emptyList())) - } - private fun manifestTrack( groupId: String, trackIndex: Int, From 00710f61de0c68a6a6c72a2df1168f2b5e8894d5 Mon Sep 17 00:00:00 2001 From: Innokentii Enikeev Date: Sat, 29 Aug 2026 13:41:39 +0200 Subject: [PATCH 17/25] Use one construction style in the subtitle tests The track builders were called both as a single positional line and as a five-line named block, for the same helper in the same file. Fold every call that fits the line limit. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01GEBLAtGvP5AHqHemWgLXAp --- .../vm/AudioTrackPreferenceResolverTest.kt | 14 +---- .../player/vm/SubtitleTrackMergerTest.kt | 57 +++---------------- .../player/vm/SubtitleTrackSelectorTest.kt | 8 +-- 3 files changed, 12 insertions(+), 67 deletions(-) diff --git a/app/src/test/kotlin/com/kino/puber/ui/feature/player/vm/AudioTrackPreferenceResolverTest.kt b/app/src/test/kotlin/com/kino/puber/ui/feature/player/vm/AudioTrackPreferenceResolverTest.kt index f5a770d9..b2adfbd8 100644 --- a/app/src/test/kotlin/com/kino/puber/ui/feature/player/vm/AudioTrackPreferenceResolverTest.kt +++ b/app/src/test/kotlin/com/kino/puber/ui/feature/player/vm/AudioTrackPreferenceResolverTest.kt @@ -140,18 +140,8 @@ internal class AudioTrackPreferenceResolverTest { fun findSubtitleTrackIndex_doesNotGuessWhenStableIdentityMatchesMultipleTracks() { val tracks = listOf( subtitleTrack(index = 0, language = "", url = ""), - subtitleTrack( - index = 1, - language = "rus", - url = "", - playerTrackUri = "https://cdn.test/a/russian.srt", - ), - subtitleTrack( - index = 2, - language = "rus", - url = "", - playerTrackUri = "https://cdn.test/b/russian.srt", - ), + subtitleTrack(index = 1, language = "rus", url = "", playerTrackUri = "https://cdn.test/a/russian.srt"), + subtitleTrack(index = 2, language = "rus", url = "", playerTrackUri = "https://cdn.test/b/russian.srt"), ) val result = resolver.findSubtitleTrackIndex( diff --git a/app/src/test/kotlin/com/kino/puber/ui/feature/player/vm/SubtitleTrackMergerTest.kt b/app/src/test/kotlin/com/kino/puber/ui/feature/player/vm/SubtitleTrackMergerTest.kt index e906ff0b..7ffa17c4 100644 --- a/app/src/test/kotlin/com/kino/puber/ui/feature/player/vm/SubtitleTrackMergerTest.kt +++ b/app/src/test/kotlin/com/kino/puber/ui/feature/player/vm/SubtitleTrackMergerTest.kt @@ -21,30 +21,12 @@ internal class SubtitleTrackMergerTest { fun merge_usesPlayerTracksAsBackbone_andEnrichesExactIdentity() { val apiTracks = listOf( offTrack(), - apiTrack( - label = "Russian embedded", - language = "rus", - url = "https://api.test/subtitles/embedded-rus.srt", - ), - apiTrack( - label = "Russian external", - language = "rus", - url = "https://api.test/subtitles/external-rus.srt", - ), + apiTrack(label = "Russian embedded", language = "rus", url = "https://api.test/subtitles/embedded-rus.srt"), + apiTrack(label = "Russian external", language = "rus", url = "https://api.test/subtitles/external-rus.srt"), ) val playerTracks = listOf( - playerTrack( - label = "external-rus.srt", - language = "rus", - id = "external-rus.srt", - groupIndex = 0, - ), - playerTrack( - label = "Русские полные", - language = "ru", - id = "hls-russian-full", - groupIndex = 1, - ), + playerTrack(label = "external-rus.srt", language = "rus", id = "external-rus.srt", groupIndex = 0), + playerTrack(label = "Русские полные", language = "ru", id = "hls-russian-full", groupIndex = 1), ) val result = merger.merge(apiTracks, playerTracks) @@ -84,11 +66,7 @@ internal class SubtitleTrackMergerTest { fun merge_dropsUnmatchedApiTrack_whenPlayerTracksAreAvailable() { val apiTracks = listOf( offTrack(), - apiTrack( - label = "Russian external", - language = "rus", - url = "https://api.test/subtitles/external.srt", - ), + apiTrack(label = "Russian external", language = "rus", url = "https://api.test/subtitles/external.srt"), ) val playerTracks = listOf( playerTrack("Russian HLS", "ru", "hls-russian", 0), @@ -103,17 +81,8 @@ internal class SubtitleTrackMergerTest { @Test fun merge_doesNotTreatDisplayLabelAsTrackIdentity() { - val apiTrack = apiTrack( - label = "Spanish API", - language = "spa", - url = "https://api.test/subtitles/spanish.srt", - ) - val playerTrack = playerTrack( - label = "spanish.srt", - language = "rus", - id = "hls-russian", - groupIndex = 0, - ) + val apiTrack = apiTrack(label = "Spanish API", language = "spa", url = "https://api.test/subtitles/spanish.srt") + val playerTrack = playerTrack(label = "spanish.srt", language = "rus", id = "hls-russian", groupIndex = 0) val result = merger.merge(listOf(offTrack(), apiTrack), listOf(playerTrack)) @@ -158,16 +127,8 @@ internal class SubtitleTrackMergerTest { fun merge_usesApiFilePathToMatchHlsRenditionUri_whenLanguageOrderDiffers() { val apiTracks = listOf( offTrack(), - apiTrack( - label = "rus #1", - language = "rus", - sourceFile = "/a/71/first.srt", - ), - apiTrack( - label = "rus #2", - language = "rus", - sourceFile = "/b/82/second.srt", - ), + apiTrack(label = "rus #1", language = "rus", sourceFile = "/a/71/first.srt"), + apiTrack(label = "rus #2", language = "rus", sourceFile = "/b/82/second.srt"), ) val playerTracks = listOf( playerTrack( diff --git a/app/src/test/kotlin/com/kino/puber/ui/feature/player/vm/SubtitleTrackSelectorTest.kt b/app/src/test/kotlin/com/kino/puber/ui/feature/player/vm/SubtitleTrackSelectorTest.kt index 032cbc38..5d7dce97 100644 --- a/app/src/test/kotlin/com/kino/puber/ui/feature/player/vm/SubtitleTrackSelectorTest.kt +++ b/app/src/test/kotlin/com/kino/puber/ui/feature/player/vm/SubtitleTrackSelectorTest.kt @@ -72,13 +72,7 @@ internal class SubtitleTrackSelectorTest { @Test fun select_picksCorrectVariant_whenGroupIdsAreBlankAndLanguagesCollide() { - val row = manifestTrack( - groupId = "", - trackIndex = 0, - formatId = "forced-rus", - url = "", - groupIndex = 1, - ) + val row = manifestTrack(groupId = "", trackIndex = 0, formatId = "forced-rus", url = "", groupIndex = 1) val candidates = listOf( PlayerTextTrack("", 0, 0, formatId = "full-rus", language = "rus"), PlayerTextTrack("", 1, 0, formatId = "forced-rus", language = "rus"), From 8d84a9545958e7e68789ae03ea697f5d73af2db9 Mon Sep 17 00:00:00 2001 From: Innokentii Enikeev Date: Sat, 29 Aug 2026 13:54:01 +0200 Subject: [PATCH 18/25] Carry the stream container instead of guessing it from the URL selectStreamUrl picked url.hls4 / url.hls / url.http and then flattened the choice to a String, so PlaybackController pattern-matched the URL text to recover which branch had been taken. Return the container along with the url and thread it through prepare/switchStream. Deletes isHlsStreamUrl, its two regex constants, and HlsStreamUrlTest, whose five cases all defended a guess that no longer happens. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01GEBLAtGvP5AHqHemWgLXAp --- .../interactor/player/PlayerInteractor.kt | 42 ++++++++++++------ .../feature/player/vm/PlaybackController.kt | 43 ++++++------------- .../feature/player/vm/PlaybackTransitions.kt | 7 +-- .../puber/ui/feature/player/vm/PlayerVM.kt | 8 ++-- .../interactor/player/PlayerInteractorTest.kt | 10 ++--- .../ui/feature/player/vm/HlsStreamUrlTest.kt | 40 ----------------- .../vm/PlaybackControllerTransitionsTest.kt | 11 ++--- .../feature/player/vm/PlayerStartModeTest.kt | 7 +-- .../ui/feature/player/vm/PlayerVMTest.kt | 2 +- .../feature/player/vm/PlayerVMTestFixture.kt | 5 ++- 10 files changed, 71 insertions(+), 104 deletions(-) delete mode 100644 app/src/test/kotlin/com/kino/puber/ui/feature/player/vm/HlsStreamUrlTest.kt diff --git a/app/src/main/java/com/kino/puber/domain/interactor/player/PlayerInteractor.kt b/app/src/main/java/com/kino/puber/domain/interactor/player/PlayerInteractor.kt index c56fee77..af4df984 100644 --- a/app/src/main/java/com/kino/puber/domain/interactor/player/PlayerInteractor.kt +++ b/app/src/main/java/com/kino/puber/domain/interactor/player/PlayerInteractor.kt @@ -16,6 +16,15 @@ import kotlinx.coroutines.CancellationException private const val WATCHED_STATUS = 1 private const val UNWATCHED_STATUS = 0 +/** + * The stream to play, with the container the API published it under. + * [isHls] is taken from the field the url came from rather than guessed from its text. + */ +internal data class StreamSource( + val url: String, + val isHls: Boolean, +) + internal data class ResolvedMedia( val files: List?, val audios: List