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
This commit is contained in:
@@ -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)
|
||||
}
|
||||
}
|
||||
}
|
||||
Reference in New Issue
Block a user