github-featured/bjarneo--cliamp
2
0
Fork 0
Code Issues Pull Requests Deployments Models Agent Notes Packages Projects Releases Wiki Activity

fix(lyrics): harden embedded metadata refresh

Browse Source
This commit is contained in:
Bjarne Øverli
2026-07-05 13:44:47 +02:00
parent aca9577857
commit 40ac05da55
9 changed files with 120 additions and 51 deletions
Download Patch File Download Diff File Expand all files Collapse all files
AI_REVIEW_NOTICE.txt
-1
View File
@@ -1 +0,0 @@
I use AI, and if this file is not deleted, I haven't reviewed my own code.
playlist/tags.go
+12 -15
View File
@@ -37,17 +37,13 @@ func RefreshEmbeddedMetadata(track Track) Track {
if track.Path == "" || track.Stream || IsURL(track.Path) || strings.HasPrefix(track.Path, "ssh://") {
return track
}
if track.EmbeddedLyrics != "" && track.AlbumArtURL != "" {
fresh, ok := readTagsWithOptions(track.Path, true)
if !ok {
return track
}
fresh := readTagsWithOptions(track.Path, track.AlbumArtURL == "")
if track.EmbeddedLyrics == "" {
track.EmbeddedLyrics = fresh.EmbeddedLyrics
}
if track.AlbumArtURL == "" {
track.AlbumArtURL = fresh.AlbumArtURL
}
track.EmbeddedLyrics = fresh.EmbeddedLyrics
track.AlbumArtURL = fresh.AlbumArtURL
return track
}
@@ -55,19 +51,20 @@ func RefreshEmbeddedMetadata(track Track) Track {
// a local audio file and returns a Track. Falls back to filename parsing if
// tag reading fails or the tags contain no title.
func readTags(path string) Track {
return readTagsWithOptions(path, false)
t, _ := readTagsWithOptions(path, false)
return t
}
func readTagsWithOptions(path string, cacheArt bool) Track {
func readTagsWithOptions(path string, cacheArt bool) (Track, bool) {
f, err := os.Open(path)
if err != nil {
return TrackFromFilename(path)
return TrackFromFilename(path), false
}
defer f.Close()
m, err := tag.ReadFrom(f)
if err != nil || m == nil {
return TrackFromFilename(path)
return TrackFromFilename(path), true
}
t := Track{
@@ -81,7 +78,7 @@ func readTagsWithOptions(path string, cacheArt bool) Track {
fallback := TrackFromFilename(path)
fallback.EmbeddedLyrics = t.EmbeddedLyrics
fallback.AlbumArtURL = t.AlbumArtURL
return fallback
return fallback, true
}
t.Title = sanitizeTag(strings.TrimSpace(m.Title()))
@@ -91,7 +88,7 @@ func readTagsWithOptions(path string, cacheArt bool) Track {
t.Year = m.Year()
trackNum, _ := m.Track()
t.TrackNumber = trackNum
return t
return t, true
}
func cacheAlbumArt(picture *tag.Picture) string {
playlist/track_test.go
+39
View File
@@ -140,6 +140,45 @@ func TestCleanupAlbumArtCacheKeepsCurrentFile(t *testing.T) {
}
}
func TestRefreshEmbeddedMetadataPreservesSavedFieldsWhenUnreadable(t *testing.T) {
track := Track{
Path: filepath.Join(t.TempDir(), "missing.mp3"),
Title: "Saved title",
Artist: "Saved artist",
EmbeddedLyrics: "old lyrics",
AlbumArtURL: "file:///old.jpg",
}
got := RefreshEmbeddedMetadata(track)
if got.EmbeddedLyrics != track.EmbeddedLyrics || got.AlbumArtURL != track.AlbumArtURL {
t.Fatalf("embedded metadata changed: got %q, %q want %q, %q", got.EmbeddedLyrics, got.AlbumArtURL, track.EmbeddedLyrics, track.AlbumArtURL)
}
if got.Title != track.Title || got.Artist != track.Artist {
t.Fatalf("non-embedded fields changed: got %+v want title/artist from %+v", got, track)
}
}
func TestRefreshEmbeddedMetadataClearsSavedFieldsWhenTagsUnreadable(t *testing.T) {
path := filepath.Join(t.TempDir(), "empty.mp3")
if err := os.WriteFile(path, nil, 0o644); err != nil {
t.Fatalf("WriteFile: %v", err)
}
track := Track{
Path: path,
Title: "Saved title",
EmbeddedLyrics: "old lyrics",
AlbumArtURL: "file:///old.jpg",
}
got := RefreshEmbeddedMetadata(track)
if got.EmbeddedLyrics != "" || got.AlbumArtURL != "" {
t.Fatalf("embedded metadata = %q, %q; want empty refreshed fields", got.EmbeddedLyrics, got.AlbumArtURL)
}
if got.Title != track.Title {
t.Fatalf("title changed: got %q want %q", got.Title, track.Title)
}
}
func TestTrackFromURL(t *testing.T) {
tests := []struct {
name string
site/index.html
+1 -1
View File
@@ -1665,7 +1665,7 @@ user_id = "your-account-user-id"</code></pre>
<div class="feature">
<div class="feature-icon">✎</div>
<div class="feature-name">Embedded Tag Reading</div>
<p>ID3v2, Vorbis comments, MP4 atoms — artist, album, genre, year, lyrics, cover art.</p>
<p>ID3v2, Vorbis comments, MP4 atoms - artist, album, genre, year, lyrics, cover art.</p>
</div>
<div class="feature">
<div class="feature-icon">▶</div>
ui/model/keys.go
+7 -9
View File
@@ -715,15 +715,13 @@ func (m *Model) handleKey(msg tea.KeyPressMsg) tea.Cmd {
if m.lyrics.visible && !m.lyrics.loading {
track, _ := m.currentPlaybackTrack()
artist, title := m.lyricsArtistTitle()
if artist != "" && title != "" {
q := artist + "\n" + title
if q != m.lyrics.query {
m.lyrics.query = q
m.lyrics.loading = true
m.lyrics.lines = nil
m.lyrics.err = nil
return fetchTrackLyricsCmd(track, artist, title)
}
q := lyricsLookupKey(track, artist, title)
if q != "" && q != m.lyrics.query {
m.lyrics.query = q
m.lyrics.loading = true
m.lyrics.lines = nil
m.lyrics.err = nil
return fetchTrackLyricsCmd(track, artist, title)
}
}
ui/model/lyrics.go
+16
View File
@@ -23,6 +23,22 @@ func (m *Model) lyricsArtistTitle() (artist, title string) {
return track.Artist, track.Title
}
func lyricsLookupKey(track playlist.Track, artist, title string) string {
if artist != "" && title != "" {
return artist + "\n" + title
}
if track.EmbeddedLyrics == "" {
return ""
}
if track.Path != "" {
return "embedded\n" + track.Path
}
if track.Title != "" {
return "embedded\n" + track.Title
}
return "embedded"
}
// lyricsSyncable reports whether synced lyrics can track the current playback
// position. This is true for local files and Navidrome streams (which have
// accurate position tracking), but false for live radio (ICY — position is
ui/model/playback.go
+7 -2
View File
@@ -254,9 +254,14 @@ func (m *Model) beginPlaybackTrack(track playlist.Track) (playlist.Track, tea.Cm
m.seek.timerFor = 0
m.seek.grace = 0
m.seek.graceFor = 0
if m.lyrics.visible && track.Artist != "" && track.Title != "" {
if m.lyrics.visible {
q := lyricsLookupKey(track, track.Artist, track.Title)
if q == "" {
m.lyrics.loading = false
return track, nil
}
m.lyrics.loading = true
m.lyrics.query = track.Artist + "\n" + track.Title
m.lyrics.query = q
return track, fetchTrackLyricsCmd(track, track.Artist, track.Title)
}
m.lyrics.loading = false
ui/model/playback_test.go
+36 -23
View File
@@ -39,15 +39,18 @@ func (f *playbackFakeEngine) Stop() { f.playin
func (f *playbackFakeEngine) Close() {}
func (f *playbackFakeEngine) TogglePause() { f.paused = !f.paused }
func (f *playbackFakeEngine) Seek(time.Duration) error { return nil }
func (f *playbackFakeEngine) SeekYTDL(time.Duration) error { return nil }
func (f *playbackFakeEngine) CancelSeekYTDL() {}
func (f *playbackFakeEngine) IsPlaying() bool { return f.playing }
func (f *playbackFakeEngine) IsPaused() bool { return false }
func (f *playbackFakeEngine) Drained() bool { return false }
func (f *playbackFakeEngine) HasPreload() bool { return false }
func (f *playbackFakeEngine) Seekable() bool { return false }
func (f *playbackFakeEngine) IsStreamSeek() bool { return false }
func (f *playbackFakeEngine) IsYTDLSeek() bool { return false }
func (f *playbackFakeEngine) SeekYTDL(d time.Duration) error {
f.seekYTDLCalls = append(f.seekYTDLCalls, d)
return nil
}
func (f *playbackFakeEngine) CancelSeekYTDL() {}
func (f *playbackFakeEngine) IsPlaying() bool { return f.playing }
func (f *playbackFakeEngine) IsPaused() bool { return f.paused }
func (f *playbackFakeEngine) Drained() bool { return false }
func (f *playbackFakeEngine) HasPreload() bool { return false }
func (f *playbackFakeEngine) Seekable() bool { return false }
func (f *playbackFakeEngine) IsStreamSeek() bool { return false }
func (f *playbackFakeEngine) IsYTDLSeek() bool { return f.ytdlSeek }
func (f *playbackFakeEngine) GaplessAdvanced() bool {
if !f.gaplessAdvanced {
return false
@@ -55,20 +58,6 @@ func (f *playbackFakeEngine) GaplessAdvanced() bool {
f.gaplessAdvanced = false
return true
}
func (f *playbackFakeEngine) Position() time.Duration { return 0 }
func (f *playbackFakeEngine) SeekYTDL(d time.Duration) error {
f.seekYTDLCalls = append(f.seekYTDLCalls, d)
return nil
}
func (f *playbackFakeEngine) CancelSeekYTDL() {}
func (f *playbackFakeEngine) IsPlaying() bool { return f.playing }
func (f *playbackFakeEngine) IsPaused() bool { return f.paused }
func (f *playbackFakeEngine) Drained() bool { return false }
func (f *playbackFakeEngine) HasPreload() bool { return false }
func (f *playbackFakeEngine) Seekable() bool { return false }
func (f *playbackFakeEngine) IsStreamSeek() bool { return false }
func (f *playbackFakeEngine) IsYTDLSeek() bool { return f.ytdlSeek }
func (f *playbackFakeEngine) GaplessAdvanced() bool { return false }
func (f *playbackFakeEngine) Position() time.Duration { return f.position }
func (f *playbackFakeEngine) Duration() time.Duration { return 0 }
func (f *playbackFakeEngine) PositionAndDuration() (time.Duration, time.Duration) {
@@ -349,6 +338,30 @@ func TestPreloadAfterProviderPlaylistLoadUsesFirstNewTrack(t *testing.T) {
}
}
func TestBeginPlaybackTrackFetchesEmbeddedLyricsWithoutNetworkMetadata(t *testing.T) {
m := Model{lyrics: lyricsState{visible: true}}
track := playlist.Track{Title: "Local", EmbeddedLyrics: "Line one\nLine two"}
_, cmd := m.beginPlaybackTrack(track)
if cmd == nil {
t.Fatal("beginPlaybackTrack() command = nil, want embedded lyrics command")
}
if !m.lyrics.loading {
t.Fatal("lyrics.loading = false, want true")
}
msg, ok := cmd().(lyricsLoadedMsg)
if !ok {
t.Fatalf("lyrics command returned %T, want lyricsLoadedMsg", msg)
}
if msg.err != nil {
t.Fatalf("lyrics command error = %v", msg.err)
}
if len(msg.lines) != 2 || msg.lines[0].Text != "Line one" || msg.lines[1].Text != "Line two" {
t.Fatalf("lyrics lines = %+v, want embedded plain text", msg.lines)
}
}
func TestGaplessAdvanceRefreshesLyricsAndArtwork(t *testing.T) {
player := &playbackFakeEngine{playing: true, gaplessAdvanced: true}
p := playlist.New()
ui/model/speed_controls_test.go
+2
View File
@@ -56,6 +56,7 @@ func TestTickPendingSpeedSaveUsesElapsedTime(t *testing.T) {
}
home := t.TempDir()
t.Setenv("XDG_CONFIG_HOME", "")
t.Setenv("HOME", home)
sharedPlayer.Stop()
@@ -96,6 +97,7 @@ func TestFlushPendingSpeedSavePersistsImmediately(t *testing.T) {
}
home := t.TempDir()
t.Setenv("XDG_CONFIG_HOME", "")
t.Setenv("HOME", home)
sharedPlayer.Stop()