jeffvli / sonixd

A full-featured Subsonic/Jellyfin compatible desktop music player
GNU General Public License v3.0
1.84k stars 74 forks source link

[Linux] KDE's MPRIS tray applet is synchronized with the playback state only when it is activated #162

Closed Nocifer closed 2 years ago

Nocifer commented 2 years ago

Describe the bug On KDE the MPRIS tray applet's state is not synchronized with playback unless it is activated (open), resulting in a wildly inaccurate progress bar status.

Probably related to #152.

To Reproduce Steps to reproduce the behavior:

  1. Start playback.
  2. After a few seconds, let's say 10, click the MPRIS tray applet to open it.
  3. After 10 more seconds, click it again to close it.
  4. After 10 more seconds, click it again to open it.

Expected behavior At point (2) the progress bar should be pointing at the 0:10 mark, but instead it points at 0:00 and starts counting from there, as if playback had only just begun. At point (3), instead of 0:20, the progress bar will of course be showing 0:10. At point (4), even going by the false start the progress bar should be showing 0:20, but instead it will still show 0:10 (i.e. the point at which we closed the tray applet in the previous step) and resume counting from there, which shows that refreshing the progress bar was suspended while the applet was closed.

Desktop (please complete the following information):

jeffvli commented 2 years ago

I haven't implemented any of the MPRIS functions besides the basic playback controls and the main metadata properties so that's why it's not synced.

I'll see if I can spend some time to add additional support for it.

jeffvli commented 2 years ago

I spent a few hours messing around with KDE, but I don't think there's a graceful way to handle the slider position sync between Sonixd and KDE's player.

I was considering the following:

  1. Continuously send the player's position to MPRIS to sync up the player's time with the MPRIS client

    • This technically does work, but it becomes super janky as KDE/other mpris players automatically update the time so the time bounces back and forth +-1 seconds depending on the interval the position is sent.
  2. Using an event that would fire when KDE's taskbar/player is opened

    • This would probably be the best solution, but I'm unable to find an event that I can use for this. If an event occurs when it opens, I could manually update the time.

Another note is that there are some instances when KDE's seek/position rate isn't properly set, and it goes faster than realtime (1.0). I've set the proper rate in the MPRIS player properties, but I still see this happen occasionally. If you're familiar with any other (open source) music players that are gracefully handling KDE's player, I'd like to see how they're handling it.

Nocifer commented 2 years ago

Well, up until now I've been using Cantata and as far as MPRIS interaction is concerned it's always been top notch. But it's a native Qt application so it utilizes Qt's libraries, along with DBus. Maybe Sonixd being an Electron app is a hindrance here? The only other Electron-based media player app I know of is Sublime Music, and AFAICT it doesn't even bother to offer MPRIS integration.

jeffvli commented 2 years ago

Thanks for that info. I'll probably need to find an app that is using an external MPRIS library to handle it.

Some additional info for anyone who may see this thread: I've been using the mpris-service library to handle events. The documentation points to an overridable getPosition() function that I assumed would handle this, but I wasn't able to get it working with my testing. You'll have to take a look at the main.dev.js file where the MPRIS player is implemented.

https://github.com/dbusjs/mpris-service/blob/81665c48ec8912aee210407e60d4b3bc114894b3/src/index.js#L88-L102

jacktheripper19 commented 2 years ago

Another behavior that I noticed in version 0.11.0 is that when trying to pause|resume an audio from the MPRIS applet, the slider adjusts correctly (well not really, it's actually shifted by 1 second, but guess that's an issue from the library since playing on VLC also had the same result). What's different though, is before the slider adjusts it moves first to the starting position 0:00 then jumps to the correct position, while for VLC it stopped|resume from the correct position without jumping back to starting position. Maybe this visual hint can tell you something about what's wrong in the code.

jeffvli commented 2 years ago

@jacktheripper19 Your comment about the slider moving back to the 0:00 starting position made me go back and do some additional checks. For some reason I assumed that was the default behavior of the KDE's player, but looking at the source code of mpris-service, it IS being reset as 0 due to the getPosition function. I'm unsure of if I can actually fix this due to how I'm currently sending/receiving the current time position from the renderer (React UI) to the main process (Electron) where the MPRIS player is handling events, but I'll try again.

https://github.com/dbusjs/mpris-service/blob/81665c48ec8912aee210407e60d4b3bc114894b3/src/interfaces/player.js#L102-L111

jeffvli commented 2 years ago

Going to take a look at headset-electron's implementation since they're also using the same library as well as Electron.

Luxed commented 2 years ago

To add to this, I have a very similar issue but using playerctl instead. Issuing the playerctl position command always return something around 0.000010. mpd together with mpDris2 works as expected though.

NyaomiDEV commented 2 years ago

I'm unsure of if I can actually fix this due to how I'm currently sending/receiving the current time position from the renderer (React UI) to the main process (Electron) where the MPRIS player is handling events, but I'll try again.

The easiest way would be to register the MPRIS service from the renderer, if you have Node integration turned on and you don't mind the security implications.

The "proper but not really" way would be to make the renderer ping the main process (via ipcRenderer) with the current position once in, say, half a second. On the main process, you'd just handle that event and store the information in a variable, and then return that inside the overridden getPosition().

You technically cannot do asynchronous stuff since that API expects getPosition() not to be async and it does not call await on it; likewise you cannot return a Promise, so... yeah chances of making this "clean and functional" are close to zero, unless you want to implement the MPRIS2 service module yourself via the dbus-next module (which sadly is a chore on its own)

jeffvli commented 2 years ago

The "proper but not really" way would be to make the renderer ping the main process (via ipcRenderer) with the current position once in, say, half a second. On the main process, you'd just handle that event and store the information in a variable, and then return that inside the overridden getPosition().

@NyaomiDEV Thanks for that information. When I went through headset-electron's implementation, it seems like that's what they were also doing. I first tried to move the mpris-service over to the renderer like you said, but I couldn't get the player instance to register properly with dbus, so I scrapped that idea. I ended up just using the ping method every half second and it works well enough for me to not want to mess with it anymore :).

One thing that bothers me still is that KDE's seek rate doesn't seem to match with what is set for the player (1.0), so the slider moves faster than intended. I tested it out with headset-electron as well and it exhibits the same behavior so I'm assuming it's a problem on either the library or with KDE's player (my DE may also be out of date, not sure).

@Luxed The playerctl position should be fixed with these changes.

Luxed commented 2 years ago

I can confirm that playerctl position works as expected now! :+1: :tada: