diff --git a/go_backend/exports_lyrics.go b/go_backend/exports_lyrics.go index 989ede5c..079c60f8 100644 --- a/go_backend/exports_lyrics.go +++ b/go_backend/exports_lyrics.go @@ -10,7 +10,7 @@ import ( func GetLyricsLRC(spotifyID, trackName, artistName string, filePath string, durationMs int64) (string, error) { if filePath != "" { lyrics, err := ExtractLyrics(filePath) - if err == nil && lyrics != "" { + if err == nil && rawLyricsHasUsableContent(lyrics) { return lyrics, nil } return "", nil @@ -34,7 +34,7 @@ func GetLyricsLRC(spotifyID, trackName, artistName string, filePath string, dura func GetLyricsLRCWithSource(spotifyID, trackName, artistName string, filePath string, durationMs int64) (string, error) { if filePath != "" { lyrics, err := ExtractLyrics(filePath) - if err == nil && lyrics != "" { + if err == nil && rawLyricsHasUsableContent(lyrics) { source := extractLyricsSourceFromLRC(lyrics) if source == "" { source = "Embedded" @@ -43,7 +43,7 @@ func GetLyricsLRCWithSource(spotifyID, trackName, artistName string, filePath st "lyrics": lyrics, "source": source, "sync_type": "EMBEDDED", - "instrumental": false, + "instrumental": isInstrumentalLyricsMarker(lyrics), } return marshalJSONString(result) } @@ -100,7 +100,7 @@ func FetchAndSaveLyrics(trackName, artistName, spotifyID string, durationMs int6 // use those directly instead of making redundant network requests. if audioFilePath != "" { existing, err := ExtractLyrics(audioFilePath) - if err == nil && strings.TrimSpace(existing) != "" { + if err == nil && rawLyricsHasUsableContent(existing) { if err := os.WriteFile(outputPath, []byte(existing), 0644); err != nil { return fmt.Errorf("failed to write LRC file: %w", err) } diff --git a/go_backend/exports_songlink_lyrics_supplement_test.go b/go_backend/exports_songlink_lyrics_supplement_test.go index 4c6d751c..6db6c186 100644 --- a/go_backend/exports_songlink_lyrics_supplement_test.go +++ b/go_backend/exports_songlink_lyrics_supplement_test.go @@ -48,6 +48,36 @@ func TestLyricsExportWrappersWithoutNetwork(t *testing.T) { } } +func TestLyricsExportWrappersRejectMetadataOnlySidecar(t *testing.T) { + dir := t.TempDir() + audioPath := filepath.Join(dir, "metadata-only.mp3") + if err := os.WriteFile(audioPath, []byte("audio"), 0600); err != nil { + t.Fatal(err) + } + metadataOnly := "[ti:Title]\n[ar:Artist]\n[al:Album]\n[by:SpotiFLAC Mobile]" + if err := os.WriteFile(filepath.Join(dir, "metadata-only.lrc"), []byte(metadataOnly), 0600); err != nil { + t.Fatal(err) + } + + if rawLyricsHasUsableContent(metadataOnly) { + t.Fatal("metadata-only LRC must not be considered usable") + } + if !rawLyricsHasUsableContent("[00:01.00]Actual lyric") { + t.Fatal("timed lyric must be considered usable") + } + if !rawLyricsHasUsableContent("[instrumental:true]") { + t.Fatal("instrumental marker must be considered usable") + } + + if lrc, err := GetLyricsLRC("", "", "", audioPath, 0); err != nil || lrc != "" { + t.Fatalf("GetLyricsLRC metadata-only sidecar = %q/%v", lrc, err) + } + if jsonText, err := GetLyricsLRCWithSource("", "", "", audioPath, 0); err != nil || + !strings.Contains(jsonText, `"lyrics":""`) || strings.Contains(jsonText, `"source":"Embedded"`) { + t.Fatalf("GetLyricsLRCWithSource metadata-only sidecar = %q/%v", jsonText, err) + } +} + func TestSongLinkExportWrappersWithFakeClient(t *testing.T) { origClient := globalSongLinkClient origRetryConfig := songLinkRetryConfig diff --git a/go_backend/lyrics_lrc.go b/go_backend/lyrics_lrc.go index 149d2de1..d0613bf0 100644 --- a/go_backend/lyrics_lrc.go +++ b/go_backend/lyrics_lrc.go @@ -12,6 +12,53 @@ import ( var lrcLinePattern = regexp.MustCompile(`\[(\d{2}):(\d{2})\.(\d{2,3})\](.*)`) +var ( + rawLyricsMetadataLinePattern = regexp.MustCompile(`(?i)^\[[a-z][a-z0-9_]*:.*\]$`) + rawLyricsBackgroundPattern = regexp.MustCompile(`(?i)^\[bg:(.*)\]$`) + rawLyricsTimestampPattern = regexp.MustCompile(`^\[\d{1,3}:\d{1,2}(?:[.:]\d{1,3})?\]`) + rawLyricsInlineTimePattern = regexp.MustCompile(`<\d{1,3}:\d{1,2}(?:[.:]\d{1,3})?>`) +) + +func isInstrumentalLyricsMarker(raw string) bool { + return strings.EqualFold(strings.TrimSpace(raw), "[instrumental:true]") +} + +// rawLyricsHasUsableContent rejects LRC payloads that contain only metadata +// headers. Those payloads are common in partially tagged files and must not be +// exposed as a blank "Embedded" lyrics result. +func rawLyricsHasUsableContent(raw string) bool { + if isInstrumentalLyricsMarker(raw) { + return true + } + + for _, line := range strings.Split(raw, "\n") { + cleaned := strings.TrimSpace(line) + if cleaned == "" { + continue + } + + if match := rawLyricsBackgroundPattern.FindStringSubmatch(cleaned); len(match) == 2 { + cleaned = strings.TrimSpace(match[1]) + } else if rawLyricsMetadataLinePattern.MatchString(cleaned) { + continue + } + + for rawLyricsTimestampPattern.MatchString(cleaned) { + cleaned = strings.TrimSpace(rawLyricsTimestampPattern.ReplaceAllString(cleaned, "")) + } + cleaned = strings.TrimSpace(rawLyricsInlineTimePattern.ReplaceAllString(cleaned, "")) + lower := strings.ToLower(cleaned) + if strings.HasPrefix(lower, "v1:") || strings.HasPrefix(lower, "v2:") { + cleaned = strings.TrimSpace(cleaned[3:]) + } + if cleaned != "" { + return true + } + } + + return false +} + func parseSyncedLyrics(syncedLyrics string) []LyricsLine { var lines []LyricsLine diff --git a/lib/screens/track_metadata_actions.dart b/lib/screens/track_metadata_actions.dart index c544e760..e3e4d523 100644 --- a/lib/screens/track_metadata_actions.dart +++ b/lib/screens/track_metadata_actions.dart @@ -73,16 +73,25 @@ extension _TrackMetadataFileActions on _TrackMetadataScreenState { try { final refreshed = await PlatformBridge.readFileMetadata(cleanFilePath); final refreshedLyrics = refreshed['lyrics']?.toString().trim() ?? ''; + final refreshedInstrumental = isInstrumentalLyricsMarker( + refreshedLyrics, + ); + final refreshedDisplayLyrics = cleanLyricsForDisplay(refreshedLyrics); _setState(() { _editedMetadata = refreshed; _lyricsError = null; - _isInstrumental = false; + _isInstrumental = refreshedInstrumental; _embeddedLyricsChecked = true; - if (refreshedLyrics.isNotEmpty) { - _lyrics = _cleanLrcForDisplay(refreshedLyrics); + if (refreshedDisplayLyrics.isNotEmpty) { + _lyrics = refreshedDisplayLyrics; _rawLyrics = refreshedLyrics; _lyricsSource = context.l10n.trackLyricsEmbeddedSource; _lyricsEmbedded = true; + } else if (refreshedInstrumental) { + _lyrics = null; + _rawLyrics = null; + _lyricsSource = context.l10n.trackLyricsEmbeddedSource; + _lyricsEmbedded = false; } else { _lyrics = null; _rawLyrics = null; diff --git a/lib/screens/track_metadata_lyrics.dart b/lib/screens/track_metadata_lyrics.dart index 23863600..b02c1737 100644 --- a/lib/screens/track_metadata_lyrics.dart +++ b/lib/screens/track_metadata_lyrics.dart @@ -2,6 +2,10 @@ part of 'track_metadata_screen.dart'; extension _TrackMetadataLyricsAndSaving on _TrackMetadataScreenState { Widget _buildLyricsCard(BuildContext context, ColorScheme colorScheme) { + final hasDisplayLyrics = _lyrics?.trim().isNotEmpty == true; + final hasDisplaySource = + (hasDisplayLyrics || _isInstrumental) && + _lyricsSource?.trim().isNotEmpty == true; return Card( elevation: 0, color: settingsGroupColor(context), @@ -27,7 +31,7 @@ extension _TrackMetadataLyricsAndSaving on _TrackMetadataScreenState { ), ), const Spacer(), - if (_lyrics != null) + if (hasDisplayLyrics) IconButton( icon: const Icon(Icons.copy, size: 20), onPressed: () => _copyToClipboard(context, _lyrics!), @@ -35,7 +39,7 @@ extension _TrackMetadataLyricsAndSaving on _TrackMetadataScreenState { ), ], ), - if (_lyricsSource != null && _lyricsSource!.trim().isNotEmpty) + if (hasDisplaySource) Padding( padding: const EdgeInsets.only(top: 4), child: Text( @@ -108,7 +112,7 @@ extension _TrackMetadataLyricsAndSaving on _TrackMetadataScreenState { ], ), ) - else if (_lyrics != null) + else if (hasDisplayLyrics) Column( crossAxisAlignment: CrossAxisAlignment.start, children: [ @@ -232,20 +236,26 @@ extension _TrackMetadataLyricsAndSaving on _TrackMetadataScreenState { if (mounted && generation == _metadataLoadGeneration && sourcePath == cleanFilePath) { - if (embeddedLyrics.isNotEmpty) { - final cleanLyrics = _cleanLrcForDisplay(embeddedLyrics); + final instrumental = isInstrumentalLyricsMarker(embeddedLyrics); + final cleanLyrics = cleanLyricsForDisplay(embeddedLyrics); + if (instrumental || cleanLyrics.isNotEmpty) { _setState(() { - _lyrics = cleanLyrics; - _rawLyrics = embeddedLyrics; + _lyrics = instrumental ? null : cleanLyrics; + _rawLyrics = instrumental ? null : embeddedLyrics; _lyricsSource = embeddedSource.isNotEmpty ? embeddedSource : context.l10n.trackLyricsEmbeddedSource; - _lyricsEmbedded = true; + _lyricsEmbedded = !instrumental; + _isInstrumental = instrumental; _lyricsLoading = false; _embeddedLyricsChecked = true; }); } else { _setState(() { + _lyrics = null; + _rawLyrics = null; + _lyricsSource = null; + _lyricsEmbedded = false; _lyricsLoading = false; _embeddedLyricsChecked = true; }); @@ -289,22 +299,29 @@ extension _TrackMetadataLyricsAndSaving on _TrackMetadataScreenState { final source = result['source']?.toString() ?? ''; final instrumental = (result['instrumental'] as bool? ?? false) || - lrcText == '[instrumental:true]'; + isInstrumentalLyricsMarker(lrcText); + final cleanLyrics = cleanLyricsForDisplay(lrcText); if (mounted) { if (instrumental) { _setState(() { + _lyrics = null; + _rawLyrics = null; _isInstrumental = true; _lyricsSource = source.isNotEmpty ? source : null; + _lyricsEmbedded = false; _lyricsLoading = false; }); - } else if (lrcText.isEmpty) { + } else if (cleanLyrics.isEmpty) { _setState(() { + _lyrics = null; + _rawLyrics = null; + _lyricsSource = null; + _lyricsEmbedded = false; _lyricsError = context.l10n.trackLyricsNotAvailable; _lyricsLoading = false; }); } else { - final cleanLyrics = _cleanLrcForDisplay(lrcText); _setState(() { _lyrics = cleanLyrics; _rawLyrics = lrcText; @@ -362,16 +379,18 @@ extension _TrackMetadataLyricsAndSaving on _TrackMetadataScreenState { final embeddedLyrics = embeddedResult['lyrics']?.toString() ?? ''; final embeddedSource = embeddedResult['source']?.toString() ?? ''; - if (embeddedLyrics.isNotEmpty) { + final embeddedInstrumental = isInstrumentalLyricsMarker(embeddedLyrics); + final cleanEmbeddedLyrics = cleanLyricsForDisplay(embeddedLyrics); + if (embeddedInstrumental || cleanEmbeddedLyrics.isNotEmpty) { if (mounted) { - final cleanLyrics = _cleanLrcForDisplay(embeddedLyrics); _setState(() { - _lyrics = cleanLyrics; - _rawLyrics = embeddedLyrics; + _lyrics = embeddedInstrumental ? null : cleanEmbeddedLyrics; + _rawLyrics = embeddedInstrumental ? null : embeddedLyrics; _lyricsSource = embeddedSource.isNotEmpty ? embeddedSource : context.l10n.trackLyricsEmbeddedSource; - _lyricsEmbedded = true; + _lyricsEmbedded = !embeddedInstrumental; + _isInstrumental = embeddedInstrumental; _lyricsLoading = false; _embeddedLyricsChecked = true; }); @@ -392,22 +411,29 @@ extension _TrackMetadataLyricsAndSaving on _TrackMetadataScreenState { final source = result['source']?.toString() ?? ''; final instrumental = (result['instrumental'] as bool? ?? false) || - lrcText == '[instrumental:true]'; + isInstrumentalLyricsMarker(lrcText); + final cleanLyrics = cleanLyricsForDisplay(lrcText); if (mounted) { if (instrumental) { _setState(() { + _lyrics = null; + _rawLyrics = null; _isInstrumental = true; _lyricsSource = source.isNotEmpty ? source : null; + _lyricsEmbedded = false; _lyricsLoading = false; }); - } else if (lrcText.isEmpty) { + } else if (cleanLyrics.isEmpty) { _setState(() { + _lyrics = null; + _rawLyrics = null; + _lyricsSource = null; + _lyricsEmbedded = false; _lyricsError = context.l10n.trackLyricsNotAvailable; _lyricsLoading = false; }); } else { - final cleanLyrics = _cleanLrcForDisplay(lrcText); _setState(() { _lyrics = cleanLyrics; _rawLyrics = lrcText; @@ -1208,46 +1234,4 @@ extension _TrackMetadataLyricsAndSaving on _TrackMetadataScreenState { _log.w('Failed to sync download history metadata: $e'); } } - - String _cleanLrcForDisplay(String lrc) { - final lines = lrc.split('\n'); - final cleanLines = []; - - for (final line in lines) { - var cleaned = line.trim(); - - if (_TrackMetadataScreenState._lrcMetadataPattern.hasMatch(cleaned) && - !_TrackMetadataScreenState._lrcBackgroundLinePattern.hasMatch( - cleaned, - )) { - continue; - } - - // Convert [bg:...] wrapper to a plain secondary vocal line. - final bgMatch = _TrackMetadataScreenState._lrcBackgroundLinePattern - .firstMatch(cleaned); - if (bgMatch != null) { - cleaned = bgMatch.group(1)?.trim() ?? ''; - } - - cleaned = cleaned - .replaceAll(_TrackMetadataScreenState._lrcTimestampPattern, '') - .trim(); - cleaned = cleaned.replaceAll( - _TrackMetadataScreenState._lrcInlineTimestampPattern, - '', - ); - cleaned = cleaned.replaceFirst( - _TrackMetadataScreenState._lrcSpeakerPrefixPattern, - '', - ); - cleaned = cleaned.replaceAll(RegExp(r'\s+'), ' ').trim(); - - if (cleaned.isNotEmpty) { - cleanLines.add(cleaned); - } - } - - return cleanLines.join('\n'); - } } diff --git a/lib/screens/track_metadata_screen.dart b/lib/screens/track_metadata_screen.dart index 47d6ae9f..2ffec8fc 100644 --- a/lib/screens/track_metadata_screen.dart +++ b/lib/screens/track_metadata_screen.dart @@ -138,15 +138,6 @@ class _TrackMetadataScreenState extends ConsumerState Map? _editedMetadata; String? _resolvedAudioFormat; String? _embeddedCoverPreviewPath; - static final RegExp _lrcTimestampPattern = RegExp( - r'^\[\d{2}:\d{2}\.\d{2,3}\]', - ); - static final RegExp _lrcMetadataPattern = RegExp(r'^\[[a-zA-Z]+:.*\]$'); - static final RegExp _lrcInlineTimestampPattern = RegExp( - r'<\d{2}:\d{2}\.\d{2,3}>', - ); - static final RegExp _lrcSpeakerPrefixPattern = RegExp(r'^(v1|v2):\s*'); - static final RegExp _lrcBackgroundLinePattern = RegExp(r'^\[bg:(.*)\]$'); static final RegExp _invalidFileNameChars = RegExp(r'[<>:"/\\|?*\x00-\x1f]'); static final RegExp _multiUnderscore = RegExp(r'_+'); static final RegExp _leadingOrTrailingDots = RegExp(r'^\.+|\.+$'); diff --git a/lib/utils/lyrics_metadata_helper.dart b/lib/utils/lyrics_metadata_helper.dart index 76d59223..08feab00 100644 --- a/lib/utils/lyrics_metadata_helper.dart +++ b/lib/utils/lyrics_metadata_helper.dart @@ -3,12 +3,72 @@ import 'dart:io'; import 'package:spotiflac_android/services/platform_bridge.dart'; import 'package:spotiflac_android/utils/file_access.dart'; +final RegExp _lrcDisplayTimestampPattern = RegExp( + r'^\[\d{1,3}:\d{1,2}(?:[.:]\d{1,3})?\]', +); +final RegExp _lrcDisplayMetadataPattern = RegExp( + r'^\[[a-zA-Z][a-zA-Z0-9_]*:.*\]$', +); +final RegExp _lrcDisplayInlineTimestampPattern = RegExp( + r'<\d{1,3}:\d{1,2}(?:[.:]\d{1,3})?>', +); +final RegExp _lrcDisplaySpeakerPrefixPattern = RegExp( + r'^(v1|v2):\s*', + caseSensitive: false, +); +final RegExp _lrcDisplayBackgroundLinePattern = RegExp( + r'^\[bg:(.*)\]$', + caseSensitive: false, +); + +bool isInstrumentalLyricsMarker(String lyrics) => + lyrics.trim().toLowerCase() == '[instrumental:true]'; + +/// Converts embedded or fetched LRC into text suitable for the metadata UI. +/// Header-only payloads intentionally produce an empty string. +String cleanLyricsForDisplay(String lyrics) { + final cleanLines = []; + + for (final line in lyrics.split('\n')) { + var cleaned = line.trim(); + + if (_lrcDisplayMetadataPattern.hasMatch(cleaned) && + !_lrcDisplayBackgroundLinePattern.hasMatch(cleaned)) { + continue; + } + + final backgroundMatch = _lrcDisplayBackgroundLinePattern.firstMatch( + cleaned, + ); + if (backgroundMatch != null) { + cleaned = backgroundMatch.group(1)?.trim() ?? ''; + } + + while (_lrcDisplayTimestampPattern.hasMatch(cleaned)) { + cleaned = cleaned.replaceFirst(_lrcDisplayTimestampPattern, '').trim(); + } + cleaned = cleaned.replaceAll(_lrcDisplayInlineTimestampPattern, ''); + cleaned = cleaned.replaceFirst(_lrcDisplaySpeakerPrefixPattern, ''); + cleaned = cleaned.replaceAll(RegExp(r'\s+'), ' ').trim(); + + if (cleaned.isNotEmpty) { + cleanLines.add(cleaned); + } + } + + return cleanLines.join('\n'); +} + +bool hasUsableLyricsContent(String lyrics) => + isInstrumentalLyricsMarker(lyrics) || + cleanLyricsForDisplay(lyrics).trim().isNotEmpty; + bool hasEmbeddedLyricsMetadata(Map metadata) { final lyrics = (metadata['LYRICS'] ?? '').trim(); - if (lyrics.isNotEmpty) return true; + if (hasUsableLyricsContent(lyrics)) return true; final unsyncedLyrics = (metadata['UNSYNCEDLYRICS'] ?? '').trim(); - if (unsyncedLyrics.isNotEmpty) return true; + if (hasUsableLyricsContent(unsyncedLyrics)) return true; return false; } diff --git a/test/lyrics_metadata_helper_test.dart b/test/lyrics_metadata_helper_test.dart new file mode 100644 index 00000000..aa1a9de3 --- /dev/null +++ b/test/lyrics_metadata_helper_test.dart @@ -0,0 +1,43 @@ +import 'package:flutter_test/flutter_test.dart'; +import 'package:spotiflac_android/utils/lyrics_metadata_helper.dart'; + +void main() { + group('lyrics display normalization', () { + test('rejects metadata-only embedded LRC', () { + const raw = ''' +[ti:Banna Re] +[ar:Dhvani Bhanushali] +[al:Banna Re] +[by:SpotiFLAC Mobile] +'''; + + expect(cleanLyricsForDisplay(raw), isEmpty); + expect(hasUsableLyricsContent(raw), isFalse); + expect( + hasEmbeddedLyricsMetadata({'LYRICS': raw, 'UNSYNCEDLYRICS': raw}), + isFalse, + ); + }); + + test('keeps timed, background, and speaker lyric content', () { + const raw = ''' +[ti:Song] +[00:01.25]Lead line +[bg:Background line] +[00:02:500]<00:02.500>v2: Harmony line +'''; + + expect( + cleanLyricsForDisplay(raw), + 'Lead line\nBackground line\nHarmony line', + ); + expect(hasUsableLyricsContent(raw), isTrue); + }); + + test('recognizes the instrumental marker without rendering it as text', () { + expect(isInstrumentalLyricsMarker(' [Instrumental:TRUE] '), isTrue); + expect(hasUsableLyricsContent('[instrumental:true]'), isTrue); + expect(cleanLyricsForDisplay('[instrumental:true]'), isEmpty); + }); + }); +}