Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[RFC] create pipewire-session-manager alternative group #46960

Draft
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

classabbyamp
Copy link
Member

@classabbyamp classabbyamp commented Oct 29, 2023

also moves pipewire/wireplumber configs to /usr/share/pipewire/pipewire.conf.avail, which seems like the upstream-compatible place.

Pros

  • this will make any future session managers easier to add and switch between
  • this removes one step in configuring pipewire (no more symlinking wireplumber conf)

Cons

  • no way to disable this, short of noextract=/usr/share/pipewire/pipewire.conf.avail/10-wireplumber.conf (maybe masking the config with an empty 10-session-manager.conf in a higher priority config location would work?)
  • causes 2 wireplumbers to run if already symlinked as 10-wireplumber.conf, which does not seem to cause issues aside from having 1 that spews errors forever (is this worth an INSTALL.msg?)

Testing the changes

  • I tested the changes in this PR: YES

needs associated documentation adjustment

cc: @cinerea0 @ahesford @paper42

this is a more proper place for it. still includes a symlink to
/usr/share/examples/... so users don't get bitten by this change
also move wireplumber config to pipewire.conf.avail, as it seems to be
the proper place. /usr/share/examples/... remains as a symlink for backcompat
@ahesford
Copy link
Member

This won't cause two WirePlumber instances to run if people followed the instructions as they exist now... the move will instead just break the symlink. We should confirm that PipeWire will operate with the configured alternative in place and a dead link.

It will cause double WirePlumber instances if somebody manually copied or otherwise created the file.

I guess that either situation merits an INSTALL.msg for a few cycles.

The alternative is a good idea in general, although noextract will cause a broken alternative symlink. XBPS behavior around alternatives could definitely be improved here. Hopefully, masking with a null /etc/pipewire/pipewire.conf.d/10-session-manager.conf will work instead. This seems preferable to skipping extraction.

@classabbyamp
Copy link
Member Author

the move will instead just break the symlink

no it won't, as I added a symlink from the old location to the new location.

@ahesford
Copy link
Member

I overlooked that... If PipeWire survives the broken symlink, it might be better to just break it and let the alternative take effect.

@classabbyamp classabbyamp marked this pull request as draft October 29, 2023 23:16
@oreo639
Copy link
Member

oreo639 commented Oct 30, 2023

Btw, exec condition statements are also a thing.
You can allow people to disable pipewire-pulse and wireplumber as demonstrated here:
https://gitlab.freedesktop.org/pipewire/pipewire/-/issues/3160#note_1870376
condition = [ { exec.session-manager = null } { exec.session-manager = true } ]
condition = [ { exec.pipewire-pulse = null } { exec.pipewire-pulse = true } ]

Idk if it is also possible to use this to prevent the file from getting exec'd multiple times if the file was symlinked multiple times.

@cinerea0
Copy link
Contributor

Yeah, the conditional execution could work. Here's how I start the pulse server and wireplumber from my user's config directory:

context.exec = [
	{ path = "/usr/bin/wireplumber" args = ""
	  condition = [ { exec.session-manager = null } { exec.session-manager = true } ] }
	{ path = "/usr/bin/pipewire" args = "-c pipewire-pulse.conf"
	  condition = [ { exec.pipewire-pulse = null } { exec.pipewire-pulse = true } ] }
]

I'd have to test it, but based on how it's documented switching the files to this shouldn't launch wireplumber or the pulse server if they're already running, even if it's the same file getting symlinked multiple times.

@cinerea0
Copy link
Contributor

I tested this PR on pipewire version 0.3.84 with the conditional execution statements included. Unfortunately, conditional execution does not work the way I thought it did. If wireplumber is started in a file in /usr/share/pipewire/pipewire.conf.d and in another file/link in a different priority directory two instances of wireplumber will start, even with the conditional statements.

The mechanics of how those work aren't especially well documented. Given this section in the man page for pipewire.conf:

Array of dictionaries. Each entry in the array is dictionary
containing the path of a program to execute on startup and
optional args.

This array used to contain an entry to start the session manager
but this mode of operation has since been demoted to development
aid. Avoid starting a session manager in this way in production
environment.

I don't think this behavior would be supported by upstream. However, given the persistent confusion around pipewire I would still support automatically starting wireplumber with the alternatives line and breaking the link, since that does work.

Also, since /usr/share/pipewire/pipewire.conf.avail already contains 10-rates.conf and 20-upmix.conf, should we consider changing the number prefixes for the files this PR would move to that directory?

Copy link

Pull Requests become stale 90 days after last activity and are closed 14 days after that. If this pull request is still relevant bump it or assign it.

@github-actions github-actions bot added the Stale label Feb 17, 2024
@classabbyamp classabbyamp self-assigned this Feb 17, 2024
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.

None yet

4 participants