fix(mediacontrols): answer "no artwork" instead of saying nothing #80

Closed
logan wants to merge 1 commits from fix/mpris-stale-art into main
2 changed files with 99 additions and 4 deletions
+26 -4
View File
@@ -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
@@ -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)
}
}
}