diff --git a/backend/mediacontrols/mpris_linux.go b/backend/mediacontrols/mpris_linux.go index b74f6d2..d0be40e 100644 --- a/backend/mediacontrols/mpris_linux.go +++ b/backend/mediacontrols/mpris_linux.go @@ -286,12 +286,36 @@ func (h *MPRISHandler) UpdateMetadata(meta Metadata) { tid := h.trackID h.mu.Unlock() + m := metadataMap(meta, tid) + + h.enqueue(func() { + h.props.SetMust(playerIf, "Metadata", m) + }) +} + +// metadataMap is the org.mpris.MediaPlayer2.Player Metadata dictionary +// for one track. +// +// Absent fields are omitted, with one deliberate exception: a track +// with **no cover art still publishes mpris:artUrl**, empty. Omitting +// it is what a missing value naturally reads as, and it is wrong here +// -- KDE's media controller keeps showing the last artwork it was +// given, so playing an untagged track under a Plasma taskbar preview +// displayed the cover of whichever earlier album happened to have one. +// An empty string is our answer to "what is the artwork" rather than +// no answer at all. +// +// A generated placeholder image would be the stronger fix and needs an +// app icon to point at; this is the version that costs nothing and can +// be checked with busctl. +func metadataMap(meta Metadata, trackID uint64) map[string]interface{} { m := map[string]interface{}{ "mpris:trackid": dbus.ObjectPath( fmt.Sprintf( - "/org/yellowjacket/Track/%d", tid, + "/org/yellowjacket/Track/%d", trackID, ), ), + "mpris:artUrl": "", } if meta.Title != "" { @@ -316,9 +340,7 @@ func (h *MPRISHandler) UpdateMetadata(meta Metadata) { ) * usPerSec } - h.enqueue(func() { - h.props.SetMust(playerIf, "Metadata", m) - }) + return m } // UpdatePlaybackState pushes the playback state and position diff --git a/backend/mediacontrols/mpris_metadata_linux_test.go b/backend/mediacontrols/mpris_metadata_linux_test.go new file mode 100644 index 0000000..6200364 --- /dev/null +++ b/backend/mediacontrols/mpris_metadata_linux_test.go @@ -0,0 +1,73 @@ +//go:build linux && !android + +package mediacontrols + +import "testing" + +// TestArtURLIsAlwaysAnswered pins the one field in the MPRIS metadata +// dictionary that must be published even when it is empty. +// +// Omitting a field is what an absent value naturally reads as, and it +// is wrong for artwork: KDE's media controller keeps showing the last +// image it was handed, so a track with no cover art displayed the +// cover of whichever earlier album had one — in the taskbar preview, +// where the user has no way to tell it is stale. +func TestArtURLIsAlwaysAnswered(t *testing.T) { + t.Parallel() + + tests := []struct { + name string + meta Metadata + want string + }{ + { + name: "art present", + meta: Metadata{ + Title: "Tideline", + ArtFilePath: "/covers/abc_lg.jpg", + }, + want: "file:///covers/abc_lg.jpg", + }, + { + name: "no art at all", + meta: Metadata{Title: "Untagged"}, + want: "", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + + m := metadataMap(tt.meta, 1) + + got, ok := m["mpris:artUrl"] + if !ok { + t.Fatal("mpris:artUrl absent: a missing key " + + "leaves the previous track's art on screen") + } + + if got != tt.want { + t.Errorf("mpris:artUrl = %q, want %q", got, tt.want) + } + }) + } +} + +// TestEmptyFieldsStayAbsent guards the other half: artUrl is the +// exception, not a new rule for the whole dictionary. A title of "" +// published as "" would make every player show an empty line where it +// would otherwise fall back to the filename. +func TestEmptyFieldsStayAbsent(t *testing.T) { + t.Parallel() + + m := metadataMap(Metadata{}, 7) + + for _, key := range []string{ + "xesam:title", "xesam:artist", "xesam:album", "mpris:length", + } { + if _, ok := m[key]; ok { + t.Errorf("%s should be omitted when it has no value", key) + } + } +}