mirror of
https://github.com/zarzet/SpotiFLAC-Mobile.git
synced 2026-08-27 05:12:29 +02:00
fix(lyrics): respect extension provider selection #511
This commit is contained in:
+69
-40
@@ -192,6 +192,7 @@ func (c *LyricsClient) durationMatches(lrcDuration, targetDuration float64) bool
|
||||
func (c *LyricsClient) FetchLyricsAllSources(spotifyID, trackName, artistName string, durationSec float64) (*LyricsResponse, error) {
|
||||
primaryArtist := normalizeArtistName(artistName)
|
||||
fetchOptions := GetLyricsFetchOptions()
|
||||
configuredProviderOrder := GetLyricsProviderOrder()
|
||||
|
||||
if isLikelyInstrumentalTrack(trackName) {
|
||||
GoLog("[Lyrics] Track marked instrumental by title heuristic, skipping lyrics search: %s - %s\n", artistName, trackName)
|
||||
@@ -203,62 +204,57 @@ func (c *LyricsClient) FetchLyricsAllSources(spotifyID, trackName, artistName st
|
||||
return instrumental, nil
|
||||
}
|
||||
|
||||
extensionProviders := make(map[string]*extensionProviderWrapper)
|
||||
extManager := getExtensionManager()
|
||||
var extensionProviders []*extensionProviderWrapper
|
||||
if extManager != nil {
|
||||
extensionProviders = extManager.GetLyricsProviders()
|
||||
for _, provider := range extManager.GetLyricsProviders() {
|
||||
providerName := "extension:" + strings.ToLower(strings.TrimSpace(provider.extension.ID))
|
||||
extensionProviders[providerName] = provider
|
||||
}
|
||||
}
|
||||
|
||||
providerOrder := resolveLyricsProviderOrder(configuredProviderOrder, extensionProviders)
|
||||
selectedExtensionCount := 0
|
||||
for _, providerName := range providerOrder {
|
||||
if strings.HasPrefix(providerName, "extension:") {
|
||||
selectedExtensionCount++
|
||||
}
|
||||
}
|
||||
|
||||
var cachedNonExtension *LyricsResponse
|
||||
if cached, found := globalLyricsCache.Get(artistName, trackName, durationSec); found {
|
||||
isExtensionCache := strings.HasPrefix(cached.Source, "Extension:")
|
||||
if len(extensionProviders) == 0 || isExtensionCache {
|
||||
cachedProviderSelected := false
|
||||
if isExtensionCache {
|
||||
cachedProviderName := "extension:" + strings.ToLower(strings.TrimSpace(strings.TrimPrefix(cached.Source, "Extension:")))
|
||||
for _, providerName := range providerOrder {
|
||||
if providerName == cachedProviderName {
|
||||
cachedProviderSelected = true
|
||||
break
|
||||
}
|
||||
}
|
||||
}
|
||||
if (!isExtensionCache && selectedExtensionCount == 0) || cachedProviderSelected {
|
||||
fmt.Printf("[Lyrics] Cache hit for: %s - %s\n", artistName, trackName)
|
||||
cachedCopy := *cached
|
||||
cachedCopy.Source = cached.Source + " (cached)"
|
||||
return &cachedCopy, nil
|
||||
}
|
||||
|
||||
// If extension providers are currently enabled, don't let stale built-in cache
|
||||
// mask newly installed/activated extensions.
|
||||
cachedNonExtension = cached
|
||||
GoLog("[Lyrics] Ignoring cached non-extension lyrics because extension providers are available\n")
|
||||
if !isExtensionCache {
|
||||
// If extension providers are currently selected, don't let stale built-in
|
||||
// cache mask them. It remains available as a fallback if they fail.
|
||||
cachedNonExtension = cached
|
||||
GoLog("[Lyrics] Ignoring cached non-extension lyrics because selected extension providers are available\n")
|
||||
} else {
|
||||
GoLog("[Lyrics] Ignoring cached lyrics from an unselected extension provider\n")
|
||||
}
|
||||
}
|
||||
|
||||
isValidResult := func(l *LyricsResponse) bool {
|
||||
return lyricsHasUsableText(l)
|
||||
}
|
||||
|
||||
if len(extensionProviders) > 0 {
|
||||
for _, provider := range extensionProviders {
|
||||
providerName := "extension:" + provider.extension.ID
|
||||
if skip, remaining, reason := shouldSkipLyricsProvider(providerName); skip {
|
||||
GoLog("[Lyrics] Skipping unavailable extension lyrics provider %s for %s: %s\n", provider.extension.ID, remaining.Round(time.Second), reason)
|
||||
continue
|
||||
}
|
||||
GoLog("[Lyrics] Trying extension lyrics provider: %s\n", provider.extension.ID)
|
||||
lyrics, err := provider.FetchLyrics(trackName, artistName, "", durationSec)
|
||||
if err == nil && isValidResult(lyrics) {
|
||||
GoLog("[Lyrics] Got lyrics from extension: %s\n", provider.extension.ID)
|
||||
markLyricsProviderAvailable(providerName)
|
||||
globalLyricsCache.Set(artistName, trackName, durationSec, lyrics)
|
||||
return lyrics, nil
|
||||
}
|
||||
if err != nil {
|
||||
GoLog("[Lyrics] Extension %s failed: %v\n", provider.extension.ID, err)
|
||||
markLyricsProviderUnavailable(providerName, err)
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
if cachedNonExtension != nil {
|
||||
cachedCopy := *cachedNonExtension
|
||||
cachedCopy.Source = cachedNonExtension.Source + " (cached fallback)"
|
||||
GoLog("[Lyrics] Extension providers unavailable for this track, using cached built-in lyrics\n")
|
||||
return &cachedCopy, nil
|
||||
}
|
||||
|
||||
providerOrder := GetLyricsProviderOrder()
|
||||
simplifiedTrack := simplifyTrackName(trackName)
|
||||
request := lyricsProviderSearchRequest{
|
||||
spotifyID: spotifyID,
|
||||
@@ -272,16 +268,48 @@ func (c *LyricsClient) FetchLyricsAllSources(spotifyID, trackName, artistName st
|
||||
|
||||
GoLog("[Lyrics] Searching for: %s - %s (providers: %v)\n", artistName, trackName, providerOrder)
|
||||
|
||||
lyrics, err := fetchBuiltInLyricsProviders(providerOrder, request, c.fetchBuiltInLyricsProvider)
|
||||
fetchProvider := func(providerName string, request lyricsProviderSearchRequest) (*LyricsResponse, error, bool) {
|
||||
if provider, ok := extensionProviders[providerName]; ok {
|
||||
lyrics, err := provider.FetchLyrics(request.trackName, request.artistName, "", request.durationSec)
|
||||
return lyrics, err, true
|
||||
}
|
||||
return c.fetchBuiltInLyricsProvider(providerName, request)
|
||||
}
|
||||
|
||||
lyrics, err := fetchLyricsProviders(providerOrder, request, fetchProvider)
|
||||
if err == nil && isValidResult(lyrics) {
|
||||
globalLyricsCache.Set(artistName, trackName, durationSec, lyrics)
|
||||
return lyrics, nil
|
||||
}
|
||||
|
||||
if cachedNonExtension != nil {
|
||||
cachedCopy := *cachedNonExtension
|
||||
cachedCopy.Source = cachedNonExtension.Source + " (cached fallback)"
|
||||
GoLog("[Lyrics] Selected extension providers unavailable for this track, using cached built-in lyrics\n")
|
||||
return &cachedCopy, nil
|
||||
}
|
||||
|
||||
return nil, fmt.Errorf("lyrics not found from any source")
|
||||
}
|
||||
|
||||
func fetchBuiltInLyricsProviders(
|
||||
func resolveLyricsProviderOrder(
|
||||
configuredOrder []string,
|
||||
extensionProviders map[string]*extensionProviderWrapper,
|
||||
) []string {
|
||||
providerOrder := make([]string, 0, len(configuredOrder))
|
||||
for _, providerName := range configuredOrder {
|
||||
if isKnownBuiltInLyricsProvider(providerName) {
|
||||
providerOrder = append(providerOrder, providerName)
|
||||
continue
|
||||
}
|
||||
if _, available := extensionProviders[providerName]; available {
|
||||
providerOrder = append(providerOrder, providerName)
|
||||
}
|
||||
}
|
||||
return providerOrder
|
||||
}
|
||||
|
||||
func fetchLyricsProviders(
|
||||
providerOrder []string,
|
||||
request lyricsProviderSearchRequest,
|
||||
fetchProvider func(string, lyricsProviderSearchRequest) (*LyricsResponse, error, bool),
|
||||
@@ -302,7 +330,8 @@ func fetchBuiltInLyricsProviders(
|
||||
continue
|
||||
}
|
||||
|
||||
knownProvider := isKnownBuiltInLyricsProvider(providerName)
|
||||
knownProvider := isKnownBuiltInLyricsProvider(providerName) ||
|
||||
(strings.HasPrefix(providerName, "extension:") && len(providerName) > len("extension:"))
|
||||
if !knownProvider {
|
||||
GoLog("[Lyrics] Unknown provider: %s, skipping\n", providerName)
|
||||
continue
|
||||
|
||||
@@ -118,11 +118,15 @@ var (
|
||||
|
||||
func SetLyricsProviderOrder(providers []string) {
|
||||
lyricsProvidersMu.Lock()
|
||||
defer lyricsProvidersMu.Unlock()
|
||||
|
||||
if len(providers) == 0 {
|
||||
changed := len(lyricsProviders) != 0
|
||||
lyricsProviders = nil
|
||||
lyricsProvidersMu.Unlock()
|
||||
clearLyricsProviderHealth()
|
||||
if changed {
|
||||
globalLyricsCache.ClearAll()
|
||||
}
|
||||
return
|
||||
}
|
||||
|
||||
@@ -140,19 +144,44 @@ func SetLyricsProviderOrder(providers []string) {
|
||||
LyricsProviderLyricsPlus: true,
|
||||
}
|
||||
|
||||
var valid []string
|
||||
valid := make([]string, 0, len(providers))
|
||||
seen := make(map[string]struct{}, len(providers))
|
||||
for _, p := range providers {
|
||||
normalized := strings.ToLower(strings.TrimSpace(p))
|
||||
if validNames[normalized] {
|
||||
valid = append(valid, normalized)
|
||||
isExtension := strings.HasPrefix(normalized, "extension:") &&
|
||||
strings.TrimSpace(strings.TrimPrefix(normalized, "extension:")) != ""
|
||||
if !validNames[normalized] && !isExtension {
|
||||
continue
|
||||
}
|
||||
if _, exists := seen[normalized]; exists {
|
||||
continue
|
||||
}
|
||||
seen[normalized] = struct{}{}
|
||||
valid = append(valid, normalized)
|
||||
}
|
||||
|
||||
changed := !equalLyricsProviderOrders(lyricsProviders, valid)
|
||||
lyricsProviders = valid
|
||||
lyricsProvidersMu.Unlock()
|
||||
clearLyricsProviderHealth()
|
||||
if changed {
|
||||
globalLyricsCache.ClearAll()
|
||||
}
|
||||
GoLog("[Lyrics] Provider order set to: %v\n", valid)
|
||||
}
|
||||
|
||||
func equalLyricsProviderOrders(a, b []string) bool {
|
||||
if len(a) != len(b) {
|
||||
return false
|
||||
}
|
||||
for i := range a {
|
||||
if a[i] != b[i] {
|
||||
return false
|
||||
}
|
||||
}
|
||||
return true
|
||||
}
|
||||
|
||||
func clearLyricsProviderHealth() {
|
||||
lyricsProviderHealthMu.Lock()
|
||||
defer lyricsProviderHealthMu.Unlock()
|
||||
@@ -242,7 +271,9 @@ func GetLyricsProviderOrder() []string {
|
||||
defer lyricsProvidersMu.RUnlock()
|
||||
|
||||
if len(lyricsProviders) == 0 {
|
||||
return DefaultLyricsProviders
|
||||
result := make([]string, len(DefaultLyricsProviders))
|
||||
copy(result, DefaultLyricsProviders)
|
||||
return result
|
||||
}
|
||||
|
||||
result := make([]string, len(lyricsProviders))
|
||||
|
||||
@@ -16,8 +16,8 @@ func TestLyricsCacheParsingAndLRCLibClient(t *testing.T) {
|
||||
if ua := appUserAgent(); !strings.Contains(ua, "4.5.0") {
|
||||
t.Fatalf("user agent = %q", ua)
|
||||
}
|
||||
SetLyricsProviderOrder([]string{"LRCLIB", "bad", "netease"})
|
||||
if providers := GetLyricsProviderOrder(); len(providers) != 2 || providers[0] != LyricsProviderLRCLIB {
|
||||
SetLyricsProviderOrder([]string{"LRCLIB", "bad", "extension:Apple-Music", "netease", "extension:apple-music"})
|
||||
if providers := GetLyricsProviderOrder(); len(providers) != 3 || providers[0] != LyricsProviderLRCLIB || providers[1] != "extension:apple-music" {
|
||||
t.Fatalf("providers = %#v", providers)
|
||||
}
|
||||
SetLyricsProviderOrder(nil)
|
||||
@@ -228,7 +228,7 @@ func TestConcurrentLyricsProvidersReturnFastFallback(t *testing.T) {
|
||||
defer clearLyricsProviderHealth()
|
||||
|
||||
start := time.Now()
|
||||
lyrics, err := fetchBuiltInLyricsProviders(
|
||||
lyrics, err := fetchLyricsProviders(
|
||||
[]string{LyricsProviderLRCLIB, LyricsProviderAppleMusic},
|
||||
lyricsProviderSearchRequest{},
|
||||
func(providerName string, _ lyricsProviderSearchRequest) (*LyricsResponse, error, bool) {
|
||||
@@ -250,11 +250,36 @@ func TestConcurrentLyricsProvidersReturnFastFallback(t *testing.T) {
|
||||
}
|
||||
}
|
||||
|
||||
func TestResolveLyricsProviderOrderOnlyIncludesSelectedAvailableExtensions(t *testing.T) {
|
||||
availableExtensions := map[string]*extensionProviderWrapper{
|
||||
"extension:apple-music": nil,
|
||||
"extension:future-provider": nil,
|
||||
}
|
||||
providers := resolveLyricsProviderOrder(
|
||||
[]string{
|
||||
LyricsProviderLRCLIB,
|
||||
"extension:future-provider",
|
||||
"extension:not-installed",
|
||||
LyricsProviderNetease,
|
||||
},
|
||||
availableExtensions,
|
||||
)
|
||||
|
||||
want := []string{
|
||||
LyricsProviderLRCLIB,
|
||||
"extension:future-provider",
|
||||
LyricsProviderNetease,
|
||||
}
|
||||
if !equalLyricsProviderOrders(providers, want) {
|
||||
t.Fatalf("providers = %#v, want %#v", providers, want)
|
||||
}
|
||||
}
|
||||
|
||||
func TestConcurrentLyricsProvidersPreferEarlierProviderWithinGrace(t *testing.T) {
|
||||
clearLyricsProviderHealth()
|
||||
defer clearLyricsProviderHealth()
|
||||
|
||||
lyrics, err := fetchBuiltInLyricsProviders(
|
||||
lyrics, err := fetchLyricsProviders(
|
||||
[]string{LyricsProviderLRCLIB, LyricsProviderAppleMusic},
|
||||
lyricsProviderSearchRequest{},
|
||||
func(providerName string, _ lyricsProviderSearchRequest) (*LyricsResponse, error, bool) {
|
||||
|
||||
@@ -1,6 +1,7 @@
|
||||
import 'package:flutter/material.dart';
|
||||
import 'package:flutter_riverpod/flutter_riverpod.dart';
|
||||
import 'package:spotiflac_android/l10n/l10n.dart';
|
||||
import 'package:spotiflac_android/providers/extension_provider.dart';
|
||||
import 'package:spotiflac_android/providers/settings_provider.dart';
|
||||
import 'package:spotiflac_android/utils/adaptive_layout.dart';
|
||||
import 'package:spotiflac_android/widgets/discard_changes_dialog.dart';
|
||||
@@ -19,7 +20,7 @@ class LyricsProviderPriorityPage extends ConsumerStatefulWidget {
|
||||
|
||||
class _LyricsProviderPriorityPageState
|
||||
extends ConsumerState<LyricsProviderPriorityPage> {
|
||||
static const _allProviderIds = [
|
||||
static const _builtInProviderIds = [
|
||||
'lrclib',
|
||||
'netease',
|
||||
'musixmatch',
|
||||
@@ -37,9 +38,6 @@ class _LyricsProviderPriorityPageState
|
||||
late List<String> _initialProviders;
|
||||
bool _hasChanges = false;
|
||||
|
||||
List<String> get _disabledProviders =>
|
||||
_allProviderIds.where((id) => !_enabledProviders.contains(id)).toList();
|
||||
|
||||
@override
|
||||
void initState() {
|
||||
super.initState();
|
||||
@@ -61,7 +59,19 @@ class _LyricsProviderPriorityPageState
|
||||
|
||||
@override
|
||||
Widget build(BuildContext context) {
|
||||
final disabled = _disabledProviders;
|
||||
final extensions = ref.watch(extensionProvider).extensions;
|
||||
final extensionNames = <String, String>{
|
||||
for (final extension in extensions)
|
||||
if (extension.enabled && extension.hasLyricsProvider)
|
||||
'extension:${extension.id.toLowerCase()}': extension.displayName,
|
||||
};
|
||||
final allProviderIds = <String>[
|
||||
..._builtInProviderIds,
|
||||
...extensionNames.keys.where((id) => !_builtInProviderIds.contains(id)),
|
||||
];
|
||||
final disabled = allProviderIds
|
||||
.where((id) => !_enabledProviders.contains(id))
|
||||
.toList();
|
||||
|
||||
return PrioritySettingsScaffold(
|
||||
hasChanges: _hasChanges,
|
||||
@@ -91,7 +101,11 @@ class _LyricsProviderPriorityPageState
|
||||
itemCount: _enabledProviders.length,
|
||||
itemBuilder: (context, index) {
|
||||
final id = _enabledProviders[index];
|
||||
final info = _getLyricsProviderInfo(id, context);
|
||||
final info = _getLyricsProviderInfo(
|
||||
id,
|
||||
context,
|
||||
extensionNames[id],
|
||||
);
|
||||
return ReorderablePriorityItem(
|
||||
key: ValueKey(id),
|
||||
index: index,
|
||||
@@ -136,7 +150,11 @@ class _LyricsProviderPriorityPageState
|
||||
sliver: SliverList(
|
||||
delegate: SliverChildBuilderDelegate((context, index) {
|
||||
final id = disabled[index];
|
||||
final info = _getLyricsProviderInfo(id, context);
|
||||
final info = _getLyricsProviderInfo(
|
||||
id,
|
||||
context,
|
||||
extensionNames[id],
|
||||
);
|
||||
return _DisabledProviderItem(
|
||||
key: ValueKey(id),
|
||||
providerId: id,
|
||||
@@ -183,6 +201,7 @@ class _LyricsProviderPriorityPageState
|
||||
static _LyricsProviderInfo _getLyricsProviderInfo(
|
||||
String id,
|
||||
BuildContext context,
|
||||
String? extensionDisplayName,
|
||||
) {
|
||||
switch (id) {
|
||||
case 'lrclib':
|
||||
@@ -253,7 +272,11 @@ class _LyricsProviderPriorityPageState
|
||||
);
|
||||
default:
|
||||
return _LyricsProviderInfo(
|
||||
name: id,
|
||||
name:
|
||||
extensionDisplayName ??
|
||||
(id.startsWith('extension:')
|
||||
? id.substring('extension:'.length)
|
||||
: id),
|
||||
description: context.l10n.lyricsProviderExtensionDesc,
|
||||
icon: Icons.extension,
|
||||
);
|
||||
|
||||
@@ -1,6 +1,7 @@
|
||||
import 'package:flutter/material.dart';
|
||||
import 'package:flutter_riverpod/flutter_riverpod.dart';
|
||||
import 'package:spotiflac_android/l10n/l10n.dart';
|
||||
import 'package:spotiflac_android/providers/extension_provider.dart';
|
||||
import 'package:spotiflac_android/providers/settings_provider.dart';
|
||||
import 'package:spotiflac_android/screens/settings/lyrics_provider_priority_page.dart';
|
||||
import 'package:spotiflac_android/widgets/settings_group.dart';
|
||||
@@ -12,6 +13,10 @@ class LyricsSettingsPage extends ConsumerWidget {
|
||||
@override
|
||||
Widget build(BuildContext context, WidgetRef ref) {
|
||||
final settings = ref.watch(settingsProvider);
|
||||
final extensionProviderNames = <String, String>{
|
||||
for (final extension in ref.watch(extensionProvider).extensions)
|
||||
'extension:${extension.id.toLowerCase()}': extension.displayName,
|
||||
};
|
||||
|
||||
return PopScope(
|
||||
canPop: true,
|
||||
@@ -59,6 +64,7 @@ class LyricsSettingsPage extends ConsumerWidget {
|
||||
subtitle: _getLyricsProvidersSubtitle(
|
||||
context,
|
||||
settings.lyricsProviders,
|
||||
extensionProviderNames,
|
||||
),
|
||||
onTap: () => Navigator.push(
|
||||
context,
|
||||
@@ -187,9 +193,19 @@ class LyricsSettingsPage extends ConsumerWidget {
|
||||
String _getLyricsProvidersSubtitle(
|
||||
BuildContext context,
|
||||
List<String> providers,
|
||||
Map<String, String> extensionProviderNames,
|
||||
) {
|
||||
if (providers.isEmpty) return context.l10n.downloadProvidersNoneEnabled;
|
||||
return providers.map((p) => _providerDisplayNames[p] ?? p).join(' > ');
|
||||
return providers
|
||||
.map(
|
||||
(provider) =>
|
||||
_providerDisplayNames[provider] ??
|
||||
extensionProviderNames[provider] ??
|
||||
(provider.startsWith('extension:')
|
||||
? provider.substring('extension:'.length)
|
||||
: provider),
|
||||
)
|
||||
.join(' > ');
|
||||
}
|
||||
|
||||
void _showLyricsModePicker(
|
||||
|
||||
Reference in New Issue
Block a user