z411 / trackma

Open multi-site list manager for Unix-like systems. (ex-wMAL)
https://z411.github.io/trackma
GNU General Public License v3.0
761 stars 82 forks source link

mpris: Rewrite entirely, replacing pydbus with jeepney #683

Closed FichteFoll closed 2 months ago

FichteFoll commented 1 year ago

pydbus had issues with properly registering, unregistering and filtering dbus signals, resulting in one listener being created for each player recognized and never cleaning them up. It is also unmaintained and the Github repository hasn't seen activity (including responses to issues or pull requests for over 4 years).

Additionally, there were several situations where opening another player in the background with the same or a different file (that may or may not be a recognizable episode) would trip up the tracker entirely, resulting in missed updates that would cascade to any following episode of the same show.

These situations are now resolved by:

  1. Switching to jeepney, a more low-level dbus library, providing full access to the dbus signal subscriber & filtering protocol. This way we only need a total of two watchers at all times.
  2. Properly tracking the full state of every player that fulfills the regex criteria.
  3. Ignoring any updates while the current player with a valid episode is still playing.
  4. Ignoring any updates for players whose filename cannot be recognized.

Because the parent Tracker class was not built with the idea of potentially handling multiple players at the same time, this can still result in confusions when the primary player is paused and another valid episode is opened in the background, which will then replace the tracking status of the previously paused player. However, this can be recovered if the newly opened player is closed in time so that the intended primary player has enough time to reach the episode update threshold afterwards (and will probably occur rarely enough).

~~The code utilizes asyncio and dataclasses, bumping the required Python version to 3.7, unless a dataclass backport is added. However, Python 3.7 is the oldest minor version that hasn't reached EOL yet, so this should be of no concern. (Edit: also uses asyncio.run, which was added in 3.7)~~ (Edit2: 3.7 was already mentioned as a requirement in the readme, so disregard)

Additionally, dependencies are re-locked due to the switch to jeepney.

Closes #440 Closes #420 Closes #636 Closes #471


I will be battle-testing this over the next few days but my initial tests seem to be rather promising.

Known issues/questions that should be addressed before merging:

FichteFoll commented 1 year ago

Thanks for taking a look. There are some issues/questions that should be addressed before merging that I've added to OP.

FichteFoll commented 2 months ago

So, I have been using this branch as my main and only branch for trackma basically since I created it, meaning for over a year now, and it just works really well. It addresses almost all of the concerns I had with the old pydbus code and beside very few remaining usability improvements that could be made, it's just a significant update over whatever we have currently. Other people, who are also using this, have urged me several times to merge it as well.

Thus, I am going to do this. Any further changes (including the three items I listed in OP) can be tackled at a later point and there is no reason to delay this PR any further. Besides, who knows when I (or someone else) will get around to looking into them.

I've rebased this banch onto master and also applied a few more low-hanging fixes that have either been reported to me (crash in the QT UI) or that I have noticed myself over time but never bothered to actually fix.