-
Notifications
You must be signed in to change notification settings - Fork 760
Fix subtitle selection #2449
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
base: master
Are you sure you want to change the base?
Fix subtitle selection #2449
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -11,7 +11,10 @@ import androidx.media3.ui.SubtitleView | |||||||||||||||||||||||||||||||||||||||||||||||||
| import com.lagradost.cloudstream3.SubtitleFile | ||||||||||||||||||||||||||||||||||||||||||||||||||
| import com.lagradost.cloudstream3.ui.subtitles.SaveCaptionStyle | ||||||||||||||||||||||||||||||||||||||||||||||||||
| import com.lagradost.cloudstream3.ui.subtitles.SubtitlesFragment.Companion.setSubtitleViewStyle | ||||||||||||||||||||||||||||||||||||||||||||||||||
| import com.lagradost.cloudstream3.utils.SubtitleHelper | ||||||||||||||||||||||||||||||||||||||||||||||||||
| import com.lagradost.cloudstream3.utils.SubtitleHelper.fromCodeToLangTagIETF | ||||||||||||||||||||||||||||||||||||||||||||||||||
| import com.lagradost.cloudstream3.utils.UIHelper.toPx | ||||||||||||||||||||||||||||||||||||||||||||||||||
| import me.xdrop.fuzzywuzzy.FuzzySearch | ||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||
| enum class SubtitleStatus { | ||||||||||||||||||||||||||||||||||||||||||||||||||
| IS_ACTIVE, | ||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
@@ -47,6 +50,56 @@ data class SubtitleData( | |||||||||||||||||||||||||||||||||||||||||||||||||
| else "$url|$name" | ||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||
| /** Returns true if langCode is the same as the IETF tag */ | ||||||||||||||||||||||||||||||||||||||||||||||||||
| fun matchesLanguage(langCode: String): Boolean { | ||||||||||||||||||||||||||||||||||||||||||||||||||
| return getIETF_tag() == langCode | ||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||
| /** Tries hard to figure out a valid IETF tag based on language code and name. Will return null if not found. */ | ||||||||||||||||||||||||||||||||||||||||||||||||||
| fun getIETF_tag(): String? { | ||||||||||||||||||||||||||||||||||||||||||||||||||
| val tag = fromCodeToLangTagIETF(this.languageCode) | ||||||||||||||||||||||||||||||||||||||||||||||||||
| if (tag != null) { | ||||||||||||||||||||||||||||||||||||||||||||||||||
| return tag | ||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||
| // Remove any numbers to make matching better | ||||||||||||||||||||||||||||||||||||||||||||||||||
| val cleanedLanguage = originalName.replace(Regex("[0-9]"), "").trim() | ||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||
| // First go for exact matches | ||||||||||||||||||||||||||||||||||||||||||||||||||
| SubtitleHelper.languages.forEach { language -> | ||||||||||||||||||||||||||||||||||||||||||||||||||
| if (language.languageName.equals(cleanedLanguage, ignoreCase = true) || | ||||||||||||||||||||||||||||||||||||||||||||||||||
| language.nativeName.equals(cleanedLanguage, ignoreCase = true) || | ||||||||||||||||||||||||||||||||||||||||||||||||||
| // Also match exact IETF tags | ||||||||||||||||||||||||||||||||||||||||||||||||||
| language.IETF_tag.equals(cleanedLanguage, ignoreCase = true) | ||||||||||||||||||||||||||||||||||||||||||||||||||
| ) { | ||||||||||||||||||||||||||||||||||||||||||||||||||
| return language.IETF_tag | ||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+69
to
+77
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is basically what is implemented in the cloudstream/library/src/commonMain/kotlin/com/lagradost/cloudstream3/utils/SubtitleHelper.kt Lines 81 to 104 in c1a2ae8
|
||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||
| var closestMatch: Pair<String?, Int> = null to 0 | ||||||||||||||||||||||||||||||||||||||||||||||||||
| // Then go for partial matches, however only use the best match | ||||||||||||||||||||||||||||||||||||||||||||||||||
| SubtitleHelper.languages.forEach { language -> | ||||||||||||||||||||||||||||||||||||||||||||||||||
| val lowerCleaned = cleanedLanguage.lowercase() | ||||||||||||||||||||||||||||||||||||||||||||||||||
| val score = maxOf( | ||||||||||||||||||||||||||||||||||||||||||||||||||
| FuzzySearch.ratio(lowerCleaned, language.languageName.lowercase()), | ||||||||||||||||||||||||||||||||||||||||||||||||||
| FuzzySearch.ratio( | ||||||||||||||||||||||||||||||||||||||||||||||||||
| lowerCleaned, language.nativeName.lowercase() | ||||||||||||||||||||||||||||||||||||||||||||||||||
| ) | ||||||||||||||||||||||||||||||||||||||||||||||||||
| ) | ||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||
| // Arbitrary cutoff at 80. | ||||||||||||||||||||||||||||||||||||||||||||||||||
| if (cleanedLanguage.contains(language.languageName, ignoreCase = true) || | ||||||||||||||||||||||||||||||||||||||||||||||||||
| cleanedLanguage.contains(language.nativeName, ignoreCase = true) || score > 80 | ||||||||||||||||||||||||||||||||||||||||||||||||||
| ) { | ||||||||||||||||||||||||||||||||||||||||||||||||||
| if (score > closestMatch.second) { | ||||||||||||||||||||||||||||||||||||||||||||||||||
| closestMatch = language.IETF_tag to score | ||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+79
to
+98
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I find it more useful to implement the FuzzySearch within the library, so it can be used by extensions as well, and not just by the app itself. My suggestion is to replace the cloudstream/library/src/commonMain/kotlin/com/lagradost/cloudstream3/utils/SubtitleHelper.kt Lines 81 to 103 in c1a2ae8
|
||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||
| return closestMatch.first | ||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||
| val name = "$originalName $nameSuffix" | ||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||
| /** | ||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,120 @@ | ||
| package com.lagradost.cloudstream3 | ||
|
|
||
| import com.lagradost.cloudstream3.ui.player.SubtitleData | ||
| import com.lagradost.cloudstream3.ui.player.SubtitleOrigin | ||
| import org.junit.Assert.assertEquals | ||
| import org.junit.Assert.assertNull | ||
| import org.junit.Test | ||
|
|
||
| /** Ensure partial subtitle language finding is reliable. */ | ||
| class SubtitleLanguageTagTest { | ||
| fun getQuickSubtitle(originalName: String, languageCode: String?): SubtitleData { | ||
| return SubtitleData( | ||
| originalName = originalName, | ||
| nameSuffix = "1", | ||
| url = "https://example.com/test.vtt", | ||
| origin = SubtitleOrigin.URL, | ||
| mimeType = "text/vtt", | ||
| headers = emptyMap(), | ||
| languageCode = languageCode | ||
| ) | ||
| } | ||
|
|
||
| @Test | ||
| fun `returns languageCode directly if already valid IETF tag`() { | ||
| val subtitle = getQuickSubtitle( | ||
| originalName = "Anything", | ||
| languageCode = "en" | ||
| ) | ||
|
|
||
| assertEquals("en", subtitle.getIETF_tag()) | ||
| } | ||
|
|
||
| @Test | ||
| fun `matches exact language name`() { | ||
| val subtitle = getQuickSubtitle( | ||
| originalName = "English", | ||
| languageCode = null | ||
| ) | ||
|
|
||
| assertEquals("en", subtitle.getIETF_tag()) | ||
| } | ||
|
|
||
| @Test | ||
| fun `matches native language name`() { | ||
| val subtitle = getQuickSubtitle( | ||
| originalName = "Español", | ||
| languageCode = null | ||
| ) | ||
|
|
||
| assertEquals("es", subtitle.getIETF_tag()) | ||
| } | ||
|
|
||
| @Test | ||
| fun `matches fuzzy partial language name`() { | ||
| val subtitle = getQuickSubtitle( | ||
| originalName = "English [SUB]", | ||
| languageCode = null | ||
| ) | ||
|
|
||
| assertEquals("en", subtitle.getIETF_tag()) | ||
| } | ||
|
|
||
| @Test | ||
| fun `returns null when no language matches`() { | ||
| val subtitle = getQuickSubtitle( | ||
| originalName = "Klingon", | ||
| languageCode = null | ||
| ) | ||
|
|
||
| assertNull(subtitle.getIETF_tag()) | ||
| } | ||
|
|
||
|
|
||
| @Test | ||
| fun `returns the correct language variant`() { | ||
| val subtitle1 = getQuickSubtitle( | ||
| originalName = "Chinese", | ||
| languageCode = null | ||
| ) | ||
| val subtitle2 = getQuickSubtitle( | ||
| originalName = "Chinese (subtitle)", | ||
| languageCode = null | ||
| ) | ||
| val subtitleSimplified1 = getQuickSubtitle( | ||
| originalName = "Chinese (simplified)", | ||
| languageCode = null | ||
| ) | ||
| val subtitleSimplified2 = getQuickSubtitle( | ||
| originalName = "Chinese - simplified", | ||
| languageCode = null | ||
| ) | ||
| val subtitleSimplified3 = getQuickSubtitle( | ||
| originalName = "Chinese simplified", | ||
| languageCode = "zh-" | ||
| ) | ||
| val subtitleSimplified4 = getQuickSubtitle( | ||
| originalName = "Chinese (simplified)2", | ||
| languageCode = "zh-hans" | ||
| ) | ||
|
|
||
| assertEquals("zh", subtitle1.getIETF_tag()) | ||
| assertEquals("zh", subtitle2.getIETF_tag()) | ||
| assertEquals("zh-hans", subtitleSimplified1.getIETF_tag()) | ||
| assertEquals("zh-hans", subtitleSimplified2.getIETF_tag()) | ||
| assertEquals("zh-hans", subtitleSimplified3.getIETF_tag()) | ||
| assertEquals("zh-hans", subtitleSimplified4.getIETF_tag()) | ||
| } | ||
|
|
||
|
|
||
| @Test | ||
| fun `returns exact language matches`() { | ||
| val subtitle = getQuickSubtitle( | ||
| originalName = "en", | ||
| languageCode = null | ||
| ) | ||
|
|
||
| assertEquals("en", subtitle.getIETF_tag()) | ||
| } | ||
| } | ||
|
|
Uh oh!
There was an error while loading. Please reload this page.
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.
Did you checked data class SubtitleFile? I feel like the same thing is done 2 times, in SubtitleFile we already try to convert lang -> ietf tag
cloudstream/library/src/commonMain/kotlin/com/lagradost/cloudstream3/MainAPI.kt
Lines 1113 to 1129 in c1a2ae8
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.
If I remember correctly,
SubtitleFileis always turned intoSubtitleDataSubtitleFile: what we get from plugins and providers
SubtitleData: the app's internal representation (that has more properties)