Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion app/build.gradle.kts
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ plugins {

// 支持通过 -PVERSION_NAME / -PVERSION_CODE 显式传入版本信息。
// 未显式传入 VERSION_NAME 时,默认把最后一段 patch 替换为 BUILD_NUMBER。
val baseAppVersionName = "2.6.0"
val baseAppVersionName = "2.7.0"

fun String?.nonBlankOrNull(): String? =
this?.trim()?.takeIf { it.isNotBlank() }
Expand Down
Original file line number Diff line number Diff line change
@@ -1,12 +1,15 @@
package com.miruplay.tv.repository

import com.miruplay.tv.core.common.Result
import com.miruplay.tv.model.MediaSourceInfo
import com.miruplay.tv.model.PlaybackSource
import com.miruplay.tv.model.buildExternalSubtitleTracks
import com.miruplay.tv.model.matchingExternalSubtitlePaths

class PlaybackSubtitleResolver(
private val index: MediaIndexRepository,
private val mediaSources: MediaSourceRepository,
private val listSiblingPaths: suspend (MediaSourceInfo, String) -> List<String> = { _, _ -> emptyList() },
) {
suspend fun resolve(source: PlaybackSource): PlaybackSource {
val episodeId = source.episodeId ?: return source
Expand All @@ -15,18 +18,20 @@ class PlaybackSubtitleResolver(
is Result.Success -> result.data
is Result.Error -> return source
}
val entries = when (val result = index.queryIndex(sourceId, "")) {
is Result.Success -> result.data
is Result.Error -> return source
}
val episodePath = episodeId.substringAfter(':', "")
if (episodePath.isBlank()) return source
val entries = index.queryIndex(sourceId, "").getOrNull().orEmpty()
val entry = entries.firstOrNull { indexed ->
mediaSource.playableUriForIndexedPath(indexed.path) == source.uri
} ?: entries.firstOrNull { indexed -> indexed.path == episodePath }
?: return source

val videoPath = entry?.path ?: episodePath
val siblingSubtitlePaths = matchingExternalSubtitlePaths(
videoPath = videoPath,
siblingPaths = listSiblingPaths(mediaSource, videoPath),
)
val discoveredTracks = buildExternalSubtitleTracks(
entry.externalSubtitlePaths.map(mediaSource::playableUriForIndexedPath),
(entry?.externalSubtitlePaths.orEmpty() + siblingSubtitlePaths)
.map(mediaSource::playableUriForIndexedPath),
)
if (discoveredTracks.isEmpty()) return source
return source.copy(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ import org.junit.Test

class PlaybackSubtitleResolverTest {
@Test
fun `resolver attaches indexed WebDAV subtitles and keeps explicit tracks`() = runBlocking {
fun `resolver merges indexed and listed WebDAV subtitles`() = runBlocking {
val mediaSource = MediaSourceInfoConventions.webDav(
url = "https://dav.example/anime",
name = "DAV",
Expand All @@ -30,6 +30,15 @@ class PlaybackSubtitleResolverTest {
),
),
mediaSources = FakeSourceRepository(mediaSource),
listSiblingPaths = { source, videoPath ->
assertEquals(mediaSource, source)
assertEquals("/Show/Episode 01.mkv", videoPath)
listOf(
"/Show/Episode 01.zh-CN.srt",
"/Show/Episode 01.ja.ass",
"/Show/Episode 02.srt",
)
},
)

val resolved = resolver.resolve(
Expand All @@ -44,11 +53,45 @@ class PlaybackSubtitleResolverTest {
listOf(
"https://dav.example/anime/Show/Episode%2001.ass",
"https://dav.example/anime/Show/Episode%2001.zh-CN.srt",
"https://dav.example/anime/Show/Episode%2001.ja.ass",
),
resolved.subtitleTracks.map { it.path },
)
}

@Test
fun `resolver discovers listed subtitles before index refresh`() = runBlocking {
val mediaSource = MediaSourceInfoConventions.webDav(
url = "https://dav.example/anime",
name = "DAV",
).copy(id = 7L)
val resolver = PlaybackSubtitleResolver(
index = FakeIndexRepository(emptyList()),
mediaSources = FakeSourceRepository(mediaSource),
listSiblingPaths = { _, _ ->
listOf(
"/Show/Episode 01.mkv",
"/Show/Episode 01.zh-Hant.ass",
"/Show/readme.txt",
)
},
)

val resolved = resolver.resolve(
PlaybackSource(
uri = "https://dav.example/anime/Show/Episode%2001.mkv",
mediaSourceId = "anime",
episodeId = "7:/Show/Episode 01.mkv",
),
)

assertEquals(
listOf("https://dav.example/anime/Show/Episode%2001.zh-Hant.ass"),
resolved.subtitleTracks.map { it.path },
)
assertEquals("zh-Hant", resolved.subtitleTracks.single().language)
}

@Test
fun `resolver leaves direct playback without an indexed episode unchanged`() = runBlocking {
val source = PlaybackSource(uri = "https://example.test/video.mkv", mediaSourceId = "direct")
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,46 @@
package com.miruplay.tv.ui.player

import android.net.Uri
import android.provider.DocumentsContract
import com.miruplay.tv.mediasource.MediaSourceFactory
import com.miruplay.tv.model.MediaPathConventions
import com.miruplay.tv.model.MediaSourceInfo

internal suspend fun listPlaybackSiblingPaths(
mediaSourceFactory: MediaSourceFactory,
mediaSourceInfo: MediaSourceInfo,
videoPath: String,
): List<String> {
val parentPath = playbackParentDirectoryPath(videoPath) ?: return emptyList()
val mediaSource = mediaSourceFactory.create(mediaSourceInfo).getOrNull() ?: return emptyList()
return try {
mediaSource.listFiles(parentPath)
.getOrNull()
.orEmpty()
.asSequence()
.filterNot { it.isDirectory }
.map { it.path }
.toList()
} catch (_: Exception) {
emptyList()
} finally {
try {
mediaSource.close()
} catch (_: Exception) {
// Directory subtitle discovery is best-effort and must not block playback.
}
Comment on lines +24 to +31

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🩺 Stability & Availability | 🟠 Major | ⚡ Quick win

🧩 Analysis chain

🌐 Web query:

In Kotlin coroutines, does catch (Exception)catchCancellationException, and should suspend functions rethrow it?

💡 Result:

Yes, a catch(Exception) or catch(Throwable) block in Kotlin will catch a CancellationException because it is a subclass of Exception [1][2][3]. However, you should always rethrow CancellationException if you catch it [1][2][4]. The reason is that CancellationException is used by the coroutines library to signal that a coroutine has been cancelled [1][2]. If you catch it and do not rethrow it, you suppress this signal, which can prevent the coroutine from terminating correctly, break structured concurrency, and lead to unexpected behavior where a coroutine continues to run after it should have been cancelled [4][5]. If you need to perform cleanup or handle exceptions within a coroutine, the recommended pattern is: 1. Catch the exception. 2. Check if it is a CancellationException. 3. If it is, rethrow it immediately [1][2][4]. Example: try { // some suspending code } catch (e: Exception) { if (e is CancellationException) { throw e // Always rethrow CancellationException } // handle other exceptions } Alternatively, if you are catching a generic exception and want to ensure the coroutine terminates if it has been cancelled, you can call ensureActive within your catch block [6][4]. This checks the cancellation status of the coroutine and throws a CancellationException if the coroutine is cancelled [1][4].

Citations:


🏁 Script executed:

#!/bin/bash
set -eu

file="ui-tv/src/main/kotlin/com/miruplay/tv/ui/player/PlaybackDirectorySubtitleFiles.kt"

printf '%s\n' '--- file outline ---'
ast-grep outline "$file" --lang kotlin || true

printf '%s\n' '--- relevant source ---'
cat -n "$file"

printf '%s\n' '--- references ---'
rg -n -C 4 'listPlaybackSiblingPaths|PlaybackDirectorySubtitleFiles' .

Repository: ModerRAS/MiruPlay

Length of output: 6779


🏁 Script executed:

#!/bin/bash
set -eu

printf '%s\n' '--- MediaSource declarations and implementations ---'
rg -n -C 5 'interface MediaSource|abstract class .*MediaSource|fun listFiles|fun close' ui-tv/src/main/kotlin

printf '%s\n' '--- resolver and ViewModel coroutine context ---'
cat -n ui-tv/src/main/kotlin/com/miruplay/tv/ui/player/PlaybackSubtitleResolver.kt 2>/dev/null || true
sed -n '160,215p' ui-tv/src/main/kotlin/com/miruplay/tv/ui/player/PlayerViewModel.kt

printf '%s\n' '--- relevant tests ---'
cat -n ui-tv/src/test/kotlin/com/miruplay/tv/ui/player/PlaybackDirectorySubtitleFilesTest.kt

Repository: ModerRAS/MiruPlay

Length of output: 9815


🏁 Script executed:

#!/bin/bash
set -eu

printf '%s\n' '--- candidate media-source files ---'
git ls-files | rg '(^|/)(MediaSource|.*MediaSource.*|PlaybackSubtitleResolver)\.(kt|java)$'

printf '%s\n' '--- all relevant declarations ---'
rg -n -C 6 'interface MediaSource|class MediaSource|listFiles\(|close\(\)' --glob '*.kt' --glob '*.java' .

printf '%s\n' '--- resolver source ---'
resolver="$(git ls-files | rg '/PlaybackSubtitleResolver\.kt$' | head -n 1)"
if [ -n "$resolver" ]; then
  cat -n "$resolver"
fi

Repository: ModerRAS/MiruPlay

Length of output: 50374


🏁 Script executed:

#!/bin/bash
set -eu

printf '%s\n' '--- MediaSource API ---'
cat -n media-source-api/src/main/kotlin/com/miruplay/tv/mediasource/MediaSource.kt
printf '%s\n' '--- MediaSource implementations close/list signatures ---'
rg -n -C 3 'override suspend fun (listFiles|close)|override fun (listFiles|close)|suspend fun (listFiles|close)' media-source media-source-api

printf '%s\n' '--- PlaybackSubtitleResolver ---'
cat -n repository-api/src/main/kotlin/com/miruplay/tv/repository/PlaybackSubtitleResolver.kt

printf '%s\n' '--- dependency versions for coroutine semantics ---'
rg -n -C 2 'kotlinx-coroutines|kotlin\(' gradle/libs.versions.toml build.gradle.kts settings.gradle.kts gradle --glob '*.kts' --glob '*.toml'

Repository: ModerRAS/MiruPlay

Length of output: 15341


🏁 Script executed:

#!/bin/bash
set -eu

printf '%s\n' '--- MediaSource API ---'
cat -n media-source-api/src/main/kotlin/com/miruplay/tv/mediasource/MediaSource.kt

printf '%s\n' '--- MediaSource implementations close/list signatures ---'
rg -n -C 3 'override suspend fun (listFiles|close)|override fun (listFiles|close)|suspend fun (listFiles|close)' media-source media-source-api

printf '%s\n' '--- PlaybackSubtitleResolver ---'
cat -n repository-api/src/main/kotlin/com/miruplay/tv/repository/PlaybackSubtitleResolver.kt

printf '%s\n' '--- dependency versions for coroutine semantics ---'
rg -n -C 2 'kotlinx-coroutines|kotlin\(' gradle/libs.versions.toml build.gradle.kts settings.gradle.kts gradle --glob '*.kts' --glob '*.toml'

Repository: ModerRAS/MiruPlay

Length of output: 15341


🏁 Script executed:

#!/bin/bash
set -eu

printf '%s\n' '--- project CancellationException usage ---'
rg -n -C 3 'CancellationException|ensureActive|catch \(_?: Exception\)' --glob '*.kt' .

printf '%s\n' '--- standalone JVM exception-hierarchy probe ---'
if command -v jshell >/dev/null 2>&1; then
  jshell <<'EOF'
import java.util.concurrent.CancellationException;
System.out.println("CancellationException extends Exception: " +
    Exception.class.isAssignableFrom(CancellationException.class));
try {
    throw new CancellationException("cancelled");
} catch (Exception error) {
    System.out.println("catch(Exception) receives it: " +
        (error instanceof CancellationException));
}
EOF
else
  printf '%s\n' 'jshell unavailable'
fi

printf '%s\n' '--- source assertions ---'
python3 - <<'PY'
from pathlib import Path

path = Path("ui-tv/src/main/kotlin/com/miruplay/tv/ui/player/PlaybackDirectorySubtitleFiles.kt")
text = path.read_text()
assert "internal suspend fun listPlaybackSiblingPaths" in text
assert text.count("catch (_: Exception)") == 2
assert "mediaSource.listFiles(parentPath)" in text
assert "mediaSource.close()" in text
print("listPlaybackSiblingPaths has two broad Exception handlers around listFiles and close")
PY

Repository: ModerRAS/MiruPlay

Length of output: 25567


🏁 Script executed:

#!/bin/bash
set -eu

printf '%s\n' '--- project CancellationException usage ---'
rg -n -C 3 'CancellationException|ensureActive|catch \(_?: Exception\)' --glob '*.kt' .

printf '%s\n' '--- standalone JVM exception-hierarchy probe ---'
if command -v jshell >/dev/null 2>&1; then
  jshell <<'EOF'
import java.util.concurrent.CancellationException;
System.out.println("CancellationException extends Exception: " +
    Exception.class.isAssignableFrom(CancellationException.class));
try {
    throw new CancellationException("cancelled");
} catch (Exception error) {
    System.out.println("catch(Exception) receives it: " +
        (error instanceof CancellationException));
EOF
else
  printf '%s\n' 'jshell unavailable'
fi

printf '%s\n' '--- source assertions ---'
python3 - <<'PY'
from pathlib import Path

path = Path("ui-tv/src/main/kotlin/com/miruplay/tv/ui/player/PlaybackDirectorySubtitleFiles.kt")
text = path.read_text()
assert "internal suspend fun listPlaybackSiblingPaths" in text
assert text.count("catch (_: Exception)") == 2
assert "mediaSource.listFiles(parentPath)" in text
assert "mediaSource.close()" in text
print("listPlaybackSiblingPaths has two broad Exception handlers around listFiles and close")
PY

Repository: ModerRAS/MiruPlay

Length of output: 25567


Propagate CancellationException from both suspend calls

listFiles and close are suspend functions. Both catch (Exception) handlers can swallow cancellation. Rethrow CancellationException before handling other exceptions so PlaybackSubtitleResolver cannot continue after cancellation.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In
`@ui-tv/src/main/kotlin/com/miruplay/tv/ui/player/PlaybackDirectorySubtitleFiles.kt`
around lines 24 - 31, Update both exception handlers in
PlaybackDirectorySubtitleFiles so they rethrow CancellationException before
converting other failures to an empty list or ignoring close errors. Preserve
the existing best-effort handling for non-cancellation exceptions in the
listFiles and mediaSource.close suspend calls.

}
}

internal fun playbackParentDirectoryPath(videoPath: String): String? {
if (!videoPath.startsWith("content://", ignoreCase = true)) {
return MediaPathConventions.parentPath(videoPath)
}
return runCatching {
val videoUri = Uri.parse(videoPath)
val documentId = DocumentsContract.getDocumentId(videoUri)
val parentDocumentId = documentId.substringBeforeLast('/', "").takeIf(String::isNotBlank)
?: return@runCatching null
DocumentsContract.buildDocumentUriUsingTree(videoUri, parentDocumentId).toString()
}.getOrNull()
}
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import android.view.View
import androidx.lifecycle.ViewModel
import androidx.lifecycle.viewModelScope
import com.miruplay.tv.core.common.Result
import com.miruplay.tv.mediasource.MediaSourceFactory
import com.miruplay.tv.model.Episode
import com.miruplay.tv.model.EpisodeVersion
import com.miruplay.tv.model.EpisodeVersionSelectionPolicy
Expand Down Expand Up @@ -62,6 +63,7 @@ class PlayerViewModel @Inject constructor(
private val progressRepository: PlaybackProgressRepository,
private val metadataRepository: Lazy<MetadataRepository>,
private val mediaRepository: Lazy<MediaSourceRepository>,
private val mediaSourceFactory: Lazy<MediaSourceFactory>,
private val mediaIndexRepository: Lazy<MediaIndexRepository>,
private val bangumiSyncEngine: Lazy<BangumiSyncEngine>,
private val playbackPreferences: PlaybackPreferencesRepository,
Expand Down Expand Up @@ -194,6 +196,9 @@ class PlayerViewModel @Inject constructor(
PlaybackSubtitleResolver(
index = mediaIndexRepository.get(),
mediaSources = mediaRepository.get(),
listSiblingPaths = { mediaSourceInfo, videoPath ->
listPlaybackSiblingPaths(mediaSourceFactory.get(), mediaSourceInfo, videoPath)
},
)
}

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,55 @@
package com.miruplay.tv.ui.player

import com.miruplay.tv.core.common.Result
import com.miruplay.tv.mediasource.MediaSource
import com.miruplay.tv.mediasource.MediaSourceFactory
import com.miruplay.tv.model.FileEntry
import com.miruplay.tv.model.MediaSourceInfoConventions
import io.mockk.Runs
import io.mockk.coEvery
import io.mockk.coVerify
import io.mockk.every
import io.mockk.just
import io.mockk.mockk
import kotlinx.coroutines.runBlocking
import org.junit.Assert.assertEquals
import org.junit.Test
import org.junit.runner.RunWith
import org.robolectric.RobolectricTestRunner

@RunWith(RobolectricTestRunner::class)
class PlaybackDirectorySubtitleFilesTest {
@Test
fun `parent directory supports regular and document tree paths`() {
assertEquals("/Show", playbackParentDirectoryPath("/Show/Episode 01.mkv"))
assertEquals(
"content://com.android.externalstorage.documents/tree/primary%3AAnime/document/primary%3AAnime%2FShow",
playbackParentDirectoryPath(
"content://com.android.externalstorage.documents/tree/primary%3AAnime/document/" +
"primary%3AAnime%2FShow%2FEpisode%2001.mkv",
),
)
}

@Test
fun `directory listing returns file paths and closes source`() = runBlocking {
val info = MediaSourceInfoConventions.webDav("https://dav.example/anime", "DAV").copy(id = 7L)
val mediaSource = mockk<MediaSource>()
val factory = mockk<MediaSourceFactory>()
every { factory.create(info) } returns Result.success(mediaSource)
coEvery { mediaSource.listFiles("/Show") } returns Result.success(
listOf(
FileEntry("Episode 01.mkv", "/Show/Episode 01.mkv", isDirectory = false),
FileEntry("Episode 01.zh-CN.ass", "/Show/Episode 01.zh-CN.ass", isDirectory = false),
FileEntry("Subs", "/Show/Subs", isDirectory = true),
),
)
coEvery { mediaSource.close() } just Runs

assertEquals(
listOf("/Show/Episode 01.mkv", "/Show/Episode 01.zh-CN.ass"),
listPlaybackSiblingPaths(factory, info, "/Show/Episode 01.mkv"),
)
coVerify(exactly = 1) { mediaSource.close() }
}
}
Loading