Skip to content

Mpris integration#197

Merged
devgianlu merged 20 commits intodevgianlu:masterfrom
tooxo:mpris
Sep 13, 2025
Merged

Mpris integration#197
devgianlu merged 20 commits intodevgianlu:masterfrom
tooxo:mpris

Conversation

@tooxo
Copy link
Copy Markdown
Contributor

@tooxo tooxo commented Aug 21, 2025

Todo:

  • locks up on receiving mpris commands with no open session
  • mpris server close is not used yet
  • add logger to mpris package

@tooxo tooxo marked this pull request as draft August 21, 2025 00:19
@tooxo
Copy link
Copy Markdown
Contributor Author

tooxo commented Aug 21, 2025

@devgianlu I know this is still a draft, so I wouldn't want to review it either, but can you take a look at the general structure and maybe feedback if you have some improvements on how it is to be integrated in the program? I tried to do it parallel to the web_api and events system but maybe there is a way to make this more unified (?)

Copy link
Copy Markdown
Owner

@devgianlu devgianlu left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd say this looks pretty good. Mainly nits here and there.

I haven't looked at the mpris package for bugs or nits, but I'd say the overall structure is sound.

At one point there will be a need to unify all the events system, but that day is not today.

Comment thread cmd/daemon/controls.go Outdated
Comment thread cmd/daemon/controls.go Outdated
Comment thread cmd/daemon/controls.go Outdated
Comment thread cmd/daemon/controls.go Outdated
Comment thread cmd/daemon/controls.go Outdated
Comment thread cmd/daemon/player.go Outdated
Comment thread cmd/daemon/player.go Outdated
Comment thread cmd/daemon/player.go Outdated
Comment thread cmd/daemon/player.go Outdated
Comment thread mpris/interfaces.go Outdated
@devgianlu devgianlu linked an issue Sep 3, 2025 that may be closed by this pull request
@tooxo tooxo marked this pull request as ready for review September 8, 2025 20:47
@tooxo
Copy link
Copy Markdown
Contributor Author

tooxo commented Sep 8, 2025

Should be ready for a more thorough review. I would also consider this to be a low-risk merge because it should be off by default (mainly because not every platform supports mpris).

Someone should also check if this compiles on a non-linux machine, because of the inclusion of the dbus dependency. (It now at least compiles on linux+macos)

Comment thread proto/spotify/metadata/metadata.proto
@tooxo tooxo changed the title Draft: Mpris integration Mpris integration Sep 8, 2025
Copy link
Copy Markdown
Owner

@devgianlu devgianlu left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We're almost there! I tested it and it seems to work pretty well.

Just a couple more small things to sort out.

Comment thread mpris/fallback-server.go Outdated
Comment thread mpris/interfaces.go Outdated
Comment thread cmd/daemon/controls.go Outdated
Comment thread go.mod Outdated
Comment thread mpris/interfaces.go
Comment thread mpris/server.go
@devgianlu devgianlu merged commit 8d888e4 into devgianlu:master Sep 13, 2025
7 checks passed
mcMineyC pushed a commit to mcMineyC/go-librespot_embedded that referenced this pull request Mar 27, 2026
* feat: add initial poc for mpris integration

* chore: add mpris enabled to config schema

* fix: stop command, sigsegv in state updates

* chore: do not allow nil in mpris channels

* refactor: mpris metadata use correct uris

* refactor: improve error handling for mpris

* fix: set mpris stopped in the right place

* chore: align usage of media control methods

* chore: refactor enum

* chore: add pointer helper method

* chore: use app logger

* refactor: mpris media state and state updates

* refactor: add logger to mpris package

* amend: use correct logger package

* chore: refactor mpris metadata code

* fix: add error to close of mpris server interface

* fix: make sure mpris:trackid is always set as required by mpris documentation

* chore: refactor to allow build on linux and non-linux hosts

* chore: revert unnecessary changes

* chore: implement change requests
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Feature Request: MPRIS Support

2 participants