Media not detected in KDE media player

Drarig29 commented 3 years ago

When I use https://github.com/jpochyla/psst on Manjaro KDE, there is no thumbnail and the play/pause button is on 'play' (and doesn't work) although music is playing. However, the previous/next buttons work.

In the taskbar context menu, the stop button is greyed out and the play/pause button doesn't work either.

I don't know if it's a bug coming from this module or from psst implementation.

Manjaro Linux 21.1.3, KDE 5.22.5 Ref: https://github.com/jpochyla/psst/issues/185

leosunmo commented 3 years ago

I don't know if this is necessarily specific to KDE. I am also using psst and I think souvlaki has to implement the PropertiesChanged signal to make this work nicely.

If you want to see the different behaviour of various media players you can run this command to watch for change signals:

dbus-monitor type=signal interface="org.mpris.MediaPlayer2.Player"

This should give you a whole bunch of output when interacting with media players. It's using what I understand to be a PropertiesChanged method that the client (media player) has to correctly publish/trigger? I don't know enough about D-Bus.

By using D-Feet to call org.freedesktop.DBus.Introspectable.Introspect on the players I noticed that only Audacious seems to correctly indicate whether a PropertiesChanged signal was supported for each property or not.

According to the D-Bus Specs I'm guessing souvlaki has to implement sending the PropertiesChanged signal upon properties changes.

Here's a rundown of what I spotted doing a real quick test of Spotify, Audacious, and Psst (running souvlaki as far as I understand). The only way to properly see how they publish changes is to test, since the registered introspection isn't always correct, especially in Spotify's case.

Spotify

Spotify's Linux client isn't... the best and a lot of MPRIS methods are incorrectly implemented or missing.The publication of Metadata works well however.

Metadata/Track info is displayed as a result of PropertiesChanged signal on pretty much any media player action. Play/Pause, changing song (Next/Prev, click a new song etc.), volume change, changing playback mode (shuffle, repeat etc.).

Every time something triggers a PropertiesChanged, it spams the Track info in the Metadata object at least 3 times. Very strange, but you certainly won't miss which song is currently playing.

Spotify's org.freedesktop.DBus.Introspectable.Introspect XML is pretty unrepresentative of what it supports.

Don't copy Spotify's behaviour :smile:

Audacious

Audacious is a Linux native client, and its behaviour is probably more "proper" than Spotify's.

It triggers the PropertiesChanged signal on all relevant media player actions (Play/Pause, Next/Previous, volume change etc.) but it does not send the Track info in the metadata for most of them, only the relevant information for the specific action taken. Meaning when I change the volume it only prints a new volume level in the response.

It shows track info on Next/Previous, not Play/Pause. It does show track info if you Stop, then Play. This seems like the ideal behaviour.

In the spec it's mentioned that it's common to only specify which property doesn't support a PropertiesChanged signal using the org.freedesktop.DBus.Property.EmitsChangedSignal annotation, and Audacious has that set for the one method that doesn't send a signal, position change:

     ...
     <property type="s" name="PlaybackStatus" access="read"/>
     <property type="x" name="Position" access="read">
       <annotation name="org.freedesktop.DBus.Property.EmitsChangedSignal" value="false"/>
     </property>
     <property type="d" name="Volume" access="readwrite"/>
     ...

Psst

psst/souvlaki doesn't seem to trigger the PropertiesChanged that I am monitoring with my above command. I can manually get the track info using: dbus-send --print-reply --session --dest=org.mpris.MediaPlayer2.psst /org/mpris/MediaPlayer2 org.freedesktop.DBus.Properties.Get string:'org.mpris.MediaPlayer2.Player' string:'Metadata'

souvlaki's org.freedesktop.DBus.Introspectable.Introspect seems accurate except for the missing org.freedesktop.DBus.Property.EmitsChangedSignal annotations.

Basically if we'd want to add these signals to souvlaki (assuming they're not already supported and I've just missed that, or Psst is misusing souvlaki), we should probably copy Audacious' behaviour and introspection XML!

leosunmo commented 3 years ago

I spotted this in one of the dependencies, https://github.com/diwic/dbus-rs/blob/master/dbus-crossroads/Problems.md. So looks like signals is a bit tricky.

tramhao commented 3 years ago

excellent materials. I tried to implement it manually in https://github.com/tramhao/termusic/blob/master/src/souvlaki.rs . It works now but everytime when setting metadata, it'll generate two propertieschanged message. For playback status, it's working as expected.

Sinono3 commented 3 years ago

Hi. Thanks @Drarig29 for posting this issue and @leosunmo and @tramhao for the research! I've been trying to tackle this for a week and a half now and currently I'm migrating the D-Bus service code from dbus-crossroads to zbus. With that done, it should automatically emit the PropertiesChanged signal and generate proper XML introspection code.

Drarig29 commented 3 years ago

Very nice! 😎

Sinono3 commented 3 years ago

It's done. The new version is on the zbus branch. I'm installing a virtual machine with KDE Plasma to test it out. If someone wants to try, go ahead.

To test it just clone psst and change souvlaki = 0.4 to souvlaki = { git = "https://github.com/Sinono3/souvlaki", branch = "zbus" } on psst-gui/Cargo.toml.

Drarig29 commented 3 years ago

I just tested and:

[x] Play/pause works
[x] Previous/next works
[x] Album image is shown
[ ] Shuffle and repeat buttons are disabled
- With Spotify, they are enabled but do not work though
- Clicking on them in the Media Player popup doesn't do anything
- Clicking on them in Spotify doesn't update the buttons in the popup
- With VLC, both of them work
[ ] Seeking is enabled but doesn't work (moving the slider doesn't seek the song)
- With Spotify, seeking doesn't work properly
- The input is disabled (you can't manipulate the slider)
- It just has a correct total track duration
- But seeking in Spotify doesn't update the slider in the popup
- With VLC, it works perfectly

So yeah, Spotify is broken and isn't a good example of implementation. But as always, VLC is perfect 😎

About the missing features, I don't know if it's because psst needs to do something special, but the checked features are the main ones and it's a good thing they are working. Thank you!

Sinono3 commented 3 years ago

Nice! So it works now.

Souvlaki does not yet implement shuffle and repeat features (coming soon though).
Seek/SetPosition is not implemented in Psst yet, but I managed to get it working, so I'm opening a PR soon. (This will only affect Linux though, as it's not implemented on macOS and windows yet)

leosunmo commented 3 years ago

Very exciting! I look forward to upgrading Psst and have my i3 media bar work properly. This means I can actually use Psst more-or-less full-time instead of the bloated Spotify app! If the dbus implementation gets full fleshed out in Psst, it'll give it a huge leg up over Spotify and actually have better feature set than the official client on Linux!!

leosunmo commented 3 years ago

@Sinono3 Running the modified version of Psst now. Works well! I did have to make a change to the code I'm using, here: https://github.com/soumya92/barista/pull/243.

We might want to look at string vs array for AlbumArtist and Artist. No clue what the convention is here.

Sinono3 commented 3 years ago

@Sinono3 Running the modified version of Psst now. Works well! I did have to make a change to the code I'm using, here: soumya92/barista#243.

We might want to look at string vs array for AlbumArtist and Artist. No clue what the convention is here.

Fixed in deb1b53. The fix was pushed in release 0.4.3 :grin:

101100 commented 2 years ago

Are there plans to release the zbus version as either a feature flag or version 0.6?

101100 commented 2 years ago

Actually, I see in the history that you went back to dbus-crossroads for version 0.5. Did you have issues with zbus?

Sinono3 commented 2 years ago

Hi! Sorry for the delay @101100. I reverted to dbus-crossroads because zbus uses async and it complicated things a bit for me because I needed to bundle an executor, even though I found the code to be much cleaner. Do you need the zbus version?

101100 commented 2 years ago

I found that the dbus-crossroads version didn't work for me with gnome. I cloned the repo and made two commits to update to zbus and the latest windows crate here: https://gitlab.101100.ca/heards/souvlaki/-/commits/kiku-updates/ I was making sure it worked with my project before opening a PR. Would you accept a PR with a feature flag to switch between dbus-crossroads and zbus?

Sinono3 commented 2 years ago

Sure! As long as it is a clean implementation I'd be happy to accept that.

Sinono3 commented 2 years ago

@Drarig29 Do you know if souvlaki's latest version still works with KDE?

YXL76 commented 2 years ago

@Drarig29 Do you know if souvlaki's latest version still works with KDE?

It works fine most of the time, sometimes it does not respond to keyboard events.

Drarig29 commented 2 years ago

I'm sorry but I don't use psst anymore since they don't report played musics to Spotify and I use last.fm with my friends. So I haven't tried souvlaki for a long time 😕

Giesch commented 1 year ago

Using ubuntu, I encountered the problem that only the first call to set_metadata had any effect. 101100's zbus branch above solves this issue.

Sinono3 commented 1 year ago

@101100 Any news on the PR?

101100 commented 1 year ago

@Sinono3 Sorry, I let this fall between the cracks. I'll have that updated PR ready for this week.

Sinono3 / souvlaki