From 15ffb3abf5152ba1d919adef18f2f85c6df53805 Mon Sep 17 00:00:00 2001 From: zarzet <42882290+zarzet@users.noreply.github.com> Date: Fri, 4 Sep 2026 16:55:02 +0700 Subject: [PATCH] feat(metadata): add safe batch editing and matching --- go_backend/ape_tags.go | 5 +- go_backend/ape_tags_supplement_test.go | 9 ++ go_backend/audio_metadata.go | 3 + go_backend/audio_metadata_ogg.go | 2 +- go_backend/audio_metadata_supplement_test.go | 25 +++- go_backend/extension_fallback.go | 45 +++---- go_backend/m4a_edit.go | 7 ++ go_backend/metadata.go | 28 ++--- go_backend/metadata_artist_tags_test.go | 14 +++ go_backend/metadata_m4a.go | 2 +- go_backend/native_tag_editors_test.go | 13 ++- go_backend/title_match_utils.go | 72 ++++++++++++ go_backend/title_match_utils_test.go | 83 +++++++++++++ lib/l10n/arb/app_en.arb | 24 ++++ lib/l10n/arb/app_id.arb | 6 + lib/screens/local_album_screen.dart | 11 +- lib/screens/queue_tab_batch_actions.dart | 11 +- lib/services/batch_metadata_re_enrich.dart | 64 +++++++++- lib/services/library_database_queue_sql.dart | 18 ++- lib/widgets/re_enrich_field_dialog.dart | 116 ++++++++++++++++++- test/batch_metadata_re_enrich_test.dart | 56 +++++++++ test/models_and_utils_test.dart | 15 +++ 22 files changed, 578 insertions(+), 51 deletions(-) diff --git a/go_backend/ape_tags.go b/go_backend/ape_tags.go index fbc06e03..c96f0b28 100644 --- a/go_backend/ape_tags.go +++ b/go_backend/ape_tags.go @@ -367,7 +367,7 @@ func APETagToAudioMetadata(tag *APETag) *AudioMetadata { metadata.DiscNumber, metadata.TotalDiscs = parseIndexPair(value) case "ISRC": metadata.ISRC = value - case "LYRICS", "UNSYNCEDLYRICS": + case "LYRICS", "UNSYNCEDLYRICS", "SYNCEDLYRICS": if metadata.Lyrics == "" { metadata.Lyrics = value } @@ -491,7 +491,7 @@ func apeKeysFromFields(fields map[string]string) map[string]struct{} { // Some fields have reader aliases that must also be cleared when the // canonical key is updated (e.g. DATE writer ↔ DATE/YEAR reader, // DISC ↔ DISCNUMBER, TRACK ↔ TRACKNUMBER, "ALBUM ARTIST" ↔ ALBUMARTIST, - // LABEL ↔ PUBLISHER, LYRICS ↔ UNSYNCEDLYRICS). + // LABEL ↔ PUBLISHER, and the supported lyrics aliases). if _, present := fields["date"]; present { result["DATE"] = struct{}{} } @@ -515,6 +515,7 @@ func apeKeysFromFields(fields map[string]string) map[string]struct{} { } if _, present := fields["lyrics"]; present { result["UNSYNCEDLYRICS"] = struct{}{} + result["SYNCEDLYRICS"] = struct{}{} } return result } diff --git a/go_backend/ape_tags_supplement_test.go b/go_backend/ape_tags_supplement_test.go index 971d8e18..74e6bc56 100644 --- a/go_backend/ape_tags_supplement_test.go +++ b/go_backend/ape_tags_supplement_test.go @@ -81,6 +81,15 @@ func TestAPETagReadWriteMergeAndMetadataConversion(t *testing.T) { if mergedMeta.Lyrics != "" { t.Fatalf("expected lyrics cleared, got %q", mergedMeta.Lyrics) } + if _, ok := override["SYNCEDLYRICS"]; !ok { + t.Fatal("lyrics edit must also clear the SYNCEDLYRICS alias") + } + if synced := APETagToAudioMetadata(&APETag{Items: []APETagItem{{ + Key: "SYNCEDLYRICS", + Value: "[00:01.00]Synced APE lyrics", + }}}); synced.Lyrics != "[00:01.00]Synced APE lyrics" { + t.Fatalf("APE SYNCEDLYRICS = %q", synced.Lyrics) + } if err := WriteAPETags(path, &APETag{Items: []APETagItem{{Key: "Title", Value: "Replacement"}}}); err != nil { t.Fatalf("replace APE tags: %v", err) diff --git a/go_backend/audio_metadata.go b/go_backend/audio_metadata.go index 8106e9ed..ee963fef 100644 --- a/go_backend/audio_metadata.go +++ b/go_backend/audio_metadata.go @@ -496,7 +496,10 @@ func isLyricsDescription(description string) bool { "lyric", "unsyncedlyrics", "unsynced lyrics", + "syncedlyrics", + "synced lyrics", "uslt", + "sylt", "lrc": return true default: diff --git a/go_backend/audio_metadata_ogg.go b/go_backend/audio_metadata_ogg.go index fdf95f7a..81afcb7c 100644 --- a/go_backend/audio_metadata_ogg.go +++ b/go_backend/audio_metadata_ogg.go @@ -260,7 +260,7 @@ func parseVorbisComments(data []byte, metadata *AudioMetadata) { metadata.Composer = value case "COMMENT", "DESCRIPTION": metadata.Comment = value - case "LYRICS", "UNSYNCEDLYRICS": + case "LYRICS", "UNSYNCEDLYRICS", "SYNCEDLYRICS": if metadata.Lyrics == "" { metadata.Lyrics = value } diff --git a/go_backend/audio_metadata_supplement_test.go b/go_backend/audio_metadata_supplement_test.go index c39add38..27723644 100644 --- a/go_backend/audio_metadata_supplement_test.go +++ b/go_backend/audio_metadata_supplement_test.go @@ -100,6 +100,9 @@ func TestAudioMetadataID3ParsingBranches(t *testing.T) { if got := syncsafeToInt([]byte{0, 0, 2, 0}); got != 256 { t.Fatalf("syncsafe = %d", got) } + if !isLyricsDescription("SYNCEDLYRICS") || !isLyricsDescription("SYLT") { + t.Fatal("synced lyrics descriptions must be recognized") + } } func TestAudioMetadataCoverAndQualityHelpers(t *testing.T) { @@ -276,6 +279,23 @@ func TestM4AMetadataAtomHelpers(t *testing.T) { t.Fatal("expected missing M4A lyrics error") } + syncedM4A := filepath.Join(dir, "synced.m4a") + syncedIlst := buildM4AFreeformAtom( + "SYNCEDLYRICS", + "[00:01.00]M4A synced lyrics", + ) + if err := os.WriteFile( + syncedM4A, + buildM4AFileWithIlst(syncedIlst, true), + 0600, + ); err != nil { + t.Fatal(err) + } + if synced, err := ReadM4ATags(syncedM4A); err != nil || + synced.Lyrics != "[00:01.00]M4A synced lyrics" { + t.Fatalf("M4A SYNCEDLYRICS = %#v/%v", synced, err) + } + sidecarAudio := filepath.Join(dir, "sidecar.mp3") if err := os.WriteFile(sidecarAudio, []byte("audio"), 0600); err != nil { t.Fatal(err) @@ -392,7 +412,7 @@ func TestOggMetadataQualityAndCoverHelpers(t *testing.T) { "ALBUMARTIST=Album Artist", "TRACKNUMBER=2/9", "DISCNUMBER=1/2", - "LYRICS=[00:00.00]Ogg Lyrics", + "SYNCEDLYRICS=[00:00.00]Ogg Lyrics", "ITUNESADVISORY=1", "RELEASETYPE=ep", "BARCODE=4006381333931", @@ -416,6 +436,9 @@ func TestOggMetadataQualityAndCoverHelpers(t *testing.T) { if err != nil || meta.Title != "Ogg Title" || meta.TrackNumber != 2 || meta.TotalTracks != 9 { t.Fatalf("ReadOggVorbisComments = %#v/%v", meta, err) } + if meta.Lyrics != "[00:00.00]Ogg Lyrics" { + t.Fatalf("Ogg SYNCEDLYRICS = %q", meta.Lyrics) + } if !meta.Explicit || meta.AlbumType != "ep" || meta.UPC != "4006381333931" { t.Fatalf("Ogg release identity = %#v", meta) } diff --git a/go_backend/extension_fallback.go b/go_backend/extension_fallback.go index 841e1e2d..7a7eeafe 100644 --- a/go_backend/extension_fallback.go +++ b/go_backend/extension_fallback.go @@ -517,32 +517,35 @@ func DownloadWithExtensionFallback(req DownloadRequest) (*DownloadResponse, erro searchQuery := req.TrackName + " " + req.ArtistName GoLog("[DownloadWithExtensionFallback] Metadata incomplete, searching providers for: %s\n", searchQuery) - // Only the first match is consumed below. Asking for five made the manager - // continue through additional providers even after it already had a usable - // match, multiplying the per-provider timeout on slow networks. - tracks, searchErr := extManager.SearchTracksWithMetadataProvidersForItemID(searchQuery, 1, true, req.ItemID) + // Inspect several candidates: the first search result can be an unrelated + // same-title recording, remix, or cover. + tracks, searchErr := extManager.SearchTracksWithMetadataProvidersForItemID(searchQuery, 5, true, req.ItemID) if shouldAbortCancelledFallback(req.ItemID, searchErr) { return nil, ErrDownloadCancelled } if searchErr == nil && len(tracks) > 0 { - track := tracks[0] - GoLog("[DownloadWithExtensionFallback] Metadata match (%s): %s - %s (album: %s, date: %s, isrc: %s)\n", - track.ProviderID, track.Name, track.Artists, track.AlbumName, track.ReleaseDate, track.ISRC) + track := selectBestMetadataEnrichmentTrack(req, tracks) + if track == nil { + GoLog("[DownloadWithExtensionFallback] No confident metadata match; preserving source metadata\n") + } else { + GoLog("[DownloadWithExtensionFallback] Metadata match (%s): %s - %s (album: %s, date: %s, isrc: %s)\n", + track.ProviderID, track.Name, track.Artists, track.AlbumName, track.ReleaseDate, track.ISRC) - overlayStr(&req.AlbumName, track.AlbumName, "") - overlayStr(&req.AlbumArtist, track.AlbumArtist, "") - overlayStr(&req.ReleaseDate, track.ReleaseDate, "") - overlayStr(&req.ISRC, track.ISRC, "") - overlayInt(&req.TrackNumber, track.TrackNumber, "") - overlayInt(&req.TotalTracks, track.TotalTracks, "") - overlayInt(&req.DiscNumber, track.DiscNumber, "") - overlayInt(&req.TotalDiscs, track.TotalDiscs, "") - overlayStr(&req.Composer, track.Composer, "") - overlayStr(&req.CoverURL, track.CoverURL, "") - overlayStr(&req.Genre, track.Genre, "") - overlayStr(&req.Label, track.Label, "") - overlayStr(&req.Copyright, track.Copyright, "") - overlayExtensionReleaseMetadata(&req, track) + overlayStr(&req.AlbumName, track.AlbumName, "") + overlayStr(&req.AlbumArtist, track.AlbumArtist, "") + overlayStr(&req.ReleaseDate, track.ReleaseDate, "") + overlayStr(&req.ISRC, track.ISRC, "") + overlayInt(&req.TrackNumber, track.TrackNumber, "") + overlayInt(&req.TotalTracks, track.TotalTracks, "") + overlayInt(&req.DiscNumber, track.DiscNumber, "") + overlayInt(&req.TotalDiscs, track.TotalDiscs, "") + overlayStr(&req.Composer, track.Composer, "") + overlayStr(&req.CoverURL, track.CoverURL, "") + overlayStr(&req.Genre, track.Genre, "") + overlayStr(&req.Label, track.Label, "") + overlayStr(&req.Copyright, track.Copyright, "") + overlayExtensionReleaseMetadata(&req, *track) + } } else if searchErr != nil { GoLog("[DownloadWithExtensionFallback] Metadata provider search failed (non-fatal): %v\n", searchErr) } diff --git a/go_backend/m4a_edit.go b/go_backend/m4a_edit.go index 13716e40..489f5aa7 100644 --- a/go_backend/m4a_edit.go +++ b/go_backend/m4a_edit.go @@ -275,6 +275,13 @@ func EditM4AFields(filePath string, fields map[string]string) error { removeFreeform["ORGANIZATION"] = struct{}{} freeformTags = append(freeformTags, m4aFreeformTag{name: "LABEL", value: strings.TrimSpace(fields["label"])}) } + if _, ok := fields["lyrics"]; ok { + // The canonical iTunes lyrics atom is written above. Remove custom + // aliases so an older synced value cannot survive a replace/clear edit. + removeFreeform["LYRICS"] = struct{}{} + removeFreeform["UNSYNCEDLYRICS"] = struct{}{} + removeFreeform["SYNCEDLYRICS"] = struct{}{} + } if v, ok := fields["album_type"]; ok { removeFreeform["RELEASETYPE"] = struct{}{} freeformTags = append(freeformTags, m4aFreeformTag{name: "RELEASETYPE", value: strings.TrimSpace(v)}) diff --git a/go_backend/metadata.go b/go_backend/metadata.go index dd859ece..c143ee65 100644 --- a/go_backend/metadata.go +++ b/go_backend/metadata.go @@ -345,10 +345,7 @@ func metadataFromParsedFlac(f *flac.File) *Metadata { metadata.ISRC = getComment(cmt, "ISRC") metadata.Description = getComment(cmt, "DESCRIPTION") - metadata.Lyrics = getComment(cmt, "LYRICS") - if metadata.Lyrics == "" { - metadata.Lyrics = getComment(cmt, "UNSYNCEDLYRICS") - } + metadata.Lyrics = getLyricsComment(cmt) trackNum := getComment(cmt, "TRACKNUMBER") if trackNum != "" { @@ -530,8 +527,10 @@ func applyVorbisFieldEdits(cmt *flacvorbis.MetaDataBlockVorbisComment, fields ma removeCommentKey(cmt, "DISC") // alias } - // Lyrics: set both LYRICS + UNSYNCEDLYRICS, or clear both. + // Lyrics: set the broadly-supported plain aliases and remove any stale + // SYNCEDLYRICS value, or clear every alias. if v, ok := fields["lyrics"]; ok { + removeCommentKey(cmt, "SYNCEDLYRICS") if v != "" { setOrClearComment(cmt, "LYRICS", v) setOrClearComment(cmt, "UNSYNCEDLYRICS", v) @@ -713,6 +712,15 @@ func getComment(cmt *flacvorbis.MetaDataBlockVorbisComment, key string) string { return values[0] } +func getLyricsComment(cmt *flacvorbis.MetaDataBlockVorbisComment) string { + for _, key := range []string{"LYRICS", "UNSYNCEDLYRICS", "SYNCEDLYRICS"} { + if lyrics := getComment(cmt, key); strings.TrimSpace(lyrics) != "" { + return lyrics + } + } + return "" +} + func getJoinedComment(cmt *flacvorbis.MetaDataBlockVorbisComment, key string) string { return joinVorbisCommentValues(getCommentValues(cmt, key)) } @@ -944,14 +952,8 @@ func extractLyricsFromFlac(filePath string) (string, error) { continue } - lyrics, err := cmt.Get("LYRICS") - if err == nil && len(lyrics) > 0 && strings.TrimSpace(lyrics[0]) != "" { - return lyrics[0], nil - } - - lyrics, err = cmt.Get("UNSYNCEDLYRICS") - if err == nil && len(lyrics) > 0 && strings.TrimSpace(lyrics[0]) != "" { - return lyrics[0], nil + if lyrics := getLyricsComment(cmt); lyrics != "" { + return lyrics, nil } } diff --git a/go_backend/metadata_artist_tags_test.go b/go_backend/metadata_artist_tags_test.go index 32ea5441..a829030c 100644 --- a/go_backend/metadata_artist_tags_test.go +++ b/go_backend/metadata_artist_tags_test.go @@ -54,6 +54,20 @@ func TestParseVorbisCommentsJoinsRepeatedArtists(t *testing.T) { } } +func TestGetLyricsCommentReadsAndClearsSyncedLyrics(t *testing.T) { + cmt := flacvorbis.New() + setComment(cmt, "SYNCEDLYRICS", "[00:01.00]Synced line") + + if got := getLyricsComment(cmt); got != "[00:01.00]Synced line" { + t.Fatalf("getLyricsComment() = %q", got) + } + + applyVorbisFieldEdits(cmt, map[string]string{"lyrics": ""}) + if got := getComment(cmt, "SYNCEDLYRICS"); got != "" { + t.Fatalf("cleared SYNCEDLYRICS = %q", got) + } +} + func buildVorbisCommentPayload(comments []string) []byte { var buf bytes.Buffer _ = binary.Write(&buf, binary.LittleEndian, uint32(len("spotiflac"))) diff --git a/go_backend/metadata_m4a.go b/go_backend/metadata_m4a.go index bbd788f8..8459cca4 100644 --- a/go_backend/metadata_m4a.go +++ b/go_backend/metadata_m4a.go @@ -101,7 +101,7 @@ func readM4ATagsFromIlst(f *os.File, fileSize int64, ilst atomHeader) (*AudioMet if metadata.Copyright == "" { metadata.Copyright = value } - case "LYRICS", "UNSYNCEDLYRICS": + case "LYRICS", "UNSYNCEDLYRICS", "SYNCEDLYRICS": if metadata.Lyrics == "" { metadata.Lyrics = value } diff --git a/go_backend/native_tag_editors_test.go b/go_backend/native_tag_editors_test.go index 94af5c1d..f9a1c34f 100644 --- a/go_backend/native_tag_editors_test.go +++ b/go_backend/native_tag_editors_test.go @@ -221,6 +221,7 @@ func TestEditM4AFieldsPreservesAtomsAndShiftsChunkOffsets(t *testing.T) { existing := append([]byte{}, buildM4ATextAtom("\xa9nam", "Old")...) existing = append(existing, buildM4ATextAtom("\xa9too", "SomeEncoder")...) // foreign, untouched existing = append(existing, buildM4AFreeformAtom("MusicBrainz Track Id", "xyz")...) + existing = append(existing, buildM4AFreeformAtom("SYNCEDLYRICS", "Old synced lyrics")...) mdatPayload := []byte("M4ADATA") file, oldOffset := buildTestM4A(t, existing, mdatPayload) @@ -230,8 +231,9 @@ func TestEditM4AFieldsPreservesAtomsAndShiftsChunkOffsets(t *testing.T) { } if err := EditM4AFields(path, map[string]string{ - "title": "A Much Longer Replacement Title", - "isrc": "USABC1234567", + "title": "A Much Longer Replacement Title", + "isrc": "USABC1234567", + "lyrics": "Updated lyrics", }); err != nil { t.Fatalf("EditM4AFields: %v", err) } @@ -249,6 +251,13 @@ func TestEditM4AFieldsPreservesAtomsAndShiftsChunkOffsets(t *testing.T) { if !bytes.Contains(updated, []byte("USABC1234567")) { t.Error("ISRC freeform missing") } + if bytes.Contains(updated, []byte("SYNCEDLYRICS")) || + bytes.Contains(updated, []byte("Old synced lyrics")) { + t.Error("stale SYNCEDLYRICS freeform was not removed") + } + if meta, err := ReadM4ATags(path); err != nil || meta.Lyrics != "Updated lyrics" { + t.Fatalf("updated M4A lyrics = %#v/%v", meta, err) + } // stco entry must still point at the mdat payload. idx := bytes.Index(updated, []byte("stco")) diff --git a/go_backend/title_match_utils.go b/go_backend/title_match_utils.go index 8690b16b..388368d8 100644 --- a/go_backend/title_match_utils.go +++ b/go_backend/title_match_utils.go @@ -458,3 +458,75 @@ func trackMatchesRequest(req DownloadRequest, resolved resolvedTrackInfo, logPre return true } + +// selectBestMetadataEnrichmentTrack only returns a provider result when it is +// safe to copy missing tags into a download request. Search ordering alone is +// not evidence of identity: providers can put covers, remixes, or unrelated +// same-title recordings first. +func selectBestMetadataEnrichmentTrack(req DownloadRequest, tracks []ExtTrackMetadata) *ExtTrackMetadata { + var best *ExtTrackMetadata + bestScore := -1 << 30 + expectedISRC := strings.TrimSpace(req.ISRC) + + for i := range tracks { + track := &tracks[i] + candidateISRC := strings.TrimSpace(track.ISRC) + exactISRCMatch := expectedISRC != "" && candidateISRC != "" && + strings.EqualFold(expectedISRC, candidateISRC) + if expectedISRC != "" && candidateISRC != "" && !exactISRCMatch { + GoLog("[MetadataEnrichment] Rejected %s result with conflicting ISRC %s\n", track.ProviderID, candidateISRC) + continue + } + + resolved := resolvedTrackInfo{ + Title: track.Name, + ArtistName: track.Artists, + AlbumName: track.AlbumName, + ISRC: track.ISRC, + Duration: track.DurationMS / 1000, + } + if !trackMatchesRequest(req, resolved, "MetadataEnrichment") { + continue + } + if !exactISRCMatch && !hasStrongTrackIdentity(req, resolved) { + GoLog("[MetadataEnrichment] Rejected low-confidence result: %s - %s\n", track.Name, track.Artists) + continue + } + + score := 2000 + if exactISRCMatch { + score += 10000 + } + if exactLooseIdentityMatch(req.TrackName, track.Name, normalizeLooseTitle) { + score += 400 + } + if exactLooseIdentityMatch(req.ArtistName, track.Artists, normalizeLooseArtistName) { + score += 320 + } + if req.AlbumName != "" && track.AlbumName != "" && titlesMatch(req.AlbumName, track.AlbumName) { + score += 120 + } + if durationMatchesRequest(req, resolved) { + score += 80 + } + if track.ISRC != "" { + score += 40 + } + if track.AlbumName != "" { + score += 30 + } + if track.ReleaseDate != "" { + score += 30 + } + if track.TrackNumber > 0 { + score += 10 + } + + if best == nil || score > bestScore { + best = track + bestScore = score + } + } + + return best +} diff --git a/go_backend/title_match_utils_test.go b/go_backend/title_match_utils_test.go index 8a8b0974..0bf865da 100644 --- a/go_backend/title_match_utils_test.go +++ b/go_backend/title_match_utils_test.go @@ -179,3 +179,86 @@ func TestTitlesMatch_EmojiStrict(t *testing.T) { t.Fatal("expected identical emoji titles to match") } } + +func TestSelectBestMetadataEnrichmentTrackSkipsWrongFirstResult(t *testing.T) { + req := DownloadRequest{ + TrackName: "Song", + ArtistName: "Original Artist", + DurationMS: 180000, + } + tracks := []ExtTrackMetadata{ + { + Name: "Song", + Artists: "Cover Band", + AlbumName: "Covers", + DurationMS: 180000, + ProviderID: "first", + }, + { + Name: "Song", + Artists: "Original Artist", + AlbumName: "Original Album", + ReleaseDate: "2026-01-01", + ISRC: "USAAA2600001", + DurationMS: 180000, + ProviderID: "second", + }, + } + + best := selectBestMetadataEnrichmentTrack(req, tracks) + if best == nil || best.ProviderID != "second" { + t.Fatalf("best metadata match = %#v, want second result", best) + } +} + +func TestSelectBestMetadataEnrichmentTrackRejectsConflictingISRC(t *testing.T) { + req := DownloadRequest{ + TrackName: "Song", + ArtistName: "Artist", + ISRC: "USAAA2600001", + } + tracks := []ExtTrackMetadata{{ + Name: "Song", + Artists: "Artist", + AlbumName: "Album", + ISRC: "USAAA2600002", + ProviderID: "provider", + }} + + if best := selectBestMetadataEnrichmentTrack(req, tracks); best != nil { + t.Fatalf("expected conflicting ISRC to be rejected, got %#v", best) + } +} + +func TestSelectBestMetadataEnrichmentTrackAcceptsExactISRC(t *testing.T) { + req := DownloadRequest{ + TrackName: "Localized Song Name", + ArtistName: "Localized Artist Name", + ISRC: "USAAA2600001", + } + tracks := []ExtTrackMetadata{{ + Name: "Original Song Name", + Artists: "Original Artist Name", + AlbumName: "Album", + ISRC: "usaaa2600001", + ProviderID: "provider", + }} + + if best := selectBestMetadataEnrichmentTrack(req, tracks); best == nil { + t.Fatal("expected exact ISRC to provide a confident metadata match") + } +} + +func TestSelectBestMetadataEnrichmentTrackRejectsWeakArtistMatch(t *testing.T) { + req := DownloadRequest{TrackName: "Song", ArtistName: "Artist"} + tracks := []ExtTrackMetadata{{ + Name: "Song", + Artists: "Artist feat. Someone Else", + AlbumName: "Album", + ProviderID: "provider", + }} + + if best := selectBestMetadataEnrichmentTrack(req, tracks); best != nil { + t.Fatalf("expected fuzzy artist match without duration to be rejected, got %#v", best) + } +} diff --git a/lib/l10n/arb/app_en.arb b/lib/l10n/arb/app_en.arb index da7254fa..f821aa62 100644 --- a/lib/l10n/arb/app_en.arb +++ b/lib/l10n/arb/app_en.arb @@ -3198,6 +3198,10 @@ "@trackReEnrichOnlineSubtitle": { "description": "Subtitle for re-enrich metadata action for local items" }, + "trackReEnrichBatchSubtitle": "Choose an online or manual update, then review every change before writing", + "@trackReEnrichBatchSubtitle": { + "description": "Subtitle for batch metadata update choices" + }, "trackReEnrichFieldCover": "Cover Art", "@trackReEnrichFieldCover": { "description": "Checkbox label for cover art field in re-enrich" @@ -3250,6 +3254,22 @@ "@trackReEnrichModeReplaceSubtitle": { "description": "Explanation for the selected-tag batch metadata mode" }, + "trackReEnrichModeManual": "Set common values", + "@trackReEnrichModeManual": { + "description": "Batch metadata mode that applies shared values to every selected track" + }, + "trackReEnrichModeManualSubtitle": "Apply the same values to every selected track", + "@trackReEnrichModeManualSubtitle": { + "description": "Explanation for the manual shared-value batch metadata mode" + }, + "trackReEnrichManualFieldsTitle": "Values to apply", + "@trackReEnrichManualFieldsTitle": { + "description": "Heading above manual batch metadata inputs" + }, + "trackReEnrichManualHint": "Leave a field empty to keep its current value. Track titles, numbers, and ISRCs stay unchanged.", + "@trackReEnrichManualHint": { + "description": "Safety explanation for manual batch metadata inputs" + }, "trackReEnrichFieldsTitle": "Tags to update", "@trackReEnrichFieldsTitle": { "description": "Heading above batch re-enrich field checkboxes" @@ -3316,6 +3336,10 @@ "@trackReEnrichSearching": { "description": "Snackbar while searching metadata from internet for local items" }, + "trackReEnrichPreparing": "Preparing metadata changes...", + "@trackReEnrichPreparing": { + "description": "Progress message while preparing manual batch metadata changes" + }, "trackReEnrichSuccess": "Metadata re-enriched successfully", "@trackReEnrichSuccess": { "description": "Snackbar after successful re-enrichment" diff --git a/lib/l10n/arb/app_id.arb b/lib/l10n/arb/app_id.arb index d34cca23..7e44eb95 100644 --- a/lib/l10n/arb/app_id.arb +++ b/lib/l10n/arb/app_id.arb @@ -720,6 +720,10 @@ "trackReEnrichModeMissingSubtitle": "Keep existing values and fill only fields that are empty", "trackReEnrichModeReplace": "Update selected tags", "trackReEnrichModeReplaceSubtitle": "Choose which existing values may be replaced by online metadata", + "trackReEnrichModeManual": "Tetapkan nilai yang sama", + "trackReEnrichModeManualSubtitle": "Terapkan nilai yang sama ke setiap lagu yang dipilih", + "trackReEnrichManualFieldsTitle": "Nilai yang akan diterapkan", + "trackReEnrichManualHint": "Biarkan kolom kosong untuk mempertahankan nilai saat ini. Judul, nomor lagu, dan ISRC tidak akan diubah.", "trackReEnrichFieldsTitle": "Tags to update", "trackReEnrichReview": "Review changes", "trackReEnrichReviewTitle": "Review metadata changes", @@ -2288,6 +2292,7 @@ "@trackReEnrichOnlineSubtitle": { "description": "Subtitle for re-enrich metadata action for local items" }, + "trackReEnrichBatchSubtitle": "Pilih pembaruan online atau manual, lalu tinjau setiap perubahan sebelum ditulis", "nowPlayingNothingPlaying": "Nothing is playing", "@nowPlayingNothingPlaying": { "description": "Empty state when no track is currently playing" @@ -5811,6 +5816,7 @@ "lyricsProviderLyricsPlusDesc": "Word-by-word karaoke lyrics (Apple/Musixmatch/Spotify/QQ, via proxy)", "dialogSave": "Simpan", "trackReEnrichSearching": "Searching metadata online...", + "trackReEnrichPreparing": "Menyiapkan perubahan metadata...", "regionCountryUS": "United States", "audioAnalysisDescription": "Verify lossless quality with spectrum analysis", "downloadFilenameFormat": "Format Nama File", diff --git a/lib/screens/local_album_screen.dart b/lib/screens/local_album_screen.dart index 30fd52c6..40de9f27 100644 --- a/lib/screens/local_album_screen.dart +++ b/lib/screens/local_album_screen.dart @@ -640,9 +640,11 @@ class _LocalAlbumScreenState extends ConsumerState var cancelled = false; BatchProgressDialog.show( context: context, - title: context.l10n.trackReEnrichSearching, + title: selection.usesManualValues + ? context.l10n.trackReEnrichPreparing + : context.l10n.trackReEnrichSearching, total: selected.length, - icon: Icons.manage_search, + icon: selection.usesManualValues ? Icons.edit_note : Icons.manage_search, onCancel: () { cancelled = true; BatchProgressDialog.dismiss(context); @@ -658,6 +660,11 @@ class _LocalAlbumScreenState extends ConsumerState ); final updateFields = selection.updateFieldsFor(item); if (updateFields.isEmpty) continue; + if (selection.usesManualValues) { + final preview = buildManualBatchReEnrichPreview(item, selection); + if (preview != null) previews.add(preview); + continue; + } try { final result = await PlatformBridge.reEnrichFile( buildBatchReEnrichRequest( diff --git a/lib/screens/queue_tab_batch_actions.dart b/lib/screens/queue_tab_batch_actions.dart index 678e51cd..c9e7c553 100644 --- a/lib/screens/queue_tab_batch_actions.dart +++ b/lib/screens/queue_tab_batch_actions.dart @@ -219,9 +219,11 @@ extension _QueueTabBatchActions on _QueueTabState { var cancelled = false; BatchProgressDialog.show( context: context, - title: context.l10n.trackReEnrichSearching, + title: selection.usesManualValues + ? context.l10n.trackReEnrichPreparing + : context.l10n.trackReEnrichSearching, total: selectedLocalItems.length, - icon: Icons.manage_search, + icon: selection.usesManualValues ? Icons.edit_note : Icons.manage_search, onCancel: () { cancelled = true; BatchProgressDialog.dismiss(context); @@ -237,6 +239,11 @@ extension _QueueTabBatchActions on _QueueTabState { ); final updateFields = selection.updateFieldsFor(item); if (updateFields.isEmpty) continue; + if (selection.usesManualValues) { + final preview = buildManualBatchReEnrichPreview(item, selection); + if (preview != null) previews.add(preview); + continue; + } try { final result = await PlatformBridge.reEnrichFile( buildBatchReEnrichRequest( diff --git a/lib/services/batch_metadata_re_enrich.dart b/lib/services/batch_metadata_re_enrich.dart index 6046109c..3cb95edd 100644 --- a/lib/services/batch_metadata_re_enrich.dart +++ b/lib/services/batch_metadata_re_enrich.dart @@ -20,13 +20,34 @@ class ReEnrichFields { ]; } -enum ReEnrichBatchMode { isrcOnly, missingOnly, selectedFields } +enum ReEnrichBatchMode { isrcOnly, missingOnly, selectedFields, manualValues } + +/// Tags where applying one shared value to multiple tracks is normally safe. +/// Per-track identifiers, titles, and track/disc numbers are deliberately +/// excluded so the batch editor cannot accidentally duplicate them. +const List manualBatchMetadataFields = [ + 'artist_name', + 'album_name', + 'album_artist', + 'release_date', + 'genre', + 'composer', + 'label', + 'copyright', +]; class ReEnrichFieldSelection { final ReEnrichBatchMode mode; final List fields; + final Map manualValues; - const ReEnrichFieldSelection({required this.mode, this.fields = const []}); + const ReEnrichFieldSelection({ + required this.mode, + this.fields = const [], + this.manualValues = const {}, + }); + + bool get usesManualValues => mode == ReEnrichBatchMode.manualValues; List updateFieldsFor(LocalLibraryItem item) { switch (mode) { @@ -36,6 +57,15 @@ class ReEnrichFieldSelection { return fields; case ReEnrichBatchMode.missingOnly: return missingReEnrichFields(item); + case ReEnrichBatchMode.manualValues: + return manualValues.entries + .where( + (entry) => + manualBatchMetadataFields.contains(entry.key) && + entry.value.trim().isNotEmpty, + ) + .map((entry) => entry.key) + .toList(growable: false); } } } @@ -138,6 +168,36 @@ class BatchReEnrichPreview { }); } +BatchReEnrichPreview? buildManualBatchReEnrichPreview( + LocalLibraryItem item, + ReEnrichFieldSelection selection, +) { + if (!selection.usesManualValues) return null; + + final enrichedMetadata = { + for (final entry in selection.manualValues.entries) + if (manualBatchMetadataFields.contains(entry.key) && + entry.value.trim().isNotEmpty) + entry.key: entry.value.trim(), + }; + final updateFields = enrichedMetadata.keys.toList(growable: false); + if (updateFields.isEmpty) return null; + + final changes = buildReEnrichMetadataChanges( + item, + enrichedMetadata, + updateFields, + ); + if (changes.isEmpty) return null; + + return BatchReEnrichPreview( + item: item, + updateFields: updateFields, + enrichedMetadata: enrichedMetadata, + changes: changes, + ); +} + String _displayValue(Object? value) { if (value == null) return ''; if (value is num && value == 0) return ''; diff --git a/lib/services/library_database_queue_sql.dart b/lib/services/library_database_queue_sql.dart index 78753976..d46c3426 100644 --- a/lib/services/library_database_queue_sql.dart +++ b/lib/services/library_database_queue_sql.dart @@ -2,6 +2,11 @@ part of 'library_database.dart'; // SQL builders for the queue tab's history+local union queries. +String confirmedMissingLyricsSqlPredicate({ + required String hasLyricsExpr, + required String lyricsKnownExpr, +}) => '($lyricsKnownExpr) AND COALESCE($hasLyricsExpr, 0) = 0'; + class _QueueOrderTerm { final String column; final bool descending; @@ -398,6 +403,7 @@ extension _LibraryDbQueueSql on LibraryDatabase { isrcExpr: 'h.isrc', labelExpr: 'h.label', hasLyricsExpr: 'h.has_lyrics', + lyricsKnownExpr: 'COALESCE(h.lyrics_metadata_scan_version, 0) >= 1', ); } @@ -441,6 +447,8 @@ extension _LibraryDbQueueSql on LibraryDatabase { isrcExpr: 'l.isrc', labelExpr: 'l.label', hasLyricsExpr: 'l.has_lyrics', + lyricsKnownExpr: + 'COALESCE(l.audio_metadata_scan_version, 0) >= ${LibraryDatabase.audioMetadataScanVersion}', ); } @@ -461,6 +469,7 @@ extension _LibraryDbQueueSql on LibraryDatabase { required String isrcExpr, required String labelExpr, required String hasLyricsExpr, + required String lyricsKnownExpr, }) { final quality = request.quality?.trim().toLowerCase(); if (quality != null && quality.isNotEmpty) { @@ -546,7 +555,14 @@ extension _LibraryDbQueueSql on LibraryDatabase { where.add('NOT ($hasLabel)'); break; case 'missing-lyrics': - where.add('COALESCE($hasLyricsExpr, 0) = 0'); + // A default false value on legacy rows means "not scanned yet", not + // "confirmed missing". Only show files whose lyrics probe completed. + where.add( + confirmedMissingLyricsSqlPredicate( + hasLyricsExpr: hasLyricsExpr, + lyricsKnownExpr: lyricsKnownExpr, + ), + ); break; } } diff --git a/lib/widgets/re_enrich_field_dialog.dart b/lib/widgets/re_enrich_field_dialog.dart index 924a3820..b908e9ea 100644 --- a/lib/widgets/re_enrich_field_dialog.dart +++ b/lib/widgets/re_enrich_field_dialog.dart @@ -12,7 +12,7 @@ Future showReEnrichFieldDialog( context: context, useRootNavigator: true, title: AppLocalizations.of(context).trackReEnrich, - subtitle: AppLocalizations.of(context).trackReEnrichOnlineSubtitle, + subtitle: AppLocalizations.of(context).trackReEnrichBatchSubtitle, maxHeightFactor: 0.9, builder: (ctx) => _ReEnrichFieldSheet(selectedCount: selectedCount), ); @@ -28,9 +28,30 @@ class _ReEnrichFieldSheet extends StatefulWidget { class _ReEnrichFieldSheetState extends State<_ReEnrichFieldSheet> { final Set _selected = Set.from(ReEnrichFields.all); + final Map _manualControllers = { + for (final field in manualBatchMetadataFields) + field: TextEditingController(), + }; ReEnrichBatchMode _mode = ReEnrichBatchMode.missingOnly; bool get _allSelected => _selected.length == ReEnrichFields.all.length; + bool get _hasManualValues => _manualControllers.values.any( + (controller) => controller.text.trim().isNotEmpty, + ); + + Map get _manualValues => { + for (final entry in _manualControllers.entries) + if (entry.value.text.trim().isNotEmpty) + entry.key: entry.value.text.trim(), + }; + + @override + void dispose() { + for (final controller in _manualControllers.values) { + controller.dispose(); + } + super.dispose(); + } void _toggleAll(bool? value) { setState(() { @@ -90,6 +111,29 @@ class _ReEnrichFieldSheetState extends State<_ReEnrichFieldSheet> { } } + String _manualLabelFor(String field, AppLocalizations l10n) { + switch (field) { + case 'artist_name': + return l10n.trackArtist; + case 'album_name': + return l10n.trackAlbum; + case 'album_artist': + return l10n.trackAlbumArtist; + case 'release_date': + return l10n.trackReleaseDate; + case 'genre': + return l10n.trackGenre; + case 'composer': + return l10n.editMetadataFieldComposer; + case 'label': + return l10n.trackLabel; + case 'copyright': + return l10n.trackCopyright; + default: + return field; + } + } + @override Widget build(BuildContext context) { final l10n = AppLocalizations.of(context); @@ -146,6 +190,17 @@ class _ReEnrichFieldSheetState extends State<_ReEnrichFieldSheet> { onTap: () => setState(() => _mode = ReEnrichBatchMode.selectedFields), ), + const Divider(height: 1, indent: 56), + ListTile( + leading: const Icon(Icons.edit_note), + title: Text(l10n.trackReEnrichModeManual), + subtitle: Text(l10n.trackReEnrichModeManualSubtitle), + trailing: _mode == ReEnrichBatchMode.manualValues + ? Icon(Icons.check, color: colorScheme.primary) + : null, + onTap: () => + setState(() => _mode = ReEnrichBatchMode.manualValues), + ), ], ), if (_mode == ReEnrichBatchMode.selectedFields) ...[ @@ -185,6 +240,58 @@ class _ReEnrichFieldSheetState extends State<_ReEnrichFieldSheet> { ], ), ], + if (_mode == ReEnrichBatchMode.manualValues) ...[ + const SizedBox(height: 8), + Padding( + padding: const EdgeInsets.symmetric(horizontal: 24), + child: Column( + crossAxisAlignment: CrossAxisAlignment.start, + children: [ + Text( + l10n.trackReEnrichManualFieldsTitle, + style: Theme.of(context).textTheme.titleSmall?.copyWith( + color: colorScheme.onSurfaceVariant, + fontWeight: FontWeight.w600, + ), + ), + const SizedBox(height: 4), + Text( + l10n.trackReEnrichManualHint, + style: Theme.of(context).textTheme.bodySmall?.copyWith( + color: colorScheme.onSurfaceVariant, + ), + ), + ], + ), + ), + SettingsGroup( + children: [ + for ( + var index = 0; + index < manualBatchMetadataFields.length; + index++ + ) ...[ + if (index > 0) const Divider(height: 1), + Padding( + padding: const EdgeInsets.fromLTRB(16, 10, 16, 10), + child: TextField( + controller: + _manualControllers[manualBatchMetadataFields[index]], + onChanged: (_) => setState(() {}), + decoration: InputDecoration( + labelText: _manualLabelFor( + manualBatchMetadataFields[index], + l10n, + ), + border: const OutlineInputBorder(), + isDense: true, + ), + ), + ), + ], + ], + ), + ], const SizedBox(height: 8), Padding( padding: const EdgeInsets.fromLTRB(16, 8, 16, 16), @@ -192,14 +299,17 @@ class _ReEnrichFieldSheetState extends State<_ReEnrichFieldSheet> { width: double.infinity, child: FilledButton.icon( onPressed: - _mode == ReEnrichBatchMode.selectedFields && - _selected.isEmpty + (_mode == ReEnrichBatchMode.selectedFields && + _selected.isEmpty) || + (_mode == ReEnrichBatchMode.manualValues && + !_hasManualValues) ? null : () => Navigator.pop( context, ReEnrichFieldSelection( mode: _mode, fields: _selected.toList(), + manualValues: _manualValues, ), ), icon: const Icon(Icons.preview_outlined, size: 18), diff --git a/test/batch_metadata_re_enrich_test.dart b/test/batch_metadata_re_enrich_test.dart index 3fdbf40c..fc29234b 100644 --- a/test/batch_metadata_re_enrich_test.dart +++ b/test/batch_metadata_re_enrich_test.dart @@ -51,6 +51,62 @@ void main() { expect(fields, const ['isrc']); }); + test('manual mode only exposes shared-value fields', () { + expect(manualBatchMetadataFields, containsAll(['album_name', 'genre'])); + expect(manualBatchMetadataFields, isNot(contains('track_name'))); + expect(manualBatchMetadataFields, isNot(contains('track_number'))); + expect(manualBatchMetadataFields, isNot(contains('isrc'))); + }); + + test('manual values build a per-track review without an online lookup', () { + final selection = const ReEnrichFieldSelection( + mode: ReEnrichBatchMode.manualValues, + manualValues: {'album_name': ' New Album ', 'genre': 'Rock'}, + ); + + expect(selection.updateFieldsFor(_item()), ['album_name', 'genre']); + final preview = buildManualBatchReEnrichPreview(_item(), selection); + + expect(preview, isNotNull); + expect(preview!.enrichedMetadata['album_name'], 'New Album'); + expect(preview.changes.map((change) => change.field), [ + 'album_name', + 'genre', + ]); + + final request = buildBatchReEnrichRequest( + item: preview.item, + settings: const AppSettings(), + updateFields: preview.updateFields, + resolvedMetadata: preview.enrichedMetadata, + ); + expect(request['search_online'], isFalse); + expect(request['album_name'], 'New Album'); + expect(request['genre'], 'Rock'); + }); + + test('manual preview omits unchanged and empty values', () { + final preview = buildManualBatchReEnrichPreview( + _item(genre: 'Rock'), + const ReEnrichFieldSelection( + mode: ReEnrichBatchMode.manualValues, + manualValues: {'album_name': 'Album', 'genre': 'Rock', 'label': ' '}, + ), + ); + + expect(preview, isNull); + }); + + test('manual mode rejects unsafe per-track identifiers', () { + final selection = const ReEnrichFieldSelection( + mode: ReEnrichBatchMode.manualValues, + manualValues: {'isrc': 'USAAA2600001', 'track_name': 'Same title'}, + ); + + expect(selection.updateFieldsFor(_item()), isEmpty); + expect(buildManualBatchReEnrichPreview(_item(), selection), isNull); + }); + test('resolved preview metadata is reused without another online search', () { final request = buildBatchReEnrichRequest( item: _item(), diff --git a/test/models_and_utils_test.dart b/test/models_and_utils_test.dart index fb0807f3..840dae36 100644 --- a/test/models_and_utils_test.dart +++ b/test/models_and_utils_test.dart @@ -165,6 +165,21 @@ void main() { }); }); + group('missing lyrics filter', () { + test( + 'requires a completed lyrics scan before treating false as missing', + () { + final predicate = confirmedMissingLyricsSqlPredicate( + hasLyricsExpr: 'item.has_lyrics', + lyricsKnownExpr: 'item.lyrics_scan_version >= 1', + ); + + expect(predicate, contains('item.lyrics_scan_version >= 1')); + expect(predicate, contains('COALESCE(item.has_lyrics, 0) = 0')); + }, + ); + }); + group('app state database migrations', () { final source = File( 'lib/services/app_state_database.dart',