From c54503f48605f30a706111da4cd49386cc69cbbb Mon Sep 17 00:00:00 2001 From: Josh Patra Date: Fri, 6 Feb 2026 13:04:40 -0500 Subject: [PATCH] fix: Spotify lyrics validation and proactive prefetching - Only attempt Spotify lyrics for tracks with valid Spotify IDs (22 chars, no 'local' or ':') - Add Spotify IDs to external matched tracks in playlists for lyrics support - Proactively fetch and cache lyrics when playback starts (background task) - Fix pre-existing SubSonicController bug (missing _cache field) - Lyrics now ready instantly when requested by client --- allstarr/Controllers/JellyfinController.cs | 201 ++++++++++++++++-- allstarr/Controllers/SubSonicController.cs | 3 + .../Spotify/SpotifyTrackMatchingService.cs | 32 +++ 3 files changed, 213 insertions(+), 23 deletions(-) diff --git a/allstarr/Controllers/JellyfinController.cs b/allstarr/Controllers/JellyfinController.cs index e781e0e..5e84437 100644 --- a/allstarr/Controllers/JellyfinController.cs +++ b/allstarr/Controllers/JellyfinController.cs @@ -1208,33 +1208,35 @@ public class JellyfinController : ControllerBase LyricsInfo? lyrics = null; - // Try Spotify lyrics first (better synced lyrics quality) - if (_spotifyLyricsService != null && _spotifyApiSettings.Enabled) + // Try Spotify lyrics ONLY if we have a valid Spotify track ID + // Spotify lyrics only work for tracks from injected playlists that have been matched + if (_spotifyLyricsService != null && _spotifyApiSettings.Enabled && !string.IsNullOrEmpty(spotifyTrackId)) { - _logger.LogInformation("Trying Spotify lyrics for: {Artist} - {Title}", searchArtist, searchTitle); + // Validate that this is a real Spotify ID (not spotify:local or other invalid formats) + var cleanSpotifyId = spotifyTrackId.Replace("spotify:track:", "").Trim(); - SpotifyLyricsResult? spotifyLyrics = null; - - // If we have a Spotify track ID, use it directly - if (!string.IsNullOrEmpty(spotifyTrackId)) + // Spotify track IDs are 22 characters, base62 encoded + if (cleanSpotifyId.Length == 22 && !cleanSpotifyId.Contains(":") && !cleanSpotifyId.Contains("local")) { - spotifyLyrics = await _spotifyLyricsService.GetLyricsByTrackIdAsync(spotifyTrackId); + _logger.LogInformation("Trying Spotify lyrics for track ID: {SpotifyId} ({Artist} - {Title})", + cleanSpotifyId, searchArtist, searchTitle); + + var spotifyLyrics = await _spotifyLyricsService.GetLyricsByTrackIdAsync(cleanSpotifyId); + + if (spotifyLyrics != null && spotifyLyrics.Lines.Count > 0) + { + _logger.LogInformation("Found Spotify lyrics for {Artist} - {Title} ({LineCount} lines, type: {SyncType})", + searchArtist, searchTitle, spotifyLyrics.Lines.Count, spotifyLyrics.SyncType); + lyrics = _spotifyLyricsService.ToLyricsInfo(spotifyLyrics); + } + else + { + _logger.LogDebug("No Spotify lyrics found for track ID {SpotifyId}", cleanSpotifyId); + } } else { - // Search by metadata (without [S] tags) - spotifyLyrics = await _spotifyLyricsService.SearchAndGetLyricsAsync( - searchTitle, - searchArtists.Count > 0 ? searchArtists[0] : searchArtist, - searchAlbum, - song.Duration.HasValue ? song.Duration.Value * 1000 : null); - } - - if (spotifyLyrics != null && spotifyLyrics.Lines.Count > 0) - { - _logger.LogInformation("Found Spotify lyrics for {Artist} - {Title} ({LineCount} lines, type: {SyncType})", - searchArtist, searchTitle, spotifyLyrics.Lines.Count, spotifyLyrics.SyncType); - lyrics = _spotifyLyricsService.ToLyricsInfo(spotifyLyrics); + _logger.LogDebug("Invalid Spotify ID format: {SpotifyId}, skipping Spotify lyrics", spotifyTrackId); } } @@ -1354,6 +1356,116 @@ public class JellyfinController : ControllerBase return Ok(response); } + + /// + /// Proactively fetches and caches lyrics for a track in the background. + /// Called when playback starts to ensure lyrics are ready when requested. + /// + private async Task PrefetchLyricsForTrackAsync(string itemId, bool isExternal, string? provider, string? externalId) + { + try + { + Song? song = null; + string? spotifyTrackId = null; + + if (isExternal && !string.IsNullOrEmpty(provider) && !string.IsNullOrEmpty(externalId)) + { + // Get external track metadata + song = await _metadataService.GetSongAsync(provider, externalId); + + // Try to find Spotify ID from matched tracks cache + if (song != null) + { + spotifyTrackId = await FindSpotifyIdForExternalTrackAsync(song); + } + } + else + { + // Get local track metadata from Jellyfin + var (item, _) = await _proxyService.GetItemAsync(itemId, Request.Headers); + if (item != null && item.RootElement.TryGetProperty("Type", out var typeEl) && + typeEl.GetString() == "Audio") + { + song = new Song + { + Title = item.RootElement.TryGetProperty("Name", out var name) ? name.GetString() ?? "" : "", + Artist = item.RootElement.TryGetProperty("AlbumArtist", out var artist) ? artist.GetString() ?? "" : "", + Album = item.RootElement.TryGetProperty("Album", out var album) ? album.GetString() ?? "" : "", + Duration = item.RootElement.TryGetProperty("RunTimeTicks", out var ticks) ? (int)(ticks.GetInt64() / 10000000) : 0 + }; + + // Check for Spotify ID in provider IDs + if (item.RootElement.TryGetProperty("ProviderIds", out var providerIds)) + { + if (providerIds.TryGetProperty("Spotify", out var spotifyId)) + { + spotifyTrackId = spotifyId.GetString(); + } + } + } + } + + if (song == null) + { + _logger.LogDebug("Could not get song metadata for lyrics prefetch: {ItemId}", itemId); + return; + } + + // Strip [S] suffix for lyrics search + var searchTitle = song.Title.Replace(" [S]", "").Trim(); + var searchArtist = song.Artist?.Replace(" [S]", "").Trim() ?? ""; + var searchAlbum = song.Album?.Replace(" [S]", "").Trim() ?? ""; + var searchArtists = song.Artists.Select(a => a.Replace(" [S]", "").Trim()).ToList(); + + if (searchArtists.Count == 0 && !string.IsNullOrEmpty(searchArtist)) + { + searchArtists.Add(searchArtist); + } + + _logger.LogDebug("🎵 Prefetching lyrics for: {Artist} - {Title}", searchArtist, searchTitle); + + // Try Spotify lyrics if we have a valid Spotify track ID + if (_spotifyLyricsService != null && _spotifyApiSettings.Enabled && !string.IsNullOrEmpty(spotifyTrackId)) + { + var cleanSpotifyId = spotifyTrackId.Replace("spotify:track:", "").Trim(); + + if (cleanSpotifyId.Length == 22 && !cleanSpotifyId.Contains(":") && !cleanSpotifyId.Contains("local")) + { + var spotifyLyrics = await _spotifyLyricsService.GetLyricsByTrackIdAsync(cleanSpotifyId); + + if (spotifyLyrics != null && spotifyLyrics.Lines.Count > 0) + { + _logger.LogDebug("✓ Prefetched Spotify lyrics for {Artist} - {Title} ({LineCount} lines)", + searchArtist, searchTitle, spotifyLyrics.Lines.Count); + return; // Success, lyrics are now cached + } + } + } + + // Fall back to LRCLIB + if (_lrclibService != null) + { + var lyrics = await _lrclibService.GetLyricsAsync( + searchTitle, + searchArtists.ToArray(), + searchAlbum, + song.Duration ?? 0); + + if (lyrics != null) + { + _logger.LogDebug("✓ Prefetched LRCLIB lyrics for {Artist} - {Title}", searchArtist, searchTitle); + } + else + { + _logger.LogDebug("No lyrics found for {Artist} - {Title}", searchArtist, searchTitle); + } + } + } + catch (Exception ex) + { + _logger.LogDebug(ex, "Error prefetching lyrics for track {ItemId}", itemId); + } + } #endregion @@ -2064,6 +2176,20 @@ public class JellyfinController : ControllerBase { _logger.LogInformation("🎵 External track playback started: {Name} ({Provider}/{ExternalId})", itemName ?? "Unknown", provider, externalId); + + // Proactively fetch lyrics in background for external tracks + _ = Task.Run(async () => + { + try + { + await PrefetchLyricsForTrackAsync(itemId, isExternal: true, provider, externalId); + } + catch (Exception ex) + { + _logger.LogDebug(ex, "Failed to prefetch lyrics for external track {ItemId}", itemId); + } + }); + // For external tracks, we can't report to Jellyfin since it doesn't know about them // Just return success so the client is happy return NoContent(); @@ -2071,6 +2197,19 @@ public class JellyfinController : ControllerBase _logger.LogInformation("🎵 Local track playback started: {Name} (ID: {ItemId})", itemName ?? "Unknown", itemId); + + // Proactively fetch lyrics in background for local tracks + _ = Task.Run(async () => + { + try + { + await PrefetchLyricsForTrackAsync(itemId, isExternal: false, null, null); + } + catch (Exception ex) + { + _logger.LogDebug(ex, "Failed to prefetch lyrics for local track {ItemId}", itemId); + } + }); } // For local tracks, forward playback start to Jellyfin FIRST @@ -3302,11 +3441,27 @@ public class JellyfinController : ControllerBase { // Convert external song to Jellyfin item format var externalItem = _responseBuilder.ConvertSongToJellyfinItem(matched.MatchedSong); + + // Add Spotify ID to ProviderIds so lyrics can work + if (!string.IsNullOrEmpty(spotifyTrack.SpotifyId)) + { + if (!externalItem.ContainsKey("ProviderIds")) + { + externalItem["ProviderIds"] = new Dictionary(); + } + + var providerIds = externalItem["ProviderIds"] as Dictionary; + if (providerIds != null && !providerIds.ContainsKey("Spotify")) + { + providerIds["Spotify"] = spotifyTrack.SpotifyId; + } + } + finalItems.Add(externalItem); externalUsedCount++; - _logger.LogDebug("📥 Position #{Pos}: '{Title}' → EXTERNAL: {Provider}/{Id}", + _logger.LogDebug("📥 Position #{Pos}: '{Title}' → EXTERNAL: {Provider}/{Id} (Spotify ID: {SpotifyId})", spotifyTrack.Position, spotifyTrack.Title, - matched.MatchedSong.ExternalProvider, matched.MatchedSong.ExternalId); + matched.MatchedSong.ExternalProvider, matched.MatchedSong.ExternalId, spotifyTrack.SpotifyId); } else { diff --git a/allstarr/Controllers/SubSonicController.cs b/allstarr/Controllers/SubSonicController.cs index ef75482..d30ee34 100644 --- a/allstarr/Controllers/SubSonicController.cs +++ b/allstarr/Controllers/SubSonicController.cs @@ -28,6 +28,7 @@ public class SubsonicController : ControllerBase private readonly SubsonicModelMapper _modelMapper; private readonly SubsonicProxyService _proxyService; private readonly PlaylistSyncService? _playlistSyncService; + private readonly RedisCacheService _cache; private readonly ILogger _logger; public SubsonicController( @@ -39,6 +40,7 @@ public class SubsonicController : ControllerBase SubsonicResponseBuilder responseBuilder, SubsonicModelMapper modelMapper, SubsonicProxyService proxyService, + RedisCacheService cache, ILogger logger, PlaylistSyncService? playlistSyncService = null) { @@ -51,6 +53,7 @@ public class SubsonicController : ControllerBase _modelMapper = modelMapper; _proxyService = proxyService; _playlistSyncService = playlistSyncService; + _cache = cache; _logger = logger; if (string.IsNullOrWhiteSpace(_subsonicSettings.Url)) diff --git a/allstarr/Services/Spotify/SpotifyTrackMatchingService.cs b/allstarr/Services/Spotify/SpotifyTrackMatchingService.cs index ba01824..37e77f1 100644 --- a/allstarr/Services/Spotify/SpotifyTrackMatchingService.cs +++ b/allstarr/Services/Spotify/SpotifyTrackMatchingService.cs @@ -894,6 +894,22 @@ public class SpotifyTrackMatchingService : BackgroundService // Convert external song to Jellyfin item format and add to finalItems var externalItem = responseBuilder.ConvertSongToJellyfinItem(externalSong); + + // Add Spotify ID to ProviderIds so lyrics can work + if (!string.IsNullOrEmpty(spotifyTrack.SpotifyId)) + { + if (!externalItem.ContainsKey("ProviderIds")) + { + externalItem["ProviderIds"] = new Dictionary(); + } + + var providerIds = externalItem["ProviderIds"] as Dictionary; + if (providerIds != null && !providerIds.ContainsKey("Spotify")) + { + providerIds["Spotify"] = spotifyTrack.SpotifyId; + } + } + finalItems.Add(externalItem); externalUsedCount++; manualExternalCount++; @@ -958,6 +974,22 @@ public class SpotifyTrackMatchingService : BackgroundService { // Convert external song to Jellyfin item format var externalItem = responseBuilder.ConvertSongToJellyfinItem(matched.MatchedSong); + + // Add Spotify ID to ProviderIds so lyrics can work + if (!string.IsNullOrEmpty(spotifyTrack.SpotifyId)) + { + if (!externalItem.ContainsKey("ProviderIds")) + { + externalItem["ProviderIds"] = new Dictionary(); + } + + var providerIds = externalItem["ProviderIds"] as Dictionary; + if (providerIds != null && !providerIds.ContainsKey("Spotify")) + { + providerIds["Spotify"] = spotifyTrack.SpotifyId; + } + } + finalItems.Add(externalItem); externalUsedCount++; }