eworm-de / mpd-notification

Notify about tracks played by mpd
GNU General Public License v3.0
86 stars 23 forks source link

Incorrect artwork shown when pausing #19

Closed josswright closed 6 years ago

josswright commented 6 years ago

When pausing mpd, mpd-notification shows the album art of a previously-played album rather than the current one. As far as I can tell, it shows the first album played in this instance of mpd-notification. On play or resume, the current album's artwork is shown.

Running on Arch Linux using the latest git checkout.

eworm-de commented 6 years ago

This is with xfce4-notifyd, no? Please update that to the latest version.

josswright commented 6 years ago

I'm using dunst as the notification daemon.

eworm-de commented 6 years ago

Sounds like dunst has a bug (that is very similar to one fixed in xfce4-notifyd recently).

With mpd-notification the pixbuf is cleared at mpd-notification.c line 475.

josswright commented 6 years ago

Oh, thanks. Do you happen to have a link to the xfce4-notifyd issue? I'll report it to dunst.

eworm-de commented 6 years ago

Bug 13950 - Updating a notification with image-data, icon_data and app_icon yields inconsistent results xfce4-notifyd git commit: Treat icon_data only as pen-ultimate fallback option

josswright commented 6 years ago

Thank you!

eworm-de commented 6 years ago

The bug report also contains reproducing code. Good luck!

bebehei commented 6 years ago

I've tested your reproducer and I'd actually advocate for a libnotify bug.

The output of dbus-monitor path=/org/freedesktop/Notifications indicates, that the following notifications sent:

So the replacing of pixbufs does not work properly. The point, which is much weirder, is that sometimes icon_data and sometimes image-data are used.

As a viewpoint of the notification daemon, we can't see, what your intention was. We can just see, what's transferred over wire. And there we have to stick to the standard.

bebehei commented 6 years ago

I found the bug in libnotify. This is a very nice bug.

libnotify checks for the notification spec version used.

https://github.com/GNOME/libnotify/blob/43aac613f1c2a56aa672c36c157c9b8eb193ac25/libnotify/notification.c#L741-L747

But at the point, where no notification is sent yet, the notification spec version is unknown and the check fails.

Therefore before the first notification displayed, the notify_notification_set_image_from_pixbuf will set icon_data, while the next notification will set image-data. And icon_data won't ever get replaced, which takes precedence according to the standard.

It might be best, to call show and close on this notification right after initializing the notification.

https://github.com/eworm-de/mpd-notification/blob/6e33b32b13bc6287255465ff59f760a7f5798c30/mpd-notification.c#L371-L378

bebehei commented 6 years ago

Hey, and this bug is even over 5 years old!

https://bugzilla.gnome.org/show_bug.cgi?id=679888

The libnotify part of gnome bugzilla is sometimes devastating!

nvllsvm commented 6 years ago

Resolved with https://github.com/eworm-de/mpd-notification/pull/21