mate-desktop / mate-settings-daemon

MATE settings daemon
https://mate-desktop.org
GNU General Public License v2.0
43 stars 48 forks source link

media-keys: migrate from dbus-glib to GDBus #404

Closed yetist closed 1 year ago

yetist commented 1 year ago

Please review and test this PR, thanks.

raveit65 commented 1 year ago

This breaks mpris2 media-keys in mediaplayers or embedded mediaplayer in website. For example the fn+play/pause on a keyboard with media keys. I have only this key on my keyboard. In the msd-mpris2 plugin we have some keys defined https://github.com/mate-desktop/mate-settings-daemon/blob/master/plugins/mpris/msd-mpris-manager.c#L150 It looks like this plugin use already GDBus. Maybe the bus address doesn't work with PR ?. Steps to reproduce:

  1. you need a keyboard with media-keys
  2. open a mediaplayer, eg. Deadbeef, vlc or an embedded http player on websites
  3. start a song or a video
  4. try to pause/restart the media
raveit65 commented 1 year ago

The msd-mpris-manager should use this bus address org.mate.SettingsDaemon.MediaKeys https://github.com/mate-desktop/mate-settings-daemon/blob/master/plugins/mpris/msd-mpris-manager.c#L278

yetist commented 1 year ago

Sorry, I did the wrong test, tested volume up and down and mute, let me change it again.

yetist commented 1 year ago

My test result is that this latest PR itself does not work, but after cherry-pick 05fd93af01651a66ec96eec2491992c60328b893 (gdbus-msd branch), the play/pause media key does.

raveit65 commented 1 year ago

@yetist There is a distcheck error, see https://app.travis-ci.com/github/mate-desktop/mate-settings-daemon/jobs/610526324#L2662

make[5]: Entering directory '/rootdir/plugins/media-keys'

make  distdir-am

make[6]: Entering directory '/rootdir/plugins/media-keys'

make[6]: *** No rule to make target 'msd-media-keys-manager.xml', needed by 'distdir-am'.  Stop.

make[6]: Leaving directory '/rootdir/plugins/media-keys'
raveit65 commented 1 year ago

Confirmed, with the other PR and latest changes here the media key fn+play/pause is working in deadbeef media-player and embedded website players. Found another keyboard in my gubbins box with more media-keys and i can confirm that the fn+ next/previous track are working too with deadbeef media-player :-) In embedded website players only pause/restart is working. Starting a stream doesn't work. But this is the same behavior as in master. So, no new issue.

lukefromdc commented 1 year ago

Building this by itself (not cherrypicking anything else into m-s-d) I got the same behavior as on my first test, the one key on my very old keyboard that can affect a media player no longer controls it. Not the fault of this PR but rather a side effect of breaking it up

lukefromdc commented 1 year ago

Still cannot control audacious with the one key I have that acts as a media control key (stops playback when pressed with git master). Not sure this keyboard is a valid test though

yetist commented 1 year ago

Still cannot control audacious with the one key I have that acts as a media control key (stops playback when pressed with git master). Not sure this keyboard is a valid test though

Please try to test this PR with #406

lukefromdc commented 1 year ago

Combining this with https://github.com/mate-desktop/mate-settings-daemon/pull/406 did not help at all, though again, since this is NOT an intended media key that sometimes works, I am thinking all of my testing of this may be invalid. At the moment that key isn't responding in master either, so I probably cannot get a valid test without playing with keymappings, which I have little experience in doing. At any rate, tonight I am out of time

raveit65 commented 1 year ago

Tested again all PRs. With the other PR and latest changes here the media key fn+play/pause and fn+ next/previous are working in deadbeef media-player and embedded website players.

raveit65 commented 1 year ago

Ok, we need to merge all 4 PRs in one rush when they are approved.

gregoryflood9 commented 9 months ago

I'm thinking about writing a repo for the mate repo mergers and forking ability, suffice to say not abundantly clustered repo liquidity should be more ubiquitous -- python3.