clementine-player / Clementine

:tangerine: Clementine Music Player
https://www.clementine-player.org/
GNU General Public License v3.0
3.79k stars 685 forks source link

A progress bar does not appear in the KDE Media Player control in the system tray #5569

Open paulmcauley opened 7 years ago

paulmcauley commented 7 years ago

In the KDE system tray there are "Media Player" controls which allow you to play/pause/skip tracks but Clementine does not offer the option of a progress bar here where you can jump within the current track. Current builds of Amarok offer this feature.

Chemrat commented 7 years ago

Could be Qt/KDE bug because Clementine exposes required data via MPRIS If you add debug output to mediaplayer plasmoid, it seems that it doesn't update length property binding consistently

mziab commented 7 years ago

This bothered me too, so I've done some digging. Here are my findings:

When Clementine is first launched, the capabilities it advertises through GetCaps don't include CAN_SEEK and CAN_GO_PREV, so the Media Player plasmoid disables those functions accordingly.

The full capabilities are only exposed through a CapsChange when Clementine actually starts playing. But GetCaps and CapsChange are MPRIS1-only and seem to have been dropped in MPRIS2. According to the MPRIS2 spec, property changes to CanSeek, CanGoPrevious, CanGoNext etc. should be handled using the org.freedesktop.DBus.Properties.PropertiesChanged signal is emitted with the new value.

However, the MPRIS2 support in Clementine doesn't seem to be doing this and only uses the old CapsChange signal, which isn't supported by MPRIS2 applications and causes erratic behavior.

Incidentally, if you restart Plasma when Clementine is playing audio (and advertising full capabilities), the progress bar and previous track button work as intended.

A quick fix I've found to work involves changing Mpris2::CanGoNext, Mpris2::CanGoPrev and Mpris2::CanSeek to return true.

The proper fix involves calling EmitNotification with the appropriate properties in Mpris2::EngineStateChanged. I think I was able to do it cleanly, so I should be making a pull request a bit later.

mziab commented 7 years ago

Actually, this seems to have been fixed properly in master in March of last year in a series of commits by Victor Dodon. So I'll be gracefully bowing out.

Chemrat commented 7 years ago

Yeah, it seems to work for me in current master, unless I'm having trouble reproducing it