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

Pulseaudio by default, if sound is enabled #35292

Closed
wants to merge 4 commits into from

Conversation

aristidb
Copy link
Contributor

@aristidb aristidb commented Feb 21, 2018

Motivation for this change

Major applications like Firefox no longer support a bare-alsa setup, the justification being that only a very small percentage of Linux users does not use pulseaudio.

This pull request still allows people who really want to to disable pulseaudio, but makes it the default on systems with sound enabled. I think it will make it easier for people to get started with NixOS if their sound works out of the box.

Minimal headless systems will also be unaffected, because they should already have sound disabled.

Previous discussion in #29250, which was closed by me due to lack of consensus, but at the time, Firefox did not yet require PulseAudio.

EDIT: I misunderstood a bit, Firefox will still apparently run with Alsa, but it's not maintained and if there's bugs with the Alsa support, there's no guarantee anybody will fix it. The thing that triggered nixpkgs to build Firefox with Pulseaudio is that some playback quality is clipped in some scenarios.

Things done
  • Tested using sandboxing (nix.useSandbox on NixOS, or option build-use-sandbox in nix.conf on non-NixOS)
  • 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 nox --run "nox-review wip"
  • [N/A] Tested execution of all binary files (usually in ./result/bin/)
  • Fits CONTRIBUTING.md.

@aristidb
Copy link
Contributor Author

Relevant tickets: #29250 , #35005 , #25077 - and more

@grahamc
Copy link
Member

grahamc commented Feb 21, 2018

cc @fpletz @vcunat for a decision on 18.03

@@ -87,7 +87,8 @@ in {
hardware.pulseaudio = {
enable = mkOption {
type = types.bool;
default = false;
default = config.sound.enable;
Copy link
Member

Choose a reason for hiding this comment

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

It's IMHO better to set hardware.pulseaudio.enable in the config attribute of this module with lib.mkDefault. Otherwise the manual will be regenerated if the user sets sound.enable to false.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

OK, sure. I will need to do some more testing once my computer is done with some bigger rebuilds.

@fpletz
Copy link
Member

fpletz commented Feb 21, 2018

While we're at the sound topic: I wonder if it's better to disable sound by default but enable it in the template configuration.nix from nixos-generate-config. This will be a breaking change and hard to detect however, so it could make some desktop users angry that don't explicitly enable sound.

@coreyoconnor
Copy link
Contributor

I would prefer an interface like:

  1. enabling sound means "enable and use audio interfaces"
  2. enabling sound default sound implementation to "pulseaudio"
  3. A sound implementation of "pulseaudio" enables pulseaudio and alsa.

@samueldr
Copy link
Member

samueldr commented Feb 21, 2018

With regards to @fpletz's disabling sound by default, would using the stateVersion >= 18.03 to default to false otherwise default to true be sane?

@fpletz
Copy link
Member

fpletz commented Feb 21, 2018

@samueldr Definitely, but will everyone read the release notes? 😄

@samueldr
Copy link
Member

@fpletz well:

  • If they don't read them and don't update stateVersion, nothing's changed.
  • If they don't read them and they update stateVersion, it's their fault like every other stateVersion issues.
  • If they read them, everything should be fine, right? 😸

@Mic92
Copy link
Member

Mic92 commented Feb 21, 2018

stateVersion should be fine

@oxij
Copy link
Member

oxij commented Feb 21, 2018 via email

@adisbladis
Copy link
Member

While we're at the sound topic: I wonder if it's better to disable sound by default but enable it in the template configuration.nix from nixos-generate-config. This will be a breaking change and hard to detect however, so it could make some desktop users angry that don't explicitly enable sound.

@fpletz We should enable sound if config.services.xserver.enable is true.
That would be a pretty smooth transition for most people who want sound.

@oxij
Copy link
Member

oxij commented Feb 22, 2018 via email

@adisbladis
Copy link
Member

Seriously, are you serious? Do displayless systems with speakers not exists in your universe?

I'm talking about a sensible default. If you run a headless system with sound output you can easily enable it.

@oxij
Copy link
Member

oxij commented Feb 22, 2018 via email

@fpletz
Copy link
Member

fpletz commented Feb 22, 2018

Could we please stop with this religious and condescending discussion style and agree that we have users that want ALSA xor Pulseaudio and try to reach a consensus that works for everyone?

Regarding disabling sound by default: I only brought it up because servers, VMs and containers generally don't need to have sound support. The sound module would add alsaUtils to the system closure and an alsa-store service which is not needed at all. This has nothing to do with a change to pulseaudio. I probably shouldn't have brought it up here, sorry.

It is my impression that right now we build most software with pulseaudio if it is supported. So why is that the case?

For instance, I do actually have problems with the ALSA pulseaudio plugin if I'm streaming to remote pulseaudio servers over the network. You might say now that pulseaudio is of course broken but that is not helpful for me because the network streaming part of pulseaudio generally works really well for my use case.

Treating libpulseaudio like libGL sounds like interesting idea to explore. Does anyone know the libpulseaudio API well enough to know if we would run into trouble here? Does pressureaudio mock/implement the complete libpulseaudio API and are the ABIs compatible?

Does anyone know if the libpulseaudio API exposes functionality that is needed and can't be implemented or emulated via the ALSA plugin?

The reality is that most NixOS desktop users counting by the votes in all the PRs and issues that are linked here are using Pulseaudio. As a compromise, why not go a similar route here as with the proposed solution to sound.enable by keeping the current hardware.pulseaudio.enable default (false) and add a line to the configuration.nix by nixos-generate-config to make new users more aware of what options are available?

@ghost
Copy link

ghost commented Feb 22, 2018

Major applications like Firefox

BTW Chrome/Chromium works fine without PA
[ Firefox still threats Linux as second-class citizen, providing poor graphics acceleration support :-( Both lack vaapi support though ]

@oxij
Copy link
Member

oxij commented Feb 22, 2018 via email

@aristidb
Copy link
Contributor Author

I will create a separate pull request to disable sound by default if stateVersion >= 18.03

@oxij
Copy link
Member

oxij commented Feb 22, 2018 via email

@aristidb
Copy link
Contributor Author

@fpletz I'm not completely opposed to the compromise of making nixos-generate-config inform the user about hardware.pulseaudio, but currently the strong majority (except of course @oxij) seems to prefer changing the default of hardware.pulseaudio.

@7c6f434c
Copy link
Member

Well, given existence of opposition and lack of opinion from Eelco Dolstra, Rob Vermaas or Domen Kozar, if it were an RFC it would stall forever.

Unfortunately, it is not an RFC and so it might actually proceed.

@grahamc
Copy link
Member

grahamc commented Feb 22, 2018

stateVersion ... nobody uses.

People don't use stateVersion, the NixOS module system uses stateVersion.

@aristidb
Copy link
Contributor Author

@grahamc I don't follow, but let's put that discussion in the upcoming separate PR.

@grahamc
Copy link
Member

grahamc commented Feb 22, 2018

That is fine, it wasn't for you -- but for oxij's assertion that nobody uses stateVersion.

@aristidb
Copy link
Contributor Author

Let's please take all discussion about disabling sound by default to #35355

@fpletz
Copy link
Member

fpletz commented Feb 22, 2018

As we're informing new users about the options with the merge of #35355, I think we can close this for now. Thanks to everyone for their input.

@oxij
Copy link
Member

oxij commented Feb 23, 2018

The libGL idea implemented: #35374.

@vcunat vcunat deleted the pulseaudio-by-default branch February 23, 2018 20:20
@oxij
Copy link
Member

oxij commented Feb 24, 2018 via email

@7c6f434c
Copy link
Member

@oxij I wonder how many of the people you mention still use mainline NixOS, though. I mean, PulseAudio is strictly less intrusive than systemd.

@oxij
Copy link
Member

oxij commented Feb 24, 2018 via email

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

10 participants