diff --git a/app/src/main/java/pl/lambada/songsync/data/UserSettingsController.kt b/app/src/main/java/pl/lambada/songsync/data/UserSettingsController.kt index f192f4d..be54c7a 100644 --- a/app/src/main/java/pl/lambada/songsync/data/UserSettingsController.kt +++ b/app/src/main/java/pl/lambada/songsync/data/UserSettingsController.kt @@ -140,6 +140,11 @@ class UserSettingsController(private val dataStore: DataStore) { var batchAutoTryProviders by mutableStateOf(prefs[batchAutoTryProvidersKey] ?: true) private set + // Fast mode for the single-song search: race only the top providers with a short timeout. Off by default — + // the full chain (plus the last-resort rescue) finds more, fast mode answers sooner. + var singleFastMode by mutableStateOf(prefs[singleFastModeKey] ?: false) + private set + var sortBy by mutableStateOf( SortValues.entries .find { it.name == (prefs[sortByKey] ?: SortValues.TITLE.name) }!! @@ -257,6 +262,11 @@ class UserSettingsController(private val dataStore: DataStore) { batchAutoTryProviders = to } + fun updateSingleFastMode(to: Boolean) { + dataStore.set(singleFastModeKey, to) + singleFastMode = to + } + fun updateSortOrder(to: SortOrders) { dataStore.set(sortOrderKey, to.queryName) sortOrder = to @@ -291,4 +301,5 @@ private val batchAddUnsyncedFallbackKey = booleanPreferencesKey("batch_add_unsyn private val batchCorrectMetadataKey = booleanPreferencesKey("batch_correct_metadata") private val batchSkipExistingKey = booleanPreferencesKey("batch_skip_existing") private val batchSkipNoLyricsKey = booleanPreferencesKey("batch_skip_no_lyrics") -private val batchAutoTryProvidersKey = booleanPreferencesKey("batch_auto_try_providers") \ No newline at end of file +private val batchAutoTryProvidersKey = booleanPreferencesKey("batch_auto_try_providers") +private val singleFastModeKey = booleanPreferencesKey("single_fast_mode") \ No newline at end of file diff --git a/app/src/main/java/pl/lambada/songsync/data/remote/lyrics_providers/SmartLyricsMatcher.kt b/app/src/main/java/pl/lambada/songsync/data/remote/lyrics_providers/SmartLyricsMatcher.kt index 2178f4b..9ccc034 100644 --- a/app/src/main/java/pl/lambada/songsync/data/remote/lyrics_providers/SmartLyricsMatcher.kt +++ b/app/src/main/java/pl/lambada/songsync/data/remote/lyrics_providers/SmartLyricsMatcher.kt @@ -47,7 +47,23 @@ data class MatchConfig( * this, a single unresponsive provider could burn retries × timeouts × candidates before the next provider * even got a chance. */ val providerTimeoutMs: Long = 25_000, -) +) { + companion object { + /** + * "Fast mode" for the single-song search: only the top [maxProviders] providers, one retry, a short + * per-provider budget and minimal politeness delay. Trades a little reliability for speed — the regular + * config stays the default for batch runs. + */ + fun fast(providerOrder: List, maxProviders: Int = 2) = MatchConfig( + providerOrder = providerOrder.take(maxProviders), + maxRetries = 1, + requestDelayMs = 50, + maxCandidatesPerProvider = 3, + retryBaseDelayMs = 250, + providerTimeoutMs = 8_000, + ) + } +} /** * The brain of MusicResync's batch fetch. For one local track it walks the candidate ladder (best-guess query @@ -203,7 +219,7 @@ class SmartLyricsMatcher( .filter { !it.plainLyrics.isNullOrBlank() } .map { r -> val pr = ProviderResult(r.trackName, r.artistName, r.duration, r.albumName, false) - Triple(pr, scoreFor(local, pr, cand.strategy), r.plainLyrics!!) + Triple(pr, scoreFor(local, pr, cand), r.plainLyrics!!) } .sortedByDescending { it.second.score } .firstOrNull { it.second.tier != MatchTier.REJECT } @@ -239,6 +255,12 @@ class SmartLyricsMatcher( candidates: List, config: MatchConfig = MatchConfig(), log: (String) -> Unit = { Log.i(TAG, it) }, + /** + * Optional album-art comparer: given a candidate's cover URL, returns a bonus in + * [0, RescueCandidate.MAX_COVER_BONUS] when it visually matches the local file's embedded art + * (0 when it differs or can't be compared). Additive-only — see [RescueCandidate.coverBonus]. + */ + coverBonus: (suspend (coverUrl: String) -> Double)? = null, ): LastResortHit? { // Try several reasonable candidate queries, not just the first — a single garbled query often // canonicalizes badly, while the loosened/filename variants land the right canonical name. @@ -287,11 +309,23 @@ class SmartLyricsMatcher( }.flatMap { it.await() } } + // Optional thumbnail check: compare each distinct rescued cover against the local file's embedded art + // once, then stamp the (additive-only) bonus onto the candidates that carry that cover. + val withBonus = if (coverBonus == null) rescue else { + val bonusByUrl = rescue.mapNotNull { it.coverUrl }.distinct().associateWith { url -> + runCatching { coverBonus(url) }.getOrDefault(0.0) + } + rescue.map { c -> + val b = c.coverUrl?.let { bonusByUrl[it] } ?: 0.0 + if (b > 0.0) c.copy(coverBonus = b) else c + } + } + val localViews = buildList { add(local) candidates.forEach { c -> add(LocalTrack(c.title, c.artist, local.durationSec, local.album)) } }.distinct() - val hit = selectBestRescue(localViews, rescue) + val hit = selectBestRescue(localViews, withBonus) if (hit == null) log(" [last-resort] no believable rescue cleared the confidence bar (${rescue.size} candidates)") else log(" [last-resort] ${if (hit.synced) "synced" else "plain"} rescue -> ${hit.artist} - ${hit.title}") return hit @@ -341,7 +375,7 @@ class SmartLyricsMatcher( album = local.album, hasSyncedLyrics = true, ) - var conf = scoreFor(local, pr, cand.strategy) + var conf = scoreFor(local, pr, cand) if (durationSec == null && conf.tier == MatchTier.AUTO_ACCEPT) { conf = conf.copy(score = 0.80, tier = MatchTier.REVIEW) } @@ -373,7 +407,7 @@ class SmartLyricsMatcher( }.getOrNull() ?: break val pr = ProviderResult(info.songName, info.artistName, null, null, true) - val conf = scoreFor(local, pr, cand.strategy) + val conf = scoreFor(local, pr, cand) val lyrics = if (conf.score >= ConfidenceScorer.REVIEW_THRESHOLD) { runCatching { withRetry(config.maxRetries, config.retryBaseDelayMs) { service.getSyncedLyrics(info, provider) } }.getOrNull() } else null @@ -388,17 +422,23 @@ class SmartLyricsMatcher( } /** - * Scores a hit, then caps remix/version base-song fallbacks at REVIEW. When the only match we could find is - * the BASE song for a remix (via the loosened candidate), its lyrics are usually right but the *timing* - * differs (a remix changes tempo), so we never silently auto-accept it -- the user verifies and nudges the - * offset in the player instead. + * Scores a hit against BOTH the raw local tags and the candidate's own parse of the messy tags/filename, + * keeping the better view. The raw tags are often garbage exactly when the filename parse is right (title + * "Ariana Grande - Focus" with artist "Unknown": the raw view sees a 30%-similar title and a disagreeing + * artist and rejects the correct hit, while the parsed view — title "Focus", artist "Ariana Grande" — + * matches it perfectly). Single-song search rescued these via the slow last-resort path; scoring the parsed + * view makes the primary providers accept them directly, in batch too. + * + * Precision guard: a view without an artist (title-only candidate) cannot veto a wrong singer, so it only + * counts when the runtime matches exactly — otherwise an identically-titled song by a different artist + * would score 1.0 on title alone. + * + * Remix/version base-song fallbacks (the loosened candidate) stay capped at REVIEW: their lyrics are + * usually right but the *timing* differs (a remix changes tempo), so we never silently auto-accept one -- + * the user verifies and nudges the offset in the player instead. */ - private fun scoreFor(local: LocalTrack, pr: ProviderResult, strategy: MatchStrategy): ConfidenceBreakdown { - val conf = ConfidenceScorer.score(local, pr) - return if (strategy == MatchStrategy.FILENAME_LOOSE && conf.tier == MatchTier.AUTO_ACCEPT) - conf.copy(score = 0.80, tier = MatchTier.REVIEW) - else conf - } + private fun scoreFor(local: LocalTrack, pr: ProviderResult, cand: QueryCandidate): ConfidenceBreakdown = + scoreHitAgainstViews(local, pr, cand) private suspend fun searchLrcLib( query: String, local: LocalTrack, cand: QueryCandidate, config: MatchConfig, log: (String) -> Unit @@ -416,7 +456,7 @@ class SmartLyricsMatcher( .filter { !it.syncedLyrics.isNullOrBlank() } // batch needs synced lyrics .map { r -> val pr = ProviderResult(r.trackName, r.artistName, r.duration, r.albumName, true) - ScoredHit(Providers.LRCLIB, cand.strategy, pr, scoreFor(local, pr, cand.strategy), r.syncedLyrics, null) + ScoredHit(Providers.LRCLIB, cand.strategy, pr, scoreFor(local, pr, cand), r.syncedLyrics, null) } .sortedByDescending { it.confidence.score } .take(config.maxCandidatesPerProvider) @@ -439,18 +479,46 @@ class SmartLyricsMatcher( album = s.album?.name, hasSyncedLyrics = true, ) - ScoredHit(Providers.NETEASE, cand.strategy, pr, scoreFor(local, pr, cand.strategy), null, s.id) + ScoredHit(Providers.NETEASE, cand.strategy, pr, scoreFor(local, pr, cand), null, s.id) } } } +/** + * Scores [pr] against BOTH the raw local tags and [cand]'s own parse of the messy tags/filename, keeping the + * better view (see the call-site docs on SmartLyricsMatcher.scoreFor). A view without an artist can't veto a + * wrong singer, so it only counts when the runtime matches exactly. Pure and Android-free — unit-testable. + */ +internal fun scoreHitAgainstViews(local: LocalTrack, pr: ProviderResult, cand: QueryCandidate): ConfidenceBreakdown { + var best = ConfidenceScorer.score(local, pr) + val view = LocalTrack(cand.title, cand.artist, local.durationSec, local.album) + if (view.title != local.title || view.artist != local.artist) { + val viewConf = ConfidenceScorer.score(view, pr) + val viewTrustable = !cand.artist.isNullOrBlank() || viewConf.durationMatched + if (viewTrustable && viewConf.score > best.score) best = viewConf + } + return if (cand.strategy == MatchStrategy.FILENAME_LOOSE && best.tier == MatchTier.AUTO_ACCEPT) + best.copy(score = 0.80, tier = MatchTier.REVIEW) + else best +} + /** A raw rescued lyrics candidate (a canonicalized provider result + its lyrics) awaiting confidence validation. */ data class RescueCandidate( val result: ProviderResult, val syncedLyrics: String?, val plainLyrics: String?, val coverUrl: String?, -) + /** + * Extra confidence in [0, MAX_COVER_BONUS] earned by the candidate's album art visually matching the local + * file's embedded art. Strictly additive: art that differs (or can't be compared) contributes 0 and never + * subtracts — covers legitimately vary between releases of the same song. + */ + val coverBonus: Double = 0.0, +) { + companion object { + const val MAX_COVER_BONUS = 0.05 + } +} /** * Validates and ranks last-resort [candidates] against several views of the local track (the raw tags plus the @@ -472,7 +540,7 @@ fun selectBestRescue(localViews: List, candidates: List 0 } val localHasDuration = durationView?.durationSec != null - data class Scored(val c: RescueCandidate, val synced: Boolean, val conf: ConfidenceBreakdown) + data class Scored(val c: RescueCandidate, val synced: Boolean, val conf: ConfidenceBreakdown, val ranking: Double) val scored = candidates.mapNotNull { c -> val synced = !c.syncedLyrics.isNullOrBlank() @@ -492,11 +560,14 @@ fun selectBestRescue(localViews: List, candidates: List { it.synced }.thenByDescending { it.conf.score } + compareByDescending { it.synced }.thenByDescending { it.ranking } ).firstOrNull() ?: return null val body = if (best.synced) best.c.syncedLyrics!! else best.c.plainLyrics!! diff --git a/app/src/main/java/pl/lambada/songsync/ui/screens/batch/BatchProgressScreen.kt b/app/src/main/java/pl/lambada/songsync/ui/screens/batch/BatchProgressScreen.kt index 0dcdc6b..52e65b4 100644 --- a/app/src/main/java/pl/lambada/songsync/ui/screens/batch/BatchProgressScreen.kt +++ b/app/src/main/java/pl/lambada/songsync/ui/screens/batch/BatchProgressScreen.kt @@ -725,9 +725,14 @@ private fun resultLabel(state: LyricState): String = stringResource(state.label) @OptIn(ExperimentalMaterial3Api::class) @Composable private fun LyricsPreviewSheet(song: Song, onDismiss: () -> Unit) { + // null = still loading, "" = looked everywhere and found nothing. The old code left the value null when no + // .lrc existed (e.g. embed-only batch runs), so the sheet showed an infinite spinner (issue #11) — now it + // also reads lyrics embedded in the audio tags and always resolves to a terminal state. val lyrics by produceState(initialValue = null, song.filePath) { value = withContext(Dispatchers.IO) { runCatching { song.filePath?.toLrcFile()?.takeIf { it.exists() }?.readText() }.getOrNull() + ?: song.filePath?.let { runCatching { pl.lambada.songsync.util.EmbeddedLyrics.read(it) }.getOrNull() } + ?: "" } } ModalBottomSheet(onDismissRequest = onDismiss) { @@ -752,7 +757,7 @@ private fun LyricsPreviewSheet(song: Song, onDismiss: () -> Unit) { CircularProgressIndicator() } else -> Text( - text = text.ifBlank { stringResource(R.string.no_lyrics_only) }, + text = text.ifBlank { stringResource(R.string.no_lyrics_in_file) }, style = MaterialTheme.typography.bodyMedium, ) } diff --git a/app/src/main/java/pl/lambada/songsync/ui/screens/home/HomeScreen.kt b/app/src/main/java/pl/lambada/songsync/ui/screens/home/HomeScreen.kt index 0566955..ab0b266 100644 --- a/app/src/main/java/pl/lambada/songsync/ui/screens/home/HomeScreen.kt +++ b/app/src/main/java/pl/lambada/songsync/ui/screens/home/HomeScreen.kt @@ -122,10 +122,8 @@ fun HomeScreen( scrollBehavior = scrollBehavior, onSelectedClearAction = viewModel.selectedSongs::clear, onNavigateToSettingsSectionRequest = { navController.navigate(ScreenSettings) }, - onProviderSelectRequest = viewModel.userSettingsController::updateSelectedProviders, // The batch setup is a full page now (like the progress view), not a popup. onBatchDownloadRequest = { navController.navigate(pl.lambada.songsync.ui.ScreenBatchOptions) }, - selectedProvider = viewModel.userSettingsController.selectedProvider, onSelectAllSongsRequest = viewModel::selectAllDisplayingSongs, onInvertSongSelectionRequest = viewModel::invertSongSelection, cachedSize = cachedSize diff --git a/app/src/main/java/pl/lambada/songsync/ui/screens/home/components/HomeAppBar.kt b/app/src/main/java/pl/lambada/songsync/ui/screens/home/components/HomeAppBar.kt index 21ae2a8..a004ef4 100644 --- a/app/src/main/java/pl/lambada/songsync/ui/screens/home/components/HomeAppBar.kt +++ b/app/src/main/java/pl/lambada/songsync/ui/screens/home/components/HomeAppBar.kt @@ -26,17 +26,14 @@ import androidx.compose.ui.Modifier import androidx.compose.ui.res.stringResource import androidx.compose.ui.unit.dp import pl.lambada.songsync.R -import pl.lambada.songsync.util.Providers @OptIn(ExperimentalMaterial3Api::class) @Composable fun HomeAppBar( showing: Boolean, scrollBehavior: TopAppBarScrollBehavior, - selectedProvider: Providers, onSelectedClearAction: () -> Unit, onNavigateToSettingsSectionRequest: () -> Unit, - onProviderSelectRequest: (Providers) -> Unit, onBatchDownloadRequest: () -> Unit, onSelectAllSongsRequest: () -> Unit, onInvertSongSelectionRequest: () -> Unit, @@ -119,9 +116,6 @@ fun HomeAppBar( } else { HomeTopAppBarDropDown( onNavigateToSettingsSectionRequest = onNavigateToSettingsSectionRequest, - selectedProvider = selectedProvider, - onProviderSelectRequest = onProviderSelectRequest, - onBatchDownloadRequest = onBatchDownloadRequest, ) } }, diff --git a/app/src/main/java/pl/lambada/songsync/ui/screens/home/components/HomeTopAppBarDropDown.kt b/app/src/main/java/pl/lambada/songsync/ui/screens/home/components/HomeTopAppBarDropDown.kt index c29b197..ea13a03 100644 --- a/app/src/main/java/pl/lambada/songsync/ui/screens/home/components/HomeTopAppBarDropDown.kt +++ b/app/src/main/java/pl/lambada/songsync/ui/screens/home/components/HomeTopAppBarDropDown.kt @@ -1,12 +1,8 @@ package pl.lambada.songsync.ui.screens.home.components -import androidx.compose.animation.AnimatedContent -import androidx.compose.animation.AnimatedContentTransitionScope -import androidx.compose.animation.ContentTransform import androidx.compose.foundation.layout.Column import androidx.compose.foundation.layout.padding import androidx.compose.material.icons.Icons -import androidx.compose.material.icons.automirrored.filled.ArrowRight import androidx.compose.material.icons.filled.MoreVert import androidx.compose.material3.DropdownMenuItem import androidx.compose.material3.Icon @@ -21,28 +17,20 @@ import androidx.compose.ui.Modifier import androidx.compose.ui.res.stringResource import androidx.compose.ui.unit.dp import pl.lambada.songsync.R -import pl.lambada.songsync.ui.components.ProvidersDropdownMenuContent import pl.lambada.songsync.ui.components.dropdown.AnimatedDropdownMenu -import pl.lambada.songsync.util.Providers -import pl.lambada.songsync.util.ui.tweenEnter -import pl.lambada.songsync.util.ui.tweenExit +/** + * The home screen's overflow menu. Only Settings lives here now (issue #12): batch download has its own + * always-visible button at the bottom of the list, and the lyrics-provider order is configured in Settings — + * duplicating both in this menu was redundant. + */ @Composable fun HomeTopAppBarDropDown( onNavigateToSettingsSectionRequest: () -> Unit, - selectedProvider: Providers, - onProviderSelectRequest: (Providers) -> Unit, - onBatchDownloadRequest: () -> Unit ) { var expanded by remember { mutableStateOf(false) } - var expandedProviders by remember { mutableStateOf(false) } - IconButton( - onClick = { - expandedProviders = false - expanded = true - } - ) { + IconButton(onClick = { expanded = true }) { Icon( imageVector = Icons.Default.MoreVert, contentDescription = "More" @@ -52,81 +40,19 @@ fun HomeTopAppBarDropDown( expanded = expanded, onDismissRequest = { expanded = false } ) { - AnimatedContent( - targetState = expandedProviders, - label = "", - transitionSpec = { - ContentTransform( - targetContentEnter = slideIntoContainer( - towards = AnimatedContentTransitionScope.SlideDirection.Start, - animationSpec = tweenEnter() - ), - initialContentExit = slideOutOfContainer( - towards = AnimatedContentTransitionScope.SlideDirection.Start, - animationSpec = tweenExit() - ), - ) - }, - ) { - if(it) - ProvidersDropdownMenuContent( - onDismissRequest = { expanded = false }, - selectedProvider = selectedProvider, - onProviderSelectRequest = onProviderSelectRequest - ) - else - HomeDropdownContent( - onExpandProviders = { expandedProviders = true }, - onBatchDownloadRequest = { - expanded = false - onBatchDownloadRequest() - }, - onNavigateToSettingsSectionRequest = { - expanded = false - onNavigateToSettingsSectionRequest() - }, - ) + Column { + DropdownMenuItem( + text = { + Text( + text = stringResource(id = R.string.settings), + modifier = Modifier.padding(horizontal = 6.dp), + ) + }, + onClick = { + expanded = false + onNavigateToSettingsSectionRequest() + } + ) } } } - -@Composable -fun HomeDropdownContent( - onExpandProviders: () -> Unit, - onBatchDownloadRequest: () -> Unit, - onNavigateToSettingsSectionRequest: () -> Unit, -) = Column { - DropdownMenuItem( - text = { - Text( - text = stringResource(R.string.provider), - modifier = Modifier.padding(horizontal = 6.dp), - ) - }, - trailingIcon = { - Icon( - imageVector = Icons.AutoMirrored.Filled.ArrowRight, - contentDescription = "expand dropdown" - ) - }, - onClick = onExpandProviders - ) - DropdownMenuItem( - text = { - Text( - text = stringResource(R.string.batch_download_lyrics), - modifier = Modifier.padding(horizontal = 6.dp), - ) - }, - onClick = onBatchDownloadRequest - ) - DropdownMenuItem( - text = { - Text( - text = stringResource(id = R.string.settings), - modifier = Modifier.padding(horizontal = 6.dp), - ) - }, - onClick = onNavigateToSettingsSectionRequest - ) -} \ No newline at end of file diff --git a/app/src/main/java/pl/lambada/songsync/ui/screens/lyricsFetch/LyricsFetchViewModel.kt b/app/src/main/java/pl/lambada/songsync/ui/screens/lyricsFetch/LyricsFetchViewModel.kt index efff1ef..d9e92a0 100644 --- a/app/src/main/java/pl/lambada/songsync/ui/screens/lyricsFetch/LyricsFetchViewModel.kt +++ b/app/src/main/java/pl/lambada/songsync/ui/screens/lyricsFetch/LyricsFetchViewModel.kt @@ -28,7 +28,9 @@ import pl.lambada.songsync.util.matching.LyricState import pl.lambada.songsync.util.matching.MatchStrategy import pl.lambada.songsync.util.matching.MatchTier import pl.lambada.songsync.util.matching.QueryCandidate +import pl.lambada.songsync.util.matching.TextMatch import com.kyant.taglib.TagLib +import pl.lambada.songsync.util.CoverArtCompare import pl.lambada.songsync.util.embedCoverInFile import pl.lambada.songsync.util.embedLyricsInFile import pl.lambada.songsync.util.ext.getVersion @@ -117,7 +119,11 @@ class LyricsFetchViewModel( // what makes a junk tag like "Kendrick Lamar - HUMBLE. / KendrickLamarVEVO" actually resolve, // and stops a wrong "first result" (e.g. a random remix) from being accepted. val durationSec = source?.filePath?.let { readDurationSeconds(context, it) } - val local = LocalTrack(querySongName, queryArtistName, durationSec) + val local = LocalTrack( + querySongName, + queryArtistName.takeIf { !TextMatch.isJunkArtist(it) }, + durationSec, + ) val candidates = FilenameParser.candidates(querySongName, queryArtistName, source?.filePath) .ifEmpty { listOf(QueryCandidate(querySongName, queryArtistName ?: "", MatchStrategy.TAGS)) } @@ -127,12 +133,16 @@ class LyricsFetchViewModel( val order = (listOf(userSettingsController.selectedProvider) + userSettingsController.enabledProviderOrder).distinct() + // Fast mode (Settings): race only the top providers with a short budget instead of the full chain. + val fastMode = userSettingsController.singleFastMode + val config = if (fastMode) MatchConfig.fast(order) else MatchConfig(providerOrder = order) + // Track which providers the search actually reached. All providers now run in parallel, but the // auto-accept early stop can still cancel slower ones mid-flight — those are genuinely UNTRIED, // not failures, and must not be painted with a red X. val attempted = linkedSetOf() val hits = matcher.search( - local, candidates, MatchConfig(providerOrder = order), + local, candidates, config, onAttempt = { provider -> attempted.add(provider); providerProbes[provider] = ProviderProbe.LOADING }, onSkipped = { provider -> attempted.remove(provider) }, ) @@ -148,12 +158,16 @@ class LyricsFetchViewModel( if (!l.isNullOrBlank()) { chosen = hit; lyrics = l; break } } - // Record per-provider probe outcomes for the cloud-icon dropdown: the winner is HAS_SYNCED, - // providers we actually reached but that had nothing are NONE, and everything past the early stop - // stays UNTRIED instead of being mislabelled as failed. + // Providers that produced a believable synced hit — even non-winning ones "found lyrics"; only + // painting them with a red X (as before) claimed they had nothing when they were simply outranked. + val providersWithSynced = ranked.filter { it.result.hasSyncedLyrics }.map { it.provider }.toSet() + + // Record per-provider probe outcomes for the cloud-icon dropdown: whoever had a believable synced + // hit is HAS_SYNCED, providers we actually reached but that had nothing are NONE, and everything + // past the early stop stays UNTRIED instead of being mislabelled as failed. order.forEach { p -> providerProbes[p] = when { - p == chosen?.provider -> ProviderProbe.HAS_SYNCED + p == chosen?.provider || p in providersWithSynced -> ProviderProbe.HAS_SYNCED p in attempted -> ProviderProbe.NONE else -> ProviderProbe.UNTRIED } @@ -161,7 +175,10 @@ class LyricsFetchViewModel( if (chosen != null && lyrics != null) { activeProvider = chosen.provider - rememberProviders(chosen.provider, order.filter { it != chosen.provider }) + rememberProviders( + chosen.provider, + attempted.filter { it != chosen.provider && it !in providersWithSynced }, + ) queryState = QueryStatus.Success( SongInfo(songName = chosen.result.title, artistName = chosen.result.artist) ) @@ -171,20 +188,27 @@ class LyricsFetchViewModel( // Last resort: canonicalize a messy filename via iTunes/Deezer and retry LRCLib under the clean // name. This path is only allowed to succeed when the rescue still clears the scorer-based gates; - // otherwise we fail honestly instead of silently substituting a different song. - val lr = runCatching { matcher.lastResort(local, candidates) }.getOrNull() + // otherwise we fail honestly instead of silently substituting a different song. A matching local + // album thumbnail adds (never subtracts) confidence to a rescue candidate. + val lr = runCatching { + matcher.lastResort( + local, candidates, config, + coverBonus = CoverArtCompare.bonusFor(context, cachePath), + ) + }.getOrNull() if (lr != null && lr.synced) { activeProvider = Providers.LRCLIB providerProbes[Providers.LRCLIB] = ProviderProbe.HAS_SYNCED - rememberProviders(Providers.LRCLIB, order.filter { it != Providers.LRCLIB }) + rememberProviders(Providers.LRCLIB, attempted.filter { it != Providers.LRCLIB && it !in providersWithSynced }) queryState = QueryStatus.Success(SongInfo(songName = lr.title, artistName = lr.artist, albumCoverLink = lr.coverUrl)) lyricsFetchState = LyricsFetchState.Success(lr.lyrics) return@launch } - // No synced lyrics anywhere -> offer plain (LRCLib first, then the last-resort's plain body) instead of erroring. - rememberProviders(null, order) - val plain = runCatching { matcher.fetchPlainLyrics(local, candidates) }.getOrNull() + // No synced lyrics anywhere -> offer plain (LRCLib first, then the last-resort's plain body) + // instead of erroring. Only providers we actually reached are remembered as failed. + rememberProviders(null, attempted.toList()) + val plain = runCatching { matcher.fetchPlainLyrics(local, candidates, config) }.getOrNull() val plainLyrics = plain?.plainLyrics ?: lr?.takeUnless { it.synced }?.lyrics queryState = QueryStatus.SyncedNotFound( song = SongInfo( diff --git a/app/src/main/java/pl/lambada/songsync/ui/screens/lyricsFetch/components/LyricsSuccessContent.kt b/app/src/main/java/pl/lambada/songsync/ui/screens/lyricsFetch/components/LyricsSuccessContent.kt index c9dcc19..c123903 100644 --- a/app/src/main/java/pl/lambada/songsync/ui/screens/lyricsFetch/components/LyricsSuccessContent.kt +++ b/app/src/main/java/pl/lambada/songsync/ui/screens/lyricsFetch/components/LyricsSuccessContent.kt @@ -1,6 +1,17 @@ package pl.lambada.songsync.ui.screens.lyricsFetch.components +import androidx.compose.animation.AnimatedContent import androidx.compose.animation.animateColorAsState +import androidx.compose.animation.core.Animatable +import androidx.compose.animation.core.Spring +import androidx.compose.animation.core.animateFloatAsState +import androidx.compose.animation.core.spring +import androidx.compose.animation.core.tween +import androidx.compose.animation.fadeIn +import androidx.compose.animation.fadeOut +import androidx.compose.animation.slideInVertically +import androidx.compose.animation.slideOutVertically +import androidx.compose.animation.togetherWith import androidx.compose.foundation.layout.Arrangement import androidx.compose.foundation.layout.Column import androidx.compose.foundation.layout.Row @@ -24,6 +35,7 @@ import androidx.compose.material3.OutlinedButton import androidx.compose.material3.OutlinedCard import androidx.compose.material3.Text import androidx.compose.runtime.Composable +import androidx.compose.runtime.LaunchedEffect import androidx.compose.runtime.getValue import androidx.compose.runtime.mutableStateOf import androidx.compose.runtime.remember @@ -31,6 +43,7 @@ import androidx.compose.runtime.setValue import androidx.compose.ui.Alignment import androidx.compose.ui.Modifier import androidx.compose.ui.graphics.Color +import androidx.compose.ui.graphics.graphicsLayer import androidx.compose.ui.res.stringResource import androidx.compose.ui.unit.dp import pl.lambada.songsync.R @@ -132,6 +145,11 @@ fun LyricsSuccessContent( } } +/** + * Save/Embed action with animated success feedback: on save the button springs (a quick overshoot pulse), + * morphs to green, and the checkmark pops in while the label crossfades+slides to the "saved" text — a clear, + * satisfying confirmation instead of an instant text swap. + */ @Composable private fun SaveButton( saved: Boolean, @@ -142,17 +160,51 @@ private fun SaveButton( ) { val container by animateColorAsState( if (saved) Color(0xFF2E7D32) else MaterialTheme.colorScheme.primary, - label = "saveBtn", + animationSpec = tween(350), + label = "saveBtnColor", + ) + // Whole-button pulse: a springy overshoot on the transition into "saved". + val pulse by animateFloatAsState( + targetValue = if (saved) 1f else 0f, + animationSpec = spring(dampingRatio = Spring.DampingRatioMediumBouncy, stiffness = Spring.StiffnessLow), + label = "saveBtnPulse", ) + val buttonScale = 1f + 0.06f * kotlin.math.sin(pulse * Math.PI).toFloat() + Button( onClick = onClick, - modifier = modifier, + modifier = modifier.graphicsLayer { scaleX = buttonScale; scaleY = buttonScale }, colors = ButtonDefaults.buttonColors(containerColor = container), ) { - if (saved) { - Icon(Icons.Filled.Check, contentDescription = null, modifier = Modifier.size(18.dp)) - Spacer(Modifier.width(6.dp)) + AnimatedContent( + targetState = saved, + transitionSpec = { + (fadeIn(tween(220)) + slideInVertically(tween(220)) { it / 2 }) + .togetherWith(fadeOut(tween(120)) + slideOutVertically(tween(120)) { -it / 2 }) + }, + label = "saveBtnLabel", + ) { isSaved -> + Row(verticalAlignment = Alignment.CenterVertically) { + if (isSaved) { + // The check pops in with a spring once the content has switched. + val checkScale = remember { Animatable(0f) } + LaunchedEffect(Unit) { + checkScale.animateTo( + 1f, + spring(dampingRatio = Spring.DampingRatioMediumBouncy, stiffness = Spring.StiffnessMedium), + ) + } + Icon( + Icons.Filled.Check, + contentDescription = null, + modifier = Modifier + .size(18.dp) + .graphicsLayer { scaleX = checkScale.value; scaleY = checkScale.value }, + ) + Spacer(Modifier.width(6.dp)) + } + Text(if (isSaved) savedText else idleText, maxLines = 1) + } } - Text(if (saved) savedText else idleText, maxLines = 1) } } diff --git a/app/src/main/java/pl/lambada/songsync/ui/screens/player/SyncedLyricsPlayerScreen.kt b/app/src/main/java/pl/lambada/songsync/ui/screens/player/SyncedLyricsPlayerScreen.kt index 6f5ec19..89e21a2 100644 --- a/app/src/main/java/pl/lambada/songsync/ui/screens/player/SyncedLyricsPlayerScreen.kt +++ b/app/src/main/java/pl/lambada/songsync/ui/screens/player/SyncedLyricsPlayerScreen.kt @@ -30,6 +30,7 @@ import androidx.compose.material.icons.filled.Replay import androidx.compose.material.icons.filled.Search import androidx.compose.material3.Button import androidx.compose.material3.ButtonDefaults +import androidx.compose.material3.CircularProgressIndicator import androidx.compose.material3.DropdownMenu import androidx.compose.material3.DropdownMenuItem import androidx.compose.material3.ExperimentalMaterial3Api @@ -73,6 +74,7 @@ import kotlinx.coroutines.launch import kotlinx.coroutines.withContext import pl.lambada.songsync.R import pl.lambada.songsync.ui.screens.lyricsFetch.components.ThumbnailPickerDialog +import pl.lambada.songsync.util.EmbeddedLyrics import pl.lambada.songsync.util.applyOffsetToLyrics import pl.lambada.songsync.util.buildNeutralLrc import pl.lambada.songsync.util.embedCoverInFile @@ -143,22 +145,31 @@ fun SyncedLyricsPlayerScreen( var menuExpanded by remember { mutableStateOf(false) } var showThumbnailPicker by remember { mutableStateOf(false) } var hasCover by remember(filePath) { mutableStateOf(null) } - var lrcText by remember(filePath) { - mutableStateOf( - initialLyrics?.takeIf { it.isNotBlank() } ?: runCatching { - File(filePath.substringBeforeLast('.') + ".lrc").takeIf { it.exists() }?.readText() - }.getOrNull().orEmpty() - ) + // Null while still loading from disk. The sidecar .lrc is tried first, then lyrics EMBEDDED in the audio + // tags (issue #11: embedded-only songs opened this player and it said "no lyrics" because only the sidecar + // was ever read). The TagLib read is real I/O, so it happens off the main thread with a loading state. + var lrcText by remember(filePath) { mutableStateOf(initialLyrics?.takeIf { it.isNotBlank() }) } + LaunchedEffect(filePath) { + if (lrcText == null) { + lrcText = withContext(Dispatchers.IO) { + runCatching { + File(filePath.substringBeforeLast('.') + ".lrc").takeIf { it.exists() }?.readText() + }.getOrNull() + ?: runCatching { EmbeddedLyrics.read(filePath) }.getOrNull() + ?: "" + } + } } + val lyricsLoaded = lrcText != null // Reconcile the two timing models into one neutral form (zero applied offset, no [offset:] tag) so all the // preview/seek/save math can treat the user's offset as a single absolute value. A file in tag mode carries // [offset:N]; a direct-shifted file has the offset baked into its timestamps and we undo it here. - val fileOffsetMs = remember(lrcText) { parseOffsetTagMs(lrcText) } + val fileOffsetMs = remember(lrcText) { parseOffsetTagMs(lrcText.orEmpty()) } val cachedOffsetMs = remember(filePath) { SongCache.get(filePath)?.offsetMs ?: 0 } val fileUsesOffsetTag = fileOffsetMs != 0 val baseAppliedOffsetMs = if (fileUsesOffsetTag) fileOffsetMs else cachedOffsetMs val neutralLrc = remember(lrcText, baseAppliedOffsetMs, fileUsesOffsetTag) { - buildNeutralLrc(lrcText, baseAppliedOffsetMs, fileUsesOffsetTag) + buildNeutralLrc(lrcText.orEmpty(), baseAppliedOffsetMs, fileUsesOffsetTag) } val lines = remember(neutralLrc) { parseSyncedLrc(neutralLrc) } @@ -400,7 +411,9 @@ fun SyncedLyricsPlayerScreen( Column(modifier = Modifier.fillMaxSize().padding(padding)) { Box(modifier = Modifier.weight(1f).fillMaxWidth()) { - if (lines.isEmpty()) { + if (!lyricsLoaded) { + CircularProgressIndicator(modifier = Modifier.align(Alignment.Center)) + } else if (lines.isEmpty()) { Text( "No synced lyrics found for this song.", modifier = Modifier.align(Alignment.Center).padding(24.dp), diff --git a/app/src/main/java/pl/lambada/songsync/ui/screens/settings/SettingsScreen.kt b/app/src/main/java/pl/lambada/songsync/ui/screens/settings/SettingsScreen.kt index 35362c8..ad1ad29 100644 --- a/app/src/main/java/pl/lambada/songsync/ui/screens/settings/SettingsScreen.kt +++ b/app/src/main/java/pl/lambada/songsync/ui/screens/settings/SettingsScreen.kt @@ -26,6 +26,7 @@ import pl.lambada.songsync.ui.screens.settings.components.AppInfoSection import pl.lambada.songsync.ui.screens.settings.components.ContributorsSection import pl.lambada.songsync.ui.screens.settings.components.CreditsSection import pl.lambada.songsync.ui.screens.settings.components.ExternalLinkSection +import pl.lambada.songsync.ui.screens.settings.components.FastModeSwitch import pl.lambada.songsync.ui.screens.settings.components.MarqueeSwitch import pl.lambada.songsync.ui.screens.settings.components.MultiPersonSwitch import pl.lambada.songsync.ui.screens.settings.components.ProviderOrderSection @@ -104,6 +105,12 @@ fun SettingsScreen( onToggle = { userSettingsController.updateDirectlyModifyTimestamps(it) } ) } + item { + FastModeSwitch( + selected = userSettingsController.singleFastMode, + onToggle = { userSettingsController.updateSingleFastMode(it) } + ) + } item { SpotifySecretsStatus() } item { SettingsHeadLabel(label = stringResource(id = R.string.provider_order)) } diff --git a/app/src/main/java/pl/lambada/songsync/ui/screens/settings/components/FastModeSwitch.kt b/app/src/main/java/pl/lambada/songsync/ui/screens/settings/components/FastModeSwitch.kt new file mode 100644 index 0000000..6fdad02 --- /dev/null +++ b/app/src/main/java/pl/lambada/songsync/ui/screens/settings/components/FastModeSwitch.kt @@ -0,0 +1,17 @@ +package pl.lambada.songsync.ui.screens.settings.components + +import androidx.compose.runtime.Composable +import androidx.compose.ui.res.stringResource +import pl.lambada.songsync.R +import pl.lambada.songsync.ui.components.SwitchItem + + +@Composable +fun FastModeSwitch(selected: Boolean, onToggle: (Boolean) -> Unit) { + SwitchItem( + label = stringResource(R.string.fast_mode), + description = stringResource(R.string.fast_mode_desc), + selected = selected, + onClick = { onToggle(!selected) } + ) +} diff --git a/app/src/main/java/pl/lambada/songsync/util/CoverArtCompare.kt b/app/src/main/java/pl/lambada/songsync/util/CoverArtCompare.kt new file mode 100644 index 0000000..344bf0b --- /dev/null +++ b/app/src/main/java/pl/lambada/songsync/util/CoverArtCompare.kt @@ -0,0 +1,91 @@ +package pl.lambada.songsync.util + +import android.content.Context +import android.graphics.Bitmap +import android.graphics.BitmapFactory +import com.kyant.taglib.TagLib +import pl.lambada.songsync.data.remote.lyrics_providers.RescueCandidate +import java.net.HttpURLConnection +import java.net.URL + +/** + * Perceptual album-art comparison for the matcher's thumbnail bonus. A provider cover that looks like the + * local file's embedded art is extra evidence the rescue found the right song, so it ADDS confidence; covers + * that differ prove nothing (singles, re-releases and rips use wildly different art), so a mismatch never + * subtracts. Comparison is a classic 8x8 average hash: cheap, resolution-independent and robust to + * re-encoding, which is all this needs. + */ +object CoverArtCompare { + + /** Hamming distance (of 64 bits) at or below which two covers count as "the same picture". */ + private const val MATCH_MAX_DISTANCE = 10 + + /** 64-bit average hash of a bitmap: downscale to 8x8, gray, threshold on the mean. */ + fun aHash64(bitmap: Bitmap): Long { + val small = Bitmap.createScaledBitmap(bitmap, 8, 8, true) + val gray = IntArray(64) + var sum = 0L + for (y in 0 until 8) for (x in 0 until 8) { + val p = small.getPixel(x, y) + val g = ((p shr 16 and 0xFF) * 299 + (p shr 8 and 0xFF) * 587 + (p and 0xFF) * 114) / 1000 + gray[y * 8 + x] = g + sum += g + } + if (small !== bitmap) small.recycle() + val avg = sum / 64 + var hash = 0L + for (i in 0 until 64) if (gray[i] > avg) hash = hash or (1L shl i) + return hash + } + + fun hammingDistance(a: Long, b: Long): Int = java.lang.Long.bitCount(a xor b) + + /** The embedded front cover's hash for [filePath], or null when the file has no art / can't be read. */ + fun localCoverHash(context: Context, filePath: String): Long? = runCatching { + val fd = getFileDescriptorFromPath(context, filePath, "r") ?: return null + val picture = TagLib.getFrontCover(fd.dup().detachFd()) ?: return null + decodeSmall(picture.data)?.let { bmp -> aHash64(bmp).also { bmp.recycle() } } + }.getOrNull() + + /** Downloads [url] (bounded) and hashes it, or null on any failure. */ + fun remoteCoverHash(url: String): Long? = runCatching { + val conn = URL(url).openConnection() as HttpURLConnection + conn.connectTimeout = 4_000 + conn.readTimeout = 4_000 + try { + if (conn.responseCode !in 200..299) return null + val bytes = conn.inputStream.use { it.readBytes() } + decodeSmall(bytes)?.let { bmp -> aHash64(bmp).also { bmp.recycle() } } + } finally { + conn.disconnect() + } + }.getOrNull() + + /** + * Builds the additive-only cover bonus lambda for [SmartLyricsMatcher.lastResort]: full bonus when the + * remote cover perceptually matches the local embedded art, 0 otherwise. Returns null when the local file + * has no usable art (nothing to compare — the matcher then behaves exactly as before). The local hash is + * computed once up front, not per candidate. + */ + fun bonusFor(context: Context, filePath: String?): (suspend (String) -> Double)? { + if (filePath == null) return null + val localHash = localCoverHash(context, filePath) ?: return null + return { coverUrl -> + val remote = remoteCoverHash(coverUrl) + if (remote != null && hammingDistance(localHash, remote) <= MATCH_MAX_DISTANCE) + RescueCandidate.MAX_COVER_BONUS + else 0.0 + } + } + + /** Decodes image bytes at a small sample size (the hash only needs 8x8). */ + private fun decodeSmall(bytes: ByteArray): Bitmap? { + val bounds = BitmapFactory.Options().apply { inJustDecodeBounds = true } + BitmapFactory.decodeByteArray(bytes, 0, bytes.size, bounds) + if (bounds.outWidth <= 0 || bounds.outHeight <= 0) return null + var sample = 1 + while (bounds.outWidth / (sample * 2) >= 32 && bounds.outHeight / (sample * 2) >= 32) sample *= 2 + val opts = BitmapFactory.Options().apply { inSampleSize = sample } + return BitmapFactory.decodeByteArray(bytes, 0, bytes.size, opts) + } +} diff --git a/app/src/main/java/pl/lambada/songsync/util/LyricsUtils.kt b/app/src/main/java/pl/lambada/songsync/util/LyricsUtils.kt index 969930e..6e812cd 100644 --- a/app/src/main/java/pl/lambada/songsync/util/LyricsUtils.kt +++ b/app/src/main/java/pl/lambada/songsync/util/LyricsUtils.kt @@ -29,6 +29,7 @@ import pl.lambada.songsync.util.matching.LrcPrescan import pl.lambada.songsync.util.matching.LyricState import pl.lambada.songsync.util.matching.MatchTier import pl.lambada.songsync.util.matching.SongMatchInfo +import pl.lambada.songsync.util.matching.TextMatch import kotlinx.coroutines.ensureActive import java.io.File import java.io.FileNotFoundException @@ -425,7 +426,9 @@ suspend fun matchAndSaveSong( val local = LocalTrack( title = song.title, - artist = song.artist, + // Placeholder artists ("Unknown", …) are noise, not evidence — with one in place the scorer sees a + // "disagreeing" artist on every correct hit and rejects it. + artist = song.artist?.takeIf { !TextMatch.isJunkArtist(it) }, durationSec = song.durationMs?.let { it / 1000.0 }, album = song.album, ) @@ -446,6 +449,9 @@ suspend fun matchAndSaveSong( val topForReport = hits.firstOrNull() val nonRejected = hits.filter { it.tier != MatchTier.REJECT } + // Providers that produced at least one believable synced hit — even a non-winning one is not a "failure". + val providersWithLyrics = nonRejected.filter { it.result.hasSyncedLyrics }.map { it.provider }.toSet() + // Fall through every acceptable hit (highest confidence first) until one actually yields synced lyrics. A // provider can match the metadata yet have no synced body ("found a match but no synced lyrics"); rather // than give up we try the next hit/provider. @@ -457,10 +463,36 @@ suspend fun matchAndSaveSong( if (!l.isNullOrBlank()) { chosen = hit; lyrics = l; break } } - // No synced lyrics from the normal providers. The old last-resort lyrics rescue is intentionally disabled: - // for some messy files it still produced a different song by the same artist, and for lyrics it's better to - // fail honestly than silently save the wrong words. We only offer a plain LRCLib fallback when explicitly - // enabled by the user. + // No synced lyrics from the normal providers: run the same last-resort rescue the single-song screen uses + // (canonicalize the messy name via iTunes/Deezer, re-query LRCLib under the clean identity). This is what + // made "it finds it when I open the song, but not in batch" true — the batch never had this path. The + // rescue is validated by the scorer-based gates in selectBestRescue (plus the additive-only album-art + // bonus), and a rescued match is always saved as REVIEW, never silently auto-accepted. + var rescued: SmartLyricsMatcher.LastResortHit? = null + if (chosen == null || lyrics == null) { + coroutineContext.ensureActive() + rescued = runCatching { + matcher.lastResort(local, candidates, config, coverBonus = CoverArtCompare.bonusFor(context, song.filePath)) + }.getOrNull()?.takeIf { it.synced } + } + if (rescued != null) { + val songInfo = SongInfo(songName = rescued.title, artistName = rescued.artist) + val lrcContent = formatLyrics(songInfo, rescued.lyrics, context, settings.directlyModifyTimestamps) + val saved = runCatching { + persistLyrics(context, song, lrcFile, lrcContent, saveLrc, embedLyrics, settings.sdCardPath) + }.getOrDefault(false) + if (saved) return SongMatchInfo( + LyricState.REVIEW, + confidencePercent = topForReport?.confidence?.percent(), + provider = Providers.LRCLIB, + matchedTitle = rescued.title, + matchedArtist = rescued.artist, + failedProviders = (attempted - Providers.LRCLIB - providersWithLyrics).toList(), + ) + } + + // Still nothing. We only offer a plain LRCLib fallback when explicitly enabled by the user — for lyrics + // it's better to fail honestly than silently save the wrong words. if (chosen == null || lyrics == null) { // Probe for plain (unsynced) lyrics even when the fallback toggle is off, so the batch can report // "N unsynced available" and let the user add them all with one tap later. @@ -501,8 +533,9 @@ suspend fun matchAndSaveSong( val songInfo = SongInfo(songName = best.result.title, artistName = best.result.artist) val lrcContent = formatLyrics(songInfo, lyrics, context, settings.directlyModifyTimestamps) - // Providers that were tried before the winning one and produced nothing usable for this song. - val failedForSong = (attempted - best.provider).toList() + // Providers that were tried for this song and produced nothing usable — NOT the winner, and not the ones + // that also had a believable synced hit (they'd have worked too; painting them as failed misleads). + val failedForSong = (attempted - best.provider - providersWithLyrics).toList() // Independent choices: write a sidecar .lrc and/or embed into the file. Default saves the .lrc. Honour the // embed boolean: a failed embed (no exception) must still count as a failure when it was the only target. diff --git a/app/src/main/java/pl/lambada/songsync/util/matching/FilenameParser.kt b/app/src/main/java/pl/lambada/songsync/util/matching/FilenameParser.kt index 339379a..c5febee 100644 --- a/app/src/main/java/pl/lambada/songsync/util/matching/FilenameParser.kt +++ b/app/src/main/java/pl/lambada/songsync/util/matching/FilenameParser.kt @@ -54,17 +54,36 @@ object FilenameParser { /** Lowercased alphanumerics only — collapses "$uicideboy$"/"_UICIDEBOY_" to the single token "uicideboy". */ private fun collapse(s: String): String = s.lowercase().filter { it.isLetterOrDigit() } - /** Pull "feat./ft." artists out of a title, returning the cleaned title and the extracted names. */ + /** Splits a captured feat-clause value into individual artist names. */ + private fun splitFeaturedNames(value: String): List = value + .split(Regex("""\s*(?:,|&|x|and|\+)\s*""", RegexOption.IGNORE_CASE)) + .map { it.trim() } + .filter { it.isNotBlank() } + + /** + * Pull "feat./ft." artists out of a title, returning the cleaned title and the extracted names. Handles + * both a bracketed clause anywhere in the string ("Harli Kvin (feat. AV47) 420" — trailing junk after the + * clause used to defeat the end-anchored regex) and an unbracketed clause at the end. + */ private fun extractFeatured(title: String): Pair> { - val m = TextMatch.featRegex.find(title) ?: return title to emptyList() - val names = m.groupValues[1] - .split(Regex("""\s*(?:,|&|x|and|\+)\s*""", RegexOption.IGNORE_CASE)) - .map { it.trim() } - .filter { it.isNotBlank() } - val cleaned = title.removeRange(m.range).trim().trim('-', '|', '/').trim() + val names = mutableListOf() + var cleaned = TextMatch.parenFeatRegex.replace(title) { m -> + names += splitFeaturedNames(m.groupValues[1]); " " + } + TextMatch.featRegex.find(cleaned)?.let { m -> + names += splitFeaturedNames(m.groupValues[1]) + cleaned = cleaned.removeRange(m.range) + } + cleaned = cleaned.replace(Regex("""\s+"""), " ").trim().trim('-', '|', '/').trim() + if (names.isEmpty()) return title to emptyList() return (cleaned.ifBlank { title }) to names } + /** Trailing orphan number left behind by SnapTube-style "_420" suffixes ("Harli Kvin 420" -> "Harli Kvin"). */ + private val trailingJunkNumber = Regex("""\s+\d{2,4}$""") + + private fun stripTrailingJunkNumber(s: String): String = trailingJunkNumber.replace(s, "").trim() + private fun candidate(title: String, artist: String?, strategy: MatchStrategy): QueryCandidate? { val cleanTitle = TextMatch.cleanTitleArtist(title) if (cleanTitle.isBlank()) return null @@ -111,6 +130,13 @@ object FilenameParser { if (!loose.equals(titlePart, ignoreCase = true) && loose.isNotBlank()) add(candidate(loose, primary, MatchStrategy.FILENAME_LOOSE)) + // A trailing orphan number is usually ripper junk (an underscore suffix like "_420" turned into + // " 420" by the cleanup). Try the title without it — as a LOOSE fallback, so a genuine numeric + // title still gets its exact query first and a stripped-title match stays review-grade. + val noJunkNo = stripTrailingJunkNumber(titlePart) + if (!noJunkNo.equals(titlePart, ignoreCase = true) && noJunkNo.isNotBlank()) + add(candidate(noJunkNo, artistPart, MatchStrategy.FILENAME_LOOSE)) + // Collapsed artist (alphanumerics only, no spaces) for stylized names whose decorations get // mangled on disk: "_UICIDEBOY_" / "$uicideboy$" -> "uicideboy", "P!nk" -> "pnk", "deadmau5". // LRCLib indexes these as one token, so a spaced/decorated query misses them entirely. @@ -128,11 +154,16 @@ object FilenameParser { val loosePlain = loosenTitle(cleaned) if (!loosePlain.equals(cleaned, ignoreCase = true) && loosePlain.isNotBlank()) add(candidate(loosePlain, null, MatchStrategy.FILENAME_TITLE_ONLY)) + val noJunkPlain = stripTrailingJunkNumber(cleaned) + if (!noJunkPlain.equals(cleaned, ignoreCase = true) && noJunkPlain.isNotBlank()) + add(candidate(noJunkPlain, null, MatchStrategy.FILENAME_TITLE_ONLY)) } - // 1) Trust tags first when they look real. + // 1) Trust tags first when they look real. Placeholder artists ("Unknown", "Various Artists"…) are + // treated as absent — querying and scoring against them only hurts (a result by the real artist would + // "disagree" with the placeholder and be rejected). val title = tagTitle?.takeIf { it.isNotBlank() && it != "" } - val artist = tagArtist?.takeIf { it.isNotBlank() && it != "" } + val artist = tagArtist?.takeIf { !TextMatch.isJunkArtist(it) } if (title != null) { add(candidate(title, artist, MatchStrategy.TAGS)) // If the tag title itself is "Artist - Title", split it too. diff --git a/app/src/main/java/pl/lambada/songsync/util/matching/TextMatch.kt b/app/src/main/java/pl/lambada/songsync/util/matching/TextMatch.kt index 0eaded2..bd85b56 100644 --- a/app/src/main/java/pl/lambada/songsync/util/matching/TextMatch.kt +++ b/app/src/main/java/pl/lambada/songsync/util/matching/TextMatch.kt @@ -28,6 +28,25 @@ object TextMatch { /** "feat. X", "ft. X", "featuring X" up to a closing bracket or end. Captured group 1 = the featured names. */ val featRegex = Regex("""[\s(\[]*(?:feat\.?|ft\.?|featuring)\s+([^()\[\]]+?)\s*[)\]]?$""", RegexOption.IGNORE_CASE) + /** + * A bracketed feat clause ANYWHERE in the string — "Harli Kvin (feat. AV47) 420" keeps trailing junk after + * the clause, which the end-anchored [featRegex] can't reach. Captured group 1 = the featured names. + */ + val parenFeatRegex = Regex("""[(\[]\s*(?:feat\.?|ft\.?|featuring)\s+([^()\[\]]+?)\s*[)\]]""", RegexOption.IGNORE_CASE) + + /** + * Placeholder "artist" values MediaStore/rippers write when the real artist is unknown. Treating these as a + * genuine artist poisons both the query and the scorer (a result by the real artist "disagrees" with + * "Unknown" and gets rejected), so callers null them out before matching. + */ + private val junkArtists = setOf( + "unknown", "unknown artist", "", "various artists", "va", "n/a", "not available", "artist", + ) + + /** True when [raw] is a placeholder rather than a real artist name ("Unknown", "", …). */ + fun isJunkArtist(raw: String?): Boolean = + raw.isNullOrBlank() || raw.trim().lowercase(Locale.ROOT) in junkArtists + /** Leading track number like "07.", "05 - ", "1) ", "12_". */ private val leadingTrackNo = Regex("""^\s*\d{1,2}\s*[.)\-_]\s*""") diff --git a/app/src/main/res/values/strings.xml b/app/src/main/res/values/strings.xml index e660934..c61d64c 100644 --- a/app/src/main/res/values/strings.xml +++ b/app/src/main/res/values/strings.xml @@ -282,6 +282,13 @@ Refresh Refreshing… + + Fast matching + When searching a single song, race only the top two providers with a short timeout. Faster answers, but may find fewer songs than the full search. + + + No lyrics found in this file + Provider order Providers are tried one by one, from top to bottom, until lyrics are found. Uncheck a provider to skip it. diff --git a/app/src/test/java/pl/lambada/songsync/matching/BadMetadataMatchTest.kt b/app/src/test/java/pl/lambada/songsync/matching/BadMetadataMatchTest.kt new file mode 100644 index 0000000..fee983b --- /dev/null +++ b/app/src/test/java/pl/lambada/songsync/matching/BadMetadataMatchTest.kt @@ -0,0 +1,111 @@ +package pl.lambada.songsync.matching + +import org.junit.Assert.assertEquals +import org.junit.Assert.assertTrue +import org.junit.Test +import pl.lambada.songsync.data.remote.lyrics_providers.scoreHitAgainstViews +import pl.lambada.songsync.util.matching.FilenameParser +import pl.lambada.songsync.util.matching.LocalTrack +import pl.lambada.songsync.util.matching.MatchStrategy +import pl.lambada.songsync.util.matching.MatchTier +import pl.lambada.songsync.util.matching.ProviderResult +import pl.lambada.songsync.util.matching.TextMatch + +/** + * Regression tests for the user-reported "single finds it, batch doesn't" songs. The fix scores each provider + * hit against the candidate's parsed view of the track in addition to the raw (often junk) tags, so the + * primary providers accept these directly instead of relying on the last-resort rescue. + */ +class BadMetadataMatchTest { + + private fun candFor(tagTitle: String?, tagArtist: String?, path: String?, title: String, artist: String?) = + FilenameParser.candidates(tagTitle, tagArtist, path) + .first { it.title.equals(title, true) && (artist == null || it.artist.equals(artist, true)) } + + @Test + fun `Ariana Grande - Focus with Unknown artist auto-accepts the right hit`() { + // Title tag carries the whole "Artist - Title" string; artist tag is the "Unknown" placeholder. + val local = LocalTrack("Ariana Grande - Focus", null, 211.0) + val cand = candFor("Ariana Grande - Focus", "Unknown", null, "Focus", "Ariana Grande") + val hit = ProviderResult("Focus", "Ariana Grande", 211.0, null, true) + + val conf = scoreHitAgainstViews(local, hit, cand) + assertEquals(MatchTier.AUTO_ACCEPT, conf.tier) + } + + @Test + fun `junk artist tag is not treated as a real artist`() { + assertTrue(TextMatch.isJunkArtist("Unknown")) + assertTrue(TextMatch.isJunkArtist("")) + assertTrue(TextMatch.isJunkArtist("Various Artists")) + assertTrue(!TextMatch.isJunkArtist("Ariana Grande")) + // The TAGS candidate must not carry the placeholder as an artist. + val cands = FilenameParser.candidates("Ariana Grande - Focus", "Unknown", null) + val tags = cands.first { it.strategy == MatchStrategy.TAGS } + assertEquals(null, tags.artist) + } + + @Test + fun `D-Devils 6th Gate rip matches its provider entry`() { + val local = LocalTrack("D-Devils - 6th Gate", "xope87", 320.0) + val cand = candFor(null, null, "/m/D-Devils - 6th Gate(MP3_320K).mp3", "6th Gate", "D-Devils") + val hit = ProviderResult("The 6th Gate (Dance With The Devil)", "D-Devils", 320.0, null, true) + + val conf = scoreHitAgainstViews(local, hit, cand) + assertTrue("expected at least REVIEW, got ${conf.tier} (${conf.percent()}%)", conf.tier != MatchTier.REJECT) + assertTrue(conf.score >= 0.80) + } + + @Test + fun `Tyler The Creator - Tamale rip auto-accepts`() { + val local = LocalTrack("Tyler, The Creator - Tamale", null, 173.0) + val cand = candFor( + "Tyler, The Creator - Tamale", "Unknown", + "/m/Tyler_ The Creator - Tamale(MP3_320K).mp3", "Tamale", null + ) + val hit = ProviderResult("Tamale", "Tyler, The Creator", 173.0, null, true) + + val conf = scoreHitAgainstViews(local, hit, cand) + assertEquals(MatchTier.AUTO_ACCEPT, conf.tier) + } + + @Test + fun `Biba - Harli Kvin filename yields a clean feat-stripped candidate`() { + val cands = FilenameParser.candidates( + null, null, "/m/Biba - Harli Kvin (feat. AV47) [Official Audio] _420(MP3_320K).mp3" + ) + // The bracketed feat clause sits mid-string (junk "_420" follows it) — it must still be extracted, and + // the orphan "420" must be droppable, leaving a clean "Biba / Harli Kvin" query. + val clean = cands.firstOrNull { + it.title.equals("Harli Kvin", true) && it.artist.equals("Biba", true) + } + assertTrue("no clean candidate in: ${cands.map { "${it.artist} / ${it.title}" }}", clean != null) + assertTrue(clean!!.featuredArtists.any { it.equals("AV47", true) }) + } + + @Test + fun `title-only view without duration proof cannot auto-accept a wrong artist`() { + // Precision guard: an identically-titled song by a different artist must not ride in on the + // artist-less title-only view when the runtime doesn't confirm it. + val local = LocalTrack("Focus", "H.E.R.", 220.0) + val cand = FilenameParser.candidates(null, null, "/m/Focus.mp3") + .first { it.strategy == MatchStrategy.FILENAME_TITLE_ONLY } + val wrong = ProviderResult("Focus", "Ariana Grande", 211.0, null, true) + + val conf = scoreHitAgainstViews(local, wrong, cand) + assertEquals(MatchTier.REJECT, conf.tier) + } + + @Test + fun `trailing junk number variant stays review-grade at best`() { + // "Song 420" stripped to "Song" is a LOOSE candidate: even a perfect hit must not silently auto-accept. + val local = LocalTrack("Harli Kvin 420", "Biba", null) + val cand = FilenameParser.candidates(null, null, "/m/Biba - Harli Kvin _420.mp3") + .first { it.strategy == MatchStrategy.FILENAME_LOOSE && it.title.equals("Harli Kvin", true) } + val hit = ProviderResult("Harli Kvin", "Biba", null, null, true) + + val conf = scoreHitAgainstViews(local, hit, cand) + assertTrue(conf.tier != MatchTier.AUTO_ACCEPT) + assertTrue(conf.tier != MatchTier.REJECT) + } +}