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
