-
Notifications
You must be signed in to change notification settings - Fork 0
feat(player): discover external subtitles in media directory #66
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Merged
Merged
Changes from all commits
Commits
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
46 changes: 46 additions & 0 deletions
46
ui-tv/src/main/kotlin/com/miruplay/tv/ui/player/PlaybackDirectorySubtitleFiles.kt
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| 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. | ||
| } | ||
| } | ||
| } | ||
|
|
||
| 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() | ||
| } | ||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
55 changes: 55 additions & 0 deletions
55
ui-tv/src/test/kotlin/com/miruplay/tv/ui/player/PlaybackDirectorySubtitleFilesTest.kt
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| 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() } | ||
| } | ||
| } |
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
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, doescatch (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:
runCatchingthat does not handle aCancellationExceptionbut re-throws it instead. Kotlin/kotlinx.coroutines#1814🏁 Script executed:
Repository: ModerRAS/MiruPlay
Length of output: 6779
🏁 Script executed:
Repository: ModerRAS/MiruPlay
Length of output: 9815
🏁 Script executed:
Repository: ModerRAS/MiruPlay
Length of output: 50374
🏁 Script executed:
Repository: ModerRAS/MiruPlay
Length of output: 15341
🏁 Script executed:
Repository: ModerRAS/MiruPlay
Length of output: 15341
🏁 Script executed:
Repository: ModerRAS/MiruPlay
Length of output: 25567
🏁 Script executed:
Repository: ModerRAS/MiruPlay
Length of output: 25567
Propagate
CancellationExceptionfrom both suspend callslistFilesandcloseare suspend functions. Bothcatch (Exception)handlers can swallow cancellation. RethrowCancellationExceptionbefore handling other exceptions soPlaybackSubtitleResolvercannot continue after cancellation.🤖 Prompt for AI Agents