diff --git a/backend/mediacontrols/mpris_linux.go b/backend/mediacontrols/mpris_linux.go index b74f6d2..06e0d2f 100644 --- a/backend/mediacontrols/mpris_linux.go +++ b/backend/mediacontrols/mpris_linux.go @@ -279,17 +279,19 @@ func (h *MPRISHandler) enqueue(fn func()) { } } -// UpdateMetadata pushes track metadata to D-Bus. -func (h *MPRISHandler) UpdateMetadata(meta Metadata) { - h.mu.Lock() - h.trackID++ - tid := h.trackID - h.mu.Unlock() - - m := map[string]interface{}{ +// metadataMap builds the org.mpris.MediaPlayer2.Player Metadata value +// for one track. +// +// It is separated from UpdateMetadata, which needs a live D-Bus +// connection, so the map's contents can be asserted on: this file is +// behind a build tag and everything in it that touches h is reachable +// only from a session bus, which is the same reason the Android +// contract lives in an untagged androidpayload.go. +func metadataMap(meta Metadata, trackID uint64) map[string]any { + m := map[string]any{ "mpris:trackid": dbus.ObjectPath( fmt.Sprintf( - "/org/yellowjacket/Track/%d", tid, + "/org/yellowjacket/Track/%d", trackID, ), ), } @@ -306,16 +308,45 @@ func (h *MPRISHandler) UpdateMetadata(meta Metadata) { m["xesam:album"] = meta.Album } + // Always present, even with nothing to point at. + // + // Every other key here can be omitted safely because a client + // reading the map sees a track with no title or no album and + // renders it that way. Art is different: KDE's applet (and + // others) treat an *absent* mpris:artUrl as "no news about the + // art" and keep drawing whatever the last track had, so playing + // something with no cover left the previous album's sleeve on + // screen — which reads as the wrong track playing rather than as + // missing artwork. + // + // An empty string is the honest answer and is what the spec's + // "URI" type degrades to; a client that cannot load it falls back + // to its own placeholder, which is the behaviour wanted. + artURL := "" if meta.ArtFilePath != "" { - m["mpris:artUrl"] = "file://" + meta.ArtFilePath + artURL = "file://" + meta.ArtFilePath } + m["mpris:artUrl"] = artURL + if meta.DurationSec > 0 { m["mpris:length"] = int64( meta.DurationSec, ) * usPerSec } + return m +} + +// UpdateMetadata pushes track metadata to D-Bus. +func (h *MPRISHandler) UpdateMetadata(meta Metadata) { + h.mu.Lock() + h.trackID++ + tid := h.trackID + h.mu.Unlock() + + m := metadataMap(meta, tid) + h.enqueue(func() { h.props.SetMust(playerIf, "Metadata", m) }) diff --git a/backend/mediacontrols/mpris_linux_test.go b/backend/mediacontrols/mpris_linux_test.go new file mode 100644 index 0000000..195fb2d --- /dev/null +++ b/backend/mediacontrols/mpris_linux_test.go @@ -0,0 +1,86 @@ +//go:build linux && !android + +package mediacontrols + +import "testing" + +// The one key that must be present even when it is empty. +// +// Everything else in the map may be omitted, because a client reading +// it renders a track with no title as a track with no title. Art is +// different: KDE's applet treats an *absent* mpris:artUrl as no news +// about the art and keeps drawing the last one it saw, so a track with +// no cover wore the previous album's sleeve — which reads as the wrong +// track playing rather than as missing artwork. +func TestMetadataMapAlwaysCarriesArtURL(t *testing.T) { + t.Parallel() + + tests := []struct { + name string + meta Metadata + want string + }{ + { + name: "no art at all", + meta: Metadata{Title: "Blue in Green"}, + want: "", + }, + { + name: "art on disk", + meta: Metadata{ + Title: "Blue in Green", + ArtFilePath: "/covers/kind-of-blue_lg.jpg", + }, + want: "file:///covers/kind-of-blue_lg.jpg", + }, + } + + 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 is absent; it must always be sent") + } + + if got != tt.want { + t.Errorf("mpris:artUrl = %v, want %q", got, tt.want) + } + }) + } +} + +// The trackid has to change between tracks or a client is entitled to +// treat the metadata as describing the same track it already has. +func TestMetadataMapTrackIDVaries(t *testing.T) { + t.Parallel() + + first := metadataMap(Metadata{Title: "A"}, 1)["mpris:trackid"] + second := metadataMap(Metadata{Title: "B"}, 2)["mpris:trackid"] + + if first == second { + t.Errorf("trackid did not change: %v", first) + } +} + +// The optional keys stay optional — this is what makes artUrl's +// always-present treatment a deliberate exception rather than drift. +func TestMetadataMapOmitsEmptyOptionalFields(t *testing.T) { + t.Parallel() + + m := metadataMap(Metadata{}, 1) + + for _, key := range []string{ + "xesam:title", + "xesam:artist", + "xesam:album", + "mpris:length", + } { + if _, ok := m[key]; ok { + t.Errorf("%s is present for an empty Metadata", key) + } + } +}