fix(lyrics): reject empty embedded payloads

This commit is contained in:
zarzet
2026-08-01 15:02:03 +07:00
parent 3e41aa0eee
commit dc58353c07
8 changed files with 243 additions and 79 deletions
+4 -4
View File
@@ -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)
}
@@ -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
+47
View File
@@ -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
+12 -3
View File
@@ -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;
+45 -61
View File
@@ -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 = <String>[];
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');
}
}
-9
View File
@@ -138,15 +138,6 @@ class _TrackMetadataScreenState extends ConsumerState<TrackMetadataScreen>
Map<String, dynamic>? _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'^\.+|\.+$');
+62 -2
View File
@@ -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 = <String>[];
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<String, String> 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;
}
+43
View File
@@ -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);
});
});
}