After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 660549 - media-keys plugin doesn't send events if no application is registered
media-keys plugin doesn't send events if no application is registered
Status: RESOLVED NOTABUG
Product: gnome-settings-daemon
Classification: Core
Component: media-keys
unspecified
Other Linux
: Normal normal
: ---
Assigned To: gnome-settings-daemon-maint
gnome-settings-daemon-maint
: 660706 (view as bug list)
Depends on:
Blocks:
 
 
Reported: 2011-09-30 08:43 UTC by Christoph Reiter (lazka)
Modified: 2011-10-03 16:04 UTC
See Also:
GNOME target: ---
GNOME version: ---



Description Christoph Reiter (lazka) 2011-09-30 08:43:54 UTC
Regarding http://git.gnome.org/browse/gnome-settings-daemon/commit/plugins/media-keys/gsd-media-keys-manager.c?id=564b3ba94167f1b89f0e895d7930f719c0746b6b

This disabled events if no application is registered. Before that it was possible to listen to events by grabbing with a low time or simply listen without grabbing. The second use case now doesn't work anymore.

Since the null pointer check and !have_listeners return value is still there, I would suggest to remove the "return" to restore the old behavior.

regards
Comment 1 Bastien Nocera 2011-09-30 13:58:50 UTC
What's the use case for that?
Comment 2 Christoph Reiter (lazka) 2011-09-30 14:34:36 UTC
Well, maybe use case was not the right word. Quod Libet uses it that way at the moment and this breaks it, which isn't nice.

The code (before the change) suggested that this was intentional behavior and the commit above looks like it was unintentionally changed.

btw: Is there a documentation for the dbus interfaces somewhere?, the README is empty and google isn't helpful either.
Comment 3 Bastien Nocera 2011-10-03 13:48:53 UTC
*** Bug 660706 has been marked as a duplicate of this bug. ***
Comment 4 Bastien Nocera 2011-10-03 13:58:24 UTC
(In reply to comment #2)
> Well, maybe use case was not the right word. Quod Libet uses it that way at the
> moment and this breaks it, which isn't nice.

Quod Libet was using it wrongly.

> The code (before the change) suggested that this was intentional behavior and
> the commit above looks like it was unintentionally changed.

No, it wasn't intentional, it was an oversight.

> btw: Is there a documentation for the dbus interfaces somewhere?, the README is
> empty and google isn't helpful either.

http://git.gnome.org/browse/gnome-settings-daemon/tree/plugins/media-keys/gsd-media-keys-manager.c#n68

Call GrabMediaPlayerKeys(), and listen to MediaPlayerKeyPressed() signals, ignoring the signals where the application name doesn't match your own.
Comment 5 Bastien Nocera 2011-10-03 14:00:36 UTC
And before you say that we "broke" existing apps, the existing apps would have been broken because 1) they ignored the "application" argument passed in the signal, 2) they would have broken things badly when 2 applications using this API were present, such as a music playback app and a video player.
Comment 6 Christoph Reiter (lazka) 2011-10-03 15:42:41 UTC
(In reply to comment #4)
> > btw: Is there a documentation for the dbus interfaces somewhere?, the README is
> > empty and google isn't helpful either.
> 
> http://git.gnome.org/browse/gnome-settings-daemon/tree/plugins/media-keys/gsd-media-keys-manager.c#n68
> 
> Call GrabMediaPlayerKeys(), and listen to MediaPlayerKeyPressed() signals,
> ignoring the signals where the application name doesn't match your own.

I wouldn't call dbus introspection documentation. (Stating it in a closed bug doesn't help much)

(In reply to comment #5)
> And before you say that we "broke" existing apps, the existing apps would have
> been broken because 1) they ignored the "application" argument passed in the
> signal, 2) they would have broken things badly when 2 applications using this
> API were present, such as a music playback app and a video player.

You broke our apps! No.. It just would make things easier for us (and for pithos it seems), since gnome point releases make it into major distro releases. And the old behavior would have no negative effect.

Thanks for looking into his.
Comment 7 Bastien Nocera 2011-10-03 16:04:58 UTC
(In reply to comment #6)
> (In reply to comment #4)
> > > btw: Is there a documentation for the dbus interfaces somewhere?, the README is
> > > empty and google isn't helpful either.
> > 
> > http://git.gnome.org/browse/gnome-settings-daemon/tree/plugins/media-keys/gsd-media-keys-manager.c#n68
> > 
> > Call GrabMediaPlayerKeys(), and listen to MediaPlayerKeyPressed() signals,
> > ignoring the signals where the application name doesn't match your own.
> 
> I wouldn't call dbus introspection documentation. (Stating it in a closed bug
> doesn't help much)

It was the same doc that was available in the past (eg. not much at all). I added a README.media-keys-API file.

> (In reply to comment #5)
> > And before you say that we "broke" existing apps, the existing apps would have
> > been broken because 1) they ignored the "application" argument passed in the
> > signal, 2) they would have broken things badly when 2 applications using this
> > API were present, such as a music playback app and a video player.
> 
> You broke our apps! No.. It just would make things easier for us (and for
> pithos it seems), since gnome point releases make it into major distro
> releases. And the old behavior would have no negative effect.

It would. If you have Rhythmbox and Pithos running for example, pressing "Play" would send "Play" for Rhythmbox (because it's the only app that requested to be registered), and both Pithos and Rhythmbox would start playing. Ditto a session with Totem and Quod Libet.

I will not change this back, please fix your apps.

(In master, we also only send it the signal only to the app that registered itself, as it avoids unnecessary wake-ups, and we have code to track the vanishing of applications).