From 128fa85a6574a62962ae5d790ee4c4d4d82fcf97 Mon Sep 17 00:00:00 2001 From: Logan Date: Tue, 18 Aug 2026 11:25:57 -0400 Subject: [PATCH] fix(mediacontrols): answer "no artwork" instead of saying nothing A track with no cover art omitted mpris:artUrl entirely, and KDE's media controller keeps showing the last image it was handed -- so the taskbar preview for an untagged track displayed the cover of whichever earlier album happened to have one, where the user has no way to tell it is stale. The key is now always published, empty when there is nothing. Omitting a field is what an absent value naturally reads as, and artwork is the one field where that is wrong. The dictionary build is extracted so it can be asserted on without a bus. Every other field stays absent when empty -- a title of "" would make players show a blank line where they would otherwise fall back to the filename. Refs #41 --- backend/mediacontrols/mpris_linux.go | 30 +++++++- .../mpris_metadata_linux_test.go | 73 +++++++++++++++++++ 2 files changed, 99 insertions(+), 4 deletions(-) create mode 100644 backend/mediacontrols/mpris_metadata_linux_test.go 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) + } + } +} -- 2.54.0