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

pipewire: update to 0.3.71, drop pipewire-media-session #43574

Closed
wants to merge 3 commits into from

Conversation

cinerea0
Copy link
Contributor

Testing the changes

  • I tested the changes in this PR: YES

@cinerea0
Copy link
Contributor Author

This release of pipewire added functionality to conditionally start a session manager. I want to draw attention to this comment by a pipewire dev on the issue that led to this feature being implemented, which references problems resulting from autostarting pipewire-media-session:

Yes, the way you did will end up running both. You will need to fully override the main pipewire.conf file instead. Alternatively either convince your distro to:

  • stop using dead software
  • adopt a launcher script or daemon like they should have long ago
  • just switch to a better distro (and, if no, you can keep your reasoning to yourself, I can already imagine them without you typing them out but it still needed to be said).

Given that pipewire contributors are now regularly referring to p-m-s as "dead software", I think we have three options:

  1. Continue to leave things as they are.
  2. Change the patch to only start p-m-s conditionally and update the documentation to reference the solution in the linked issue.
  3. Drop p-m-s as in [RFC] pipewire: update, add virtual session manager to pull in wireplumber without build cycle #41868, even though it still builds fine.

@ahesford
Copy link
Member

ahesford commented May 6, 2023

I think it's time to drop pipewire-media-session.

Edit: we sure as hell shouldn't start patching it, so the choice is between leaving it alone or dropping it altogether.

@oreo639
Copy link
Member

oreo639 commented May 6, 2023

we sure as hell shouldn't start patching it, so the choice is between leaving it alone or dropping it altogether.

It isn't about adding a new patch, it is about updating the existing path for this:
https://gitlab.freedesktop.org/pipewire/pipewire/-/commit/27bc60aeab83a91d4eae3be14f6eff4f67552d02

See here in the diff:
@sm_comment@ condition = [ { exec.session-manager = null } { exec.session-manager = true } ]
(i.e. should the @sm_comment@ be there or not)

Unless I am misunderstanding you?

(imo, it should be fine to have the condition and that would avoid having to replace the pipewire.conf)

@ahesford
Copy link
Member

ahesford commented May 6, 2023

Upstream has been telling us unequivocally for a long time to stop using this program. We need to rip off this Band-Aid and stop patching our configs to do things we shouldn't have been doing in the first place. We are now at the point where we are causing problems for upstream because our users are complaining to them when our problematic configuration changes break. This is inconsiderate and should not be allowed to continue.

The marginal benefit of keeping p-m-s going and adopting the conditional change is that some users who have not already switched to wireplumber in the months we've been warning them to do so, but will suddenly decide to do so before we drop p-m-s, can do so with a drop-in configuration snippet that will become a no-op after we actually drop the session manager.

  1. Those that have already switched to WirePlumber will have already shadowed the system configuration. This isn't a major issue, but can be a minor inconvenience because they may not notice other configuration changes that would have to be reconciled with their overrides.
  2. All remaining users will still have their pipewire configuration break at some point in the future when p-m-s stops working properly.

@marmeladema
Copy link
Contributor

I concur, it would be really great to drop pms. This has been deprecated for ages and yet, in every Void Linux installation, we have to disable it manually to run wireplumber instead.
Half of the time, I do it wrong and end up with a configuration that starts both :(
Keeping pms doesn't really help anyone at this point and just puts burden on upstream for no good reasons.

@cinerea0
Copy link
Contributor Author

I agree that the best choice at this point is to drop p-m-s. The install message has been around for 4 package updates and almost 5 months, so it's not like people haven't been given warning. There absolutely will be people whose setups break with this, but I don't think anything besides a breakage would get them to change at this point.

I'm temporarily marking this as a draft while I test the new 0.3.71 release. I'd be happy to include the changes from the above-linked PR in this one if that would be the best way forward.

@cinerea0 cinerea0 marked this pull request as draft May 17, 2023 18:18
@ahesford
Copy link
Member

I think my approach is the easiest way to avoid a build cycle, so go ahead and pull that into this PR if you'd like.

This dummy package will be the default provider for the the session
manager pulled in by pipewire, breaking a build cycle.
Now depends on virtual session manager
@cinerea0 cinerea0 changed the title pipewire: update to 0.3.70 pipewire: update to 0.3.71, drop pipewire-media-session May 20, 2023
@cinerea0 cinerea0 marked this pull request as ready for review May 20, 2023 02:39
@ahesford ahesford closed this in 43889c8 May 24, 2023
@cinerea0 cinerea0 deleted the pipewire branch May 26, 2023 17:16
sirkhancision pushed a commit to sirkhancision/void-packages that referenced this pull request Jun 10, 2023
Now depends on virtual session manager

Closes: void-linux#43574 [via git-merge-pr]
sirkhancision pushed a commit to sirkhancision/void-packages that referenced this pull request Jun 12, 2023
Now depends on virtual session manager

Closes: void-linux#43574 [via git-merge-pr]
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.

4 participants