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: 0.3.13 -> 0.3.15 #102514

Merged
merged 1 commit into from Nov 5, 2020
Merged

pipewire: 0.3.13 -> 0.3.15 #102514

merged 1 commit into from Nov 5, 2020

Conversation

bbigras
Copy link
Contributor

@bbigras bbigras commented Nov 2, 2020

Motivation for this change
Things done
  • Tested using sandboxing (nix.useSandbox on NixOS, or option sandbox in nix.conf on non-NixOS linux)
  • Built on platform(s)
    • NixOS
    • macOS
    • other Linux distributions
  • Tested via one or more NixOS test(s) if existing and applicable for the change (look inside nixos/tests)
  • Tested compilation of all pkgs that depend on this change using nix-shell -p nixpkgs-review --run "nixpkgs-review wip"
  • Tested execution of all binary files (usually in ./result/bin/)
  • Determined the impact on package closure size (by running nix path-info -S before and after)
  • Ensured that relevant documentation is up to date
  • Fits CONTRIBUTING.md.

/nix/store/whfvrjrv3wdcz8bfybm9ha7vclka8knc-pipewire-0.3.13 326.9M
/nix/store/k9fnfpf6s7rjb3k7yv6230gn0yry1hnm-pipewire-0.3.14 327.1M

cd nixpkgs/nixos/tests/installed-tests
nix-build -A pipewire

[...]
machine # [   22.407894] gnome-desktop-testing-runner[679]: PASS: pipewire-0.3/spa-benchmark-dict.test
machine # [   22.423440] gnome-desktop-testing-runner[679]: PASS: pipewire-0.3/pw-test-client.test
machine # [   22.438886] gnome-desktop-testing-runner[679]: PASS: pipewire-0.3/pw-test-properties.test
machine # [   22.505593] gnome-desktop-testing-runner[679]: PASS: pipewire-0.3/pw-test-context.test
machine # [   23.001419] gnome-desktop-testing-runner[679]: Executing: pipewire-0.3/spa-stress-ringbuffer.test
machine # [   24.529835] gnome-desktop-testing-runner[679]: PASS: pipewire-0.3/spa-stress-ringbuffer.test
machine # [   24.545912] gnome-desktop-testing-runner[679]: PASS: pipewire-0.3/spa-test-node.test
machine # [   24.559084] gnome-desktop-testing-runner[679]: PASS: pipewire-0.3/spa-test-utils.test
machine # [   24.575278] gnome-desktop-testing-runner[679]: PASS: pipewire-0.3/pw-test-array.test
machine # [   24.584764] gnome-desktop-testing-runner[679]: SUMMARY: total=25; passed=25; skipped=0; failed=0; user=4.4s; system=11.1s; maxrss=5444
(25.83 seconds)
(25.83 seconds)
test script finished in 25.85s
cleaning up
killing machine (pid 9)
(0.00 seconds)
/nix/store/80l3257a0xl15ciklmcd2g807brfw28x-vm-test-run-pipewire-0.3.14

jtojnar
jtojnar previously approved these changes Nov 2, 2020
Copy link
Contributor

@jtojnar jtojnar left a comment

Choose a reason for hiding this comment

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

Changes look okay, the pulse-server might be of interest to people from #93725
https://gitlab.freedesktop.org/pipewire/pipewire/-/blob/0.3.14/NEWS#L1-97

@jansol
Copy link
Contributor

jansol commented Nov 2, 2020

How is that server used? Asking so I can document it in #102547.

@bbigras
Copy link
Contributor Author

bbigras commented Nov 3, 2020

How is that server used?

I have no idea.

@jtojnar
Copy link
Contributor

jtojnar commented Nov 3, 2020

Skimmitng the commit log, it seems to create a socket pulseaudio would:

https://gitlab.freedesktop.org/pipewire/pipewire/-/commit/d180b8d9272b72815d28fc4c56b962a4c53fa713#f196078120be2e82814fa95fe9498f38c957f3b0_0_669

No idea if it is enabled by default or how it can be enabled.

It also looks like it was later split into a separate module:

https://gitlab.freedesktop.org/pipewire/pipewire/-/commit/f0102fd490b7da4bce9cb4f1b2a62de95d122269

but I do not see any new files in out, lib or pulse outputs.

@jtojnar
Copy link
Contributor

jtojnar commented Nov 3, 2020

Turns out I had binaries filtered out in Meld, it is installed to $lib/lib/pipewire-0.3 so we will need to add load-module directive to the config file.

Of course, this means we will need to make pipewire load a config from /etc so we will need a patch like https://github.com/NixOS/nixpkgs/blob/36a0c931ffcdebca713f3bfd41215afd972ed891/pkgs/os-specific/linux/firmware/fwupd/add-option-for-installation-sysconfdir.patch to distinguish between installation and runtime paths. And then generate the config file in a module and link all those unedited files using environment.etc option.

@jtojnar jtojnar added the 8.has: upstream changes reviewed Reviewer checked the changelogs/commit logs associated with the release and did not find any issues. label Nov 3, 2020
@jansol
Copy link
Contributor

jansol commented Nov 3, 2020

Looks like the fake pulse server is not meant to be usable quite yet and should be disabled: https://gitlab.freedesktop.org/pipewire/pipewire/-/issues/360

This PR should be changed to disable it or wait for that next release this week mentioned in that issue. Then again getting the NixOS module stuff in order would be nice, so it is ready to go as soon as upstream dares to enable it by default again.

@jtojnar jtojnar dismissed their stale review November 3, 2020 13:55

hmm, we will need to cherry-pick the patch or wait for that release then

@SuperSandro2000
Copy link
Member

Result of nixpkgs-review pr 102514 run on x86_64-darwin 1

5 packages marked as broken and skipped:
  • firefox-esr-78-unwrapped
  • firefox-unwrapped
  • firefoxPackages.firefox
  • firefoxPackages.firefox-esr-78
  • xulrunner

@jansol
Copy link
Contributor

jansol commented Nov 5, 2020

@bbigras bbigras changed the title pipewire: 0.3.13 -> 0.3.14 pipewire: 0.3.13 -> 0.3.15 Nov 5, 2020
@jansol
Copy link
Contributor

jansol commented Nov 5, 2020

Oh,

Disable the bluez5 and pulse-bridge modules by default because
they interfere with pulseaudio. These options should only be
enabled if pulseaudio is removed or disabled in the system.

Since the pipewire module in NixOS does not allow having pulseaudio at the same time as the pipewire pulse shim we can just enable the bridge module and adjust these assertions to force pulseaudio off when pipewire is enabled. Wiring up the relevant config options and nix infrastructure for this would be good at least. Can you do that @bbigras? If not, I'd say just bump to 0.3.15 and the pulse-bridge stuff can be handled in a separate PR.

@bbigras
Copy link
Contributor Author

bbigras commented Nov 5, 2020

I'll take a look in the morning.

@jtojnar
Copy link
Contributor

jtojnar commented Nov 5, 2020

Pipewire is required for screencasting on Wayland so we cannot just make it mutually exclusive with Pulseaudio. We would need to enable it in module as described above but yeah, this is probably too much for now.

@ofborg ofborg bot requested a review from jtojnar November 5, 2020 09:14
@bbigras
Copy link
Contributor Author

bbigras commented Nov 5, 2020

So should we merge this PR as is?

@jansol
Copy link
Contributor

jansol commented Nov 5, 2020

Yeah, let's have a separate PR for the module stuff. Since the pulse-bridge stuff is off by default this PR shouldn't change anything from the current state (where it doesn't exist to begin with).

@jtojnar jtojnar merged commit da09d65 into NixOS:master Nov 5, 2020
@jtojnar
Copy link
Contributor

jtojnar commented Nov 5, 2020

Thanks.

@bbigras bbigras deleted the pipewire branch November 5, 2020 20:16
@bbigras
Copy link
Contributor Author

bbigras commented Nov 5, 2020

Thanks everyone.

@jansol jansol mentioned this pull request Nov 9, 2020
5 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
8.has: upstream changes reviewed Reviewer checked the changelogs/commit logs associated with the release and did not find any issues. 10.rebuild-darwin: 1-10 10.rebuild-linux: 11-100
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants