pulseaudio: Add support for package configuration files. #16834
Conversation
|
@Jookia, thanks for your PR! By analyzing the annotation information on this pull request, we identified @wkennington, @rickynils and @edolstra to be potential reviewers |
nixos/modules/config/pulseaudio.nix
Outdated
|
|
||
| for i in ${toString cfg.packages}; do | ||
| echo "Adding configuration for package $i" | ||
| copy_dir $i/usr/share/pulseaudio/alsa-mixer profile-sets alsa-profiles |
There was a problem hiding this comment.
Using /usr is undesirable in packages -- can this be fixed to just /share/pulseaudio?
There was a problem hiding this comment.
Sure! I only did this to make my packaging easier
|
I'm curious, when are those files needed? Any example of a package which can be added to environment? |
nixos/modules/config/pulseaudio.nix
Outdated
| @@ -135,6 +177,7 @@ in { | |||
|
|
|||
| (mkIf cfg.enable { | |||
| environment.systemPackages = [ cfg.package ]; | |||
| environment.sessionVariables = moduleEnvVars; | |||
There was a problem hiding this comment.
An idea for potential improvement: instead add to environment not raw cfg.package but a wrapped one with makeWrappered binaries adding those variables. Less pollution in the environment and more robustness.
There was a problem hiding this comment.
I'm not sure what you mean here exactly- the module packages have no binaries, and these variables just tell pulseaudio where to look for the module configuration. If you mean wrapping pulseaudio itself to look for moduleEnvVars wouldn't that require building pulseaudio each change?
There was a problem hiding this comment.
I mean wrapping pulseaudio, but not itself -- rather as a part of a wrapper (see deadbeef, pidgin, dwarf-therapist etc.) The main idea is:
symlinkJoin {
name = "pulseaudio-with-packages";
paths = [ cfg.package ] ++ cfg.packages;
buildInputs = [ makeWrapper ];
postBuild = ''
wrapProgram $out/bin/pulseaudio \
--set PA_ALSA_PATHS_DIR ... \
--set PA_ALSA_PROFILE_SETS_DIR ....
'';
}
, and this goes directly to `systemPackages`. This drops need for system-wide environment variables and may make the whole thing simpler (because most of `buildCommand` code above should not be needed this way -- if I'm not mistaking anything!).
There was a problem hiding this comment.
Sounds like a good idea :) I'll implement it along with your other suggestions when I can then redo my commit.
|
An example package is https://github.com/xobs/pulseaudio-novena which is the main reason I've done this |
|
So months later I've come back to this finally since the current patch is broken beyond belief outside my ARM machine. Wrapping the pulseaudio binaries causes some grief, but ultimately isn't doable because some packages use 'cfg.pulseaudio.package' to run pulseaudio binaries, which wouldn't be the wrapped version. Instead I've dropped the environment variables patch and now just get pulseaudio to look in /share/pulse/alsa-mixer for the alsa-mixer data which is generated the same, this is done by editing the configure.ac file to change the defines. The only other way to do this would be to get pulseaudio's alsa-mixer module to support taking an paths-dir argument. Indeed, some of the code looks to be set up for such a situation. Doing this would mean it'd go in the default.pa file generated. This would be the ideal situation, but it doesn't solve the issue for other modules. I'll put up a patch in a week or so when I can test it out on my ARM machine where it actually needs to work. |
In a fashion like udev's support, this patch allows configurations from packages to be merged in to directories for PulseAudio to read from. Currently supported directories are the alsa-mixer mdoule's profile-sets and paths. This is accomplished by configuring the PulseAudio daemon to read directories from /etc/pulse/alsa-mixer, and having NixOS generate that directory.
|
27 days later I've pushed the newer cleaned up version, which works on my ARM machine. It's much simpler and does the job. |
|
@Jookia It looks like this has gone under the radar for way too long… are you still interested in moving this forward? |
|
On Sun, Dec 02, 2018 at 02:19:12AM -0800, Léo Gaspard wrote:
@Jookia It looks like this has gone under the radar for way too long… are you still interested in moving this forward?
Not any more, I don't use NixOS. If I have any other issues/PRs here
those should probably be closed too.
|
|
OK thank you! sorry for not having moved that forward when you were still willing to push it forward, and hope someone else will come upon this if they want the same thing :) |
In a fashion like udev's support, this patch allows configurations from packages
to be merged in to directories for PulseAudio to read from. Currently supported
directories are the alsa-mixer mdoule's profile-sets and paths.
This is accomplished by patching PulseAudio to read directories from environment
variables globally defined by NixOS rather from the data path in the Nix store.
Modules that support it (such as the alsa module) can still be configured at
runtime to use specific paths, this just changes the default paths.
The environment variables are only used if they're defined, as such the previous
behaviour can be reverted to if the variables are unset or NixOS isn't running.