[pulseaudio-discuss] [PATCH] Add env vars for PA_ALSA_PATHS_DIR and PA_ALSA_PROFILE_SETS_DIR

[Albert Astals Cid]

On Fri, Dec 2, 2016 at 3:25 PM, Tanu Kaskinen <tanuk at iki.fi> wrote:
> On Thu, 2016-12-01 at 12:55 +0100, Albert Astals Cid wrote:
>> commit 5621ce4ecc63c289a99ec09cddd8088fa65db726
>> Author: Albert Astals Cid <albert.astals at canonical.com>
>> Date:   Wed Nov 23 17:03:41 2016 +0100
>
> "git am" doesn't recognize this formatting. Could you use "git send-
> email" for submitting patches? Instructions are available here:
> https://www.freedesktop.org/wiki/Software/PulseAudio/HowToUseGitSendEmail/
>
>>     Add env var for data dir
>>
>>     Helps making pulseaudio relocatable, i.e. having the code and data in a different
>>     root path than the one that was decided on compilation time
>
> I asked you to provide a use case in the commit message, but you only
> added an explanation for what "relocatable" means. If want to make a
> snap for PulseAudio and being relocatable helps with that, say it in
> the commit message.
>
> Also explain *why* you want to make a snap for PulseAudio, because I
> don't really see a use case for that. AFAIK, the usual reason for
> distributing applications as snaps is that snaps work on any
> distribution and they run in a secure sandbox. However, the model
> doesn't work well for system daemons. How are applications expected to
> connect to the sandboxed PulseAudio? Do you export the socket from the
> PulseAudio sandbox? If so, how do you avoid conflicts with the
> operating system's own PulseAudio?

This is not so much a snap for  PulseAudio than a snap with PulseAudio in it.

There's also no "operating system's own PulseAudio" since the snap i
am working on is basically a "desktop" snap.

And yes, the plan AFAIK is exporting the socket so that other apps
that need it can use it.

> Another problem is that if you want to use udev, that's not portable,
> because mixing different versions of libudev and udevd is not expected
> to work.

One problem at a time :)

>
> By the way, is it common for snaps to bundle unaltered binary debs from
> Ubuntu? A lot of software has files in /usr/share, and if snaps can't
> install their files there, I'd expect a lot of debs to not be
> relocatable.

It's an easy and common way to start, yes. Can't comment on if other
software has trouble or not, but there's quite some that work fine.

>> diff --git a/src/modules/alsa/alsa-mixer.c b/src/modules/alsa/alsa-mixer.c
>> index 3dbf6b1..da5b657 100644
>> --- a/src/modules/alsa/alsa-mixer.c
>> +++ b/src/modules/alsa/alsa-mixer.c
>> @@ -2507,8 +2507,17 @@ static int path_verify(pa_alsa_path *p) {
>>  static const char *get_default_paths_dir(void) {
>>      if (pa_run_from_build_tree())
>>          return PA_SRCDIR "/modules/alsa/mixer/paths/";
>> -    else
>> -        return PA_ALSA_PATHS_DIR;
>> +    else {
>> +        static const char *paths_dir = NULL;
>> +        if (!paths_dir) {
>> +            paths_dir = getenv("PULSE_DATA_PATH");
>> +            if (!paths_dir)
>> +                paths_dir = PA_ALSA_PATHS_DIR;
>> +            else
>> +                paths_dir = pa_sprintf_malloc("%s/alsa-mixer/paths", paths_dir);
>
> paths_dir is never freed. We try to keep PulseAudio Valgrind-clean, so
> I think this should be handled in some other way. I think it would be
> best to require the function caller to free the returned string.

Yeah i saw that but since it's basically a "one alloc for the whole
run" i thought it would be ok, but i understand your POV.

I'll see if making the caller free the string is ok (not to make it
malloc/free too often in case this is a hot-ish path).

>
> Another solution that I'd be ok with would be to save the datadir in
> pa_core when pulseaudio starts. I think the alsa-mixer code doesn't
> currently always have a pointer to pa_core, but that's probably not
> difficult to change.
>
>> +        }
>> +        return paths_dir;
>> +    }
>>  }
>>
>>  pa_alsa_path* pa_alsa_path_new(const char *paths_dir, const char *fname, pa_alsa_direction_t direction) {
>> @@ -4333,6 +4342,23 @@ void pa_alsa_decibel_fix_dump(pa_alsa_decibel_fix *db_fix) {
>>      pa_xfree(db_values);
>>  }
>>
>> +static const char *alsa_profile_sets_dir() {
>> +    if (pa_run_from_build_tree())
>> +        return PA_SRCDIR "/modules/alsa/mixer/profile-sets/";
>> +    else {
>> +        static const char *profile_sets_dir = NULL;
>> +        if (!profile_sets_dir) {
>> +            profile_sets_dir = getenv("PULSE_DATA_PATH");
>> +            if (!profile_sets_dir)
>> +                profile_sets_dir = PA_ALSA_PROFILE_SETS_DIR;
>> +            else
>> +                profile_sets_dir = pa_sprintf_malloc("%s/alsa-mixer/profile-sets", profile_sets_dir);
>
> Same comment as above.
>
> --
> Tanu
>
> https://www.patreon.com/tanuk
> _______________________________________________
> pulseaudio-discuss mailing list
> pulseaudio-discuss at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/pulseaudio-discuss