date:20151019




On 2015-10-19 14:27, Felipe Sateler wrote:

On 19 October 2015 at 05:41, Tanu Kaskinen  wrote:

(Side note: I would have liked to see the device reservation protocol on
the system bus instead of the session one, but that's likely too late to
change now)


I think it's probably possible to do the transition to the system bus,
if we just can find someone to write patches for both PulseAudio and
Jack. During a transition phase we would have to use both buses, to
cooperate with applications using only the session bus.


Note that now that everyone is moving to a user instead of session
bus,


Are we? :-) I haven't heard anything about e g Ubuntu switching over, 
but I could have just missed it?



the benefits of moving to the system bus are reduced.


For me the main use case would still be collaboration between a 
user-level PulseAudio and system-level daemon(s). Which does not change 
with the move from session to user.



Also, who
would ship the dbus/polkit policy to allow logged in users to own the
name?


Hmm, that's a good point. As a starting point, maybe the same package 
that gives access to the soundcard itself to logged in users?


--
David Henningsson, Canonical Ltd.
https://launchpad.net/~diwic
___
pulseaudio-discuss mailing list
pulseaudio-discuss@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/pulseaudio-discuss

Re: [pulseaudio-discuss] Flat volumes and programmatic volume access (again)

On Tue, 2015-10-20 at 10:34 +0530, Arun Raghavan wrote:
> On Mon, 2015-10-19 at 13:30 +0300, Tanu Kaskinen wrote:
> > On Mon, 2015-10-19 at 15:19 +0530, Arun Raghavan wrote:
> > > * We add a new stream volume API -- pa_stream_get_volume(),
> > > pa_stream_set_volume(), pa_stream_set_volume_callback(). I think
> > > this
> > > is good to have in general, to have a simpler client API. In the
> > > context of this proposal, this allows us to know when an
> > > application is
> > > concerned about the stream volume in the stream context vs. a
> > > global
> > > context. (this follow's from Tanu's proposal to deal with
> > > applications
> > > that play streams as well as implement their own system mixer --
> > > such
> > > as a browser-based system UI might).
> > > 
> > > * It is not clear to me at the moment whether the new volume API
> > > should
> > > be synchronous. pa_stream_volume_get() should be. I think, but I'm
> > > undecided on pa_stream_set_volume().
> > 
> > pa_stream_set_volume() should be asynchronous like everything else
> > that
> > requires a round-trip. pa_stream_get_volume() can be synchronous,
> > because we can cache the stream volume in the client, but we need to
> > think about what should happen when the client doesn't yet know its
> > volume. If the server sends the initial volume in the stream creation
> > reply, this problem doesn't exist, but what should happen with old
> > servers that don't support that? Could the new API be entirely
> > unsupported with older servers? That would mean no volume support in
> > clients that don't fall back to the introspection API... Then again,
> > maybe it's reasonable to recommend all applications to fall back to
> > the
> > introspection API for some time, if they don't have guarantees about
> > the server version.
> 
> I'd rather not have to ask application developers to start worrying
> about server version.
> 
> We could add some fallback code for older versions to defer emitting
> READY on the stream until we query and cache the volume.

That sounds like a very good idea. Deferring the READY state didn't
occur to me.

-- 
Tanu
___
pulseaudio-discuss mailing list
pulseaudio-discuss@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/pulseaudio-discuss

Re: [pulseaudio-discuss] Flat volumes and programmatic volume access (again)

On Mon, 2015-10-19 at 15:30 +0500, Alexander E. Patrakov wrote:
> 19.10.2015 14:49, Arun Raghavan wrote:
[...]
> > * I'd also like to add a policy module to allow blacklisting
> > specific
> > applications, and forcing this behaviour on them. This will need a
> > protocol update to set a stream flag after the stream is connected.
> > 
> > * For legacy apps that are not covered by the blacklist we could
> > add an
> > environment variable that makes all stream and sink-input volume
> > -related bits to use the new behaviour.
> 
> The question here is whether we would later want to force this upon
> all 
> sandboxed applications (xdg-app).

For sandboxing, the approach would probably need to be different --
such as the access control system we've been talking about for xdg-app.

Having the decision and implementation done server-side means that the
same mechanism as the blacklist would work for the permissions system.

-- Arun
___
pulseaudio-discuss mailing list
pulseaudio-discuss@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/pulseaudio-discuss

Re: [pulseaudio-discuss] Flat volumes and programmatic volume access (again)

On Mon, 2015-10-19 at 13:30 +0300, Tanu Kaskinen wrote:
> On Mon, 2015-10-19 at 15:19 +0530, Arun Raghavan wrote:
> > Hi folks,
> > Thought I'd restart this thread since it's been a while. Let me
> > summarise the discussion so far.
> > 
> > The decision to use flat-volumes has certainly been controversial.
> > However, I'm convinced it provides a better user experience than
> > the
> > standard model of having separate per-app and device volumes (even
> > though this is more familiar).
> > 
> > The problem
> 
> I think you meant "a problem" :) Badly behaving applications are not
> the only issue with flat volumes.
> 
> > we face is that some applications set the volume to 100%
> > arbitrarily, and we'd like to avoid having that affect the system
> > volume. Most of the cases where this complaint comes from seems
> > related
> > to applications that allow programmatic access to volume controls
> > (browsers, gnash, etc.).
> > 
> > We discussed a number of solutions in the past, but the one I'd
> > like to
> > take forward is what I'd proposed originally -- a per-stream flag
> > to
> > allow clients to opt out of participating in flat-volume logic. In
> > this
> > way, programs that know they can't make guarantees of being well
> > -behaved. We also need a mechanism to force this on clients that we
> > know aren't well-behaved that we can't change (proprietary and
> > legacy
> > applications).
> > 
> > The idea is that only controlling the stream within the
> > application's
> > volume slider would have this non-flat behaviour. Mixer
> > applications
> > such as pavucontrol would not distinguish this stream from other
> > streams, and changing the volume from there would behave just as
> > any
> > other stream. This should minimise confusion from users'
> > perspective,
> > while disabling the mechanism for rogue applications unexpectedly
> > bump
> > the volume.
> > 
> > That's how I'd like to see the behaviour work. Now let's talk about
> > implementation. The previous RFC patch I'd sent communicates the
> > stream
> > flag for disabling flat volumes to the server, which always
> > disables
> > flat volumes for that stream. This doesn't work with the behaviour
> > I
> > described above. So what I think we should do is:
> > 
> > * Streams set a PA_STREAM_DISABLE_FLAT_VOLUME (or
> > PA_STREAM_PROG_VOLUME_CONTROL or whatever) on the streams for which
> > they want the new behaviour.
> > 
> > * We add a new stream volume API -- pa_stream_get_volume(),
> > pa_stream_set_volume(), pa_stream_set_volume_callback(). I think
> > this
> > is good to have in general, to have a simpler client API. In the
> > context of this proposal, this allows us to know when an
> > application is
> > concerned about the stream volume in the stream context vs. a
> > global
> > context. (this follow's from Tanu's proposal to deal with
> > applications
> > that play streams as well as implement their own system mixer --
> > such
> > as a browser-based system UI might).
> > 
> > * It is not clear to me at the moment whether the new volume API
> > should
> > be synchronous. pa_stream_volume_get() should be. I think, but I'm
> > undecided on pa_stream_set_volume().
> 
> pa_stream_set_volume() should be asynchronous like everything else
> that
> requires a round-trip. pa_stream_get_volume() can be synchronous,
> because we can cache the stream volume in the client, but we need to
> think about what should happen when the client doesn't yet know its
> volume. If the server sends the initial volume in the stream creation
> reply, this problem doesn't exist, but what should happen with old
> servers that don't support that? Could the new API be entirely
> unsupported with older servers? That would mean no volume support in
> clients that don't fall back to the introspection API... Then again,
> maybe it's reasonable to recommend all applications to fall back to
> the
> introspection API for some time, if they don't have guarantees about
> the server version.

I'd rather not have to ask application developers to start worrying
about server version.

We could add some fallback code for older versions to defer emitting
READY on the stream until we query and cache the volume.

> > * I'm inclined to keep the implementation of the relative volume
> > calculation server-side,
> 
> If we want a policy module to be able to enforce the behaviour at the
> server side, I don't see any other way.

That's true indeed, for older clients.

> > in order to keep the client library simple. To
> > do this, pa_stream_volume_get/set() could reuse the same protocol
> > as
> > the pa_context_* API but set a flag to let the server know the
> > client
> > wants relative volumes.
> 
> How is this compatible with the mixer-app-in-browser use case? How
> can
> the mixer app get the absolute volume, if the browser has set the
> relative volume flag for a video app? I think the introspection API
> should stay unaffected by the relative volume flag.

Your next reply clarif

Re: [pulseaudio-discuss] flat volumes for privileged apps

On Mon, 2015-10-19 at 16:38 +0200, Wim Taymans wrote:
> Hi all,
> 
> Now that we are talking about flat volumes again (but I don't want to
> hijack the other thread), I would like to present another alternative
> to fix
> the problems with flat-volumes.
> 
> The idea is that all apps, by default, operate in non-flat volume
> mode.
> This means all volume control done from the app is relative to the
> master
> volume.
> 
> Privileged apps can see flat-volumes and thus (indirectly) change the
> master volume. One such privileged app is the volume control applet
> but
> it could be possible to manually enable trusted apps (maybe with a
> switch
> in the volume control next to the app stream).
> 
> I made a little hack to let you try this, gnome-control-center is a
> hardcoded
> privileged app but you can see how we can store that in the database
> later
> or how we can hook this into the security framework.
> 
> http://cgit.freedesktop.org/~wtay/pulseaudio/commit/?h=flat-volume-pr
> ivilege-hack&id=1b203fe6bcc8bba1db1911fd4dbf225f36a6dbb9
> 
> I like this idea because:
> 
> 1) it does not need any new api or changes to apps
> 2) sets a default that will not cause 100% master volume with
> misbehaving
> apps
> 3) has the master/app volume separation that people understand and
> that is
> also exposed in apps (volume in totem, master in gnome-shell
> header).
> 4) still exposes the flat-volume model if needed, which is IMHO the
> only way
> to sanely increase the volume of just 1 single app (when it needs
> adjusting
> the master volume).
> 5) minimal code changes.
> 
> What do you think?

I think this option is more or less the same as disabling flat volumes.
The user is back to having at least two actions to control volume
(either app slider and panel slider, or open panel and adjust app
slider).

The point of flat volumes was that if you're using the application
volume slider, you don't need to hunt for two volume sliders to adjust
to get the full range of volume that your hardware allows. To my mind,
your proposal makes sense as an alternative to what I suggested only if
we move to a model where we suggests applications do _not_ have volume
sliders at all, and then design the desktop UX differently to provide a
single-action way to adjust volumes.

As an example, the shell could track what the current playing stream is
based on the foreground application and volume controls (hardware and
panel mixer) could track that volume and apply changes as flat volumes.
So the user is always only dealing with one control, by default. (I
just thought of this, so probably needs to be fleshed out to make
sense, or maybe it doesn't at all.)

-- Arun
___
pulseaudio-discuss mailing list
pulseaudio-discuss@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/pulseaudio-discuss

Re: [pulseaudio-discuss] [PATCH 6/8] module-switch-on-port-available: Use input and output names

On Tue, 2015-05-05 at 17:01 +0200, David Henningsson wrote:
> diff --git a/src/modules/module-switch-on-port-available.c 
> b/src/modules/module-switch-on-port-available.c
> index eb8f2d7..8de68a3 100644
> --- a/src/modules/module-switch-on-port-available.c
> +++ b/src/modules/module-switch-on-port-available.c
> @@ -34,6 +35,9 @@ static bool profile_good_for_output(pa_card_profile 
> *profile) {
>  
>  pa_assert(profile);
>  
> +if (!pa_safe_streq(profile->card->active_profile->input_name, 
> profile->input_name))
> +return false;

It should be easy to make this work with profiles that have multiple
sources: just check that the candidate profile has all the sources that
the active profile has.

-- 
Tanu
___
pulseaudio-discuss mailing list
pulseaudio-discuss@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/pulseaudio-discuss

Re: [pulseaudio-discuss] [PATCH 5/8] card-restore: Save and restore "preferred profile" of port

On Tue, 2015-05-05 at 17:01 +0200, David Henningsson wrote:
> diff --git a/src/modules/module-card-restore.c 
> b/src/modules/module-card-restore.c
> index 5c55cec..5d278c1 100644
> --- a/src/modules/module-card-restore.c
> +++ b/src/modules/module-card-restore.c
> @@ -375,8 +385,44 @@ finish:
>  return PA_HOOK_OK;
>  }
>  
> +static const char* profile_name_for_dir(pa_card_profile *cp, pa_direction_t 
> dir) {
> +if (dir == PA_DIRECTION_OUTPUT && cp->output_name)
> +return cp->output_name;
> +if (dir == PA_DIRECTION_INPUT && cp->input_name)
> +return cp->input_name;
> +return cp->name;
> +}
> +
> +static void update_profile_for_port(struct entry *entry, pa_card *card, 
> pa_device_port *p) {
> +struct port_info *p_info;
> +const char *profilename;
> +
> +if (p == NULL)
> +return;
> +
> +if (!(p_info = pa_hashmap_get(entry->ports, p->name))) {
> +p_info = port_info_new(p);
> +pa_assert_se(pa_hashmap_put(entry->ports, p_info->name, p_info) >= 
> 0);
> +}
> +
> +profilename = profile_name_for_dir(card->active_profile, p->direction);
> +if (pa_safe_streq(p_info->profile, profilename))
> +return;
> +
> +pa_xfree(p->preferred_profile);
> +p->preferred_profile = pa_xstrdup(profilename);

I don't think updating the preferred sink or source belongs in module-
card-restore. module-card-restore should only concern itself with
updating the database, and restoring things when new cards appear.
pa_card_set_profile() seems like a better place to me to set
pa_device_port.preferred_device (my previous suggestion to name the
field "preferred_sink" was obviously bad, since we need to cover
sources too). I'd also use add a function to the pa_device_port API to
set the preferred device name, to keep writes to the struct
encapsulated within device-port.c.

-- 
Tanu
___
pulseaudio-discuss mailing list
pulseaudio-discuss@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/pulseaudio-discuss

Re: [pulseaudio-discuss] [PATCH 4/8] device-port: Add preferred_profile field to pa_device_port

On Tue, 2015-05-05 at 17:01 +0200, David Henningsson wrote:
> Signed-off-by: David Henningsson 
> ---
>  src/pulsecore/device-port.c | 1 +
>  src/pulsecore/device-port.h | 1 +
>  2 files changed, 2 insertions(+)
> 
> diff --git a/src/pulsecore/device-port.c b/src/pulsecore/device-port.c
> index cfe2a80..38f7354 100644
> --- a/src/pulsecore/device-port.c
> +++ b/src/pulsecore/device-port.c
> @@ -98,6 +98,7 @@ static void device_port_free(pa_object *o) {
>  if (p->profiles)
>  pa_hashmap_free(p->profiles);
>  
> +pa_xfree(p->preferred_profile);
>  pa_xfree(p->name);
>  pa_xfree(p->description);
>  pa_xfree(p);
> diff --git a/src/pulsecore/device-port.h b/src/pulsecore/device-port.h
> index f35d07c..ec45a54 100644
> --- a/src/pulsecore/device-port.h
> +++ b/src/pulsecore/device-port.h
> @@ -43,6 +43,7 @@ struct pa_device_port {
>  
>  char *name;
>  char *description;
> +char *preferred_profile;
>  
>  unsigned priority;
>  pa_available_t available; /* PA_AVAILABLE_UNKNOWN, 
> PA_AVAILABLE_NO or PA_AVAILABLE_YES */

It's not actually a profile that the port prefers. When the user
changes from stereo to surround, the analog-output-speakers port
doesn't care which profile gets restored, as long as the profile
contains a surround sink. Therefore, the field should be named
preferred_sink.

-- 
Tanu
___
pulseaudio-discuss mailing list
pulseaudio-discuss@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/pulseaudio-discuss

[pulseaudio-discuss] [PATCH v2] thread-mainloop: keep SIGSYS unblocked if currently trapped

2015-10-19 Thread Julien Isorce

Seccomp-BPF uses SIGSYS signal to trigger
the trap handler attached to sys_open.
If the signal is blocked then the kernel kills
the process whenever pulse audio calls 'open'.
The result backtrace is terminating in sys_open.

That's why it is required to keep SIGSYS unblocked
if it is currently unblocked and trapped.

This patch allows to have pulse audio working
in the Chromium sandbox.

Signed-off-by: Julien Isorce 
---
 src/pulse/thread-mainloop.c | 17 +++--
 1 file changed, 15 insertions(+), 2 deletions(-)

diff --git a/src/pulse/thread-mainloop.c b/src/pulse/thread-mainloop.c
index afd0581..232b5d0 100644
--- a/src/pulse/thread-mainloop.c
+++ b/src/pulse/thread-mainloop.c
@@ -77,10 +77,23 @@ static void thread(void *userdata) {
 
 #ifndef OS_IS_WIN32
 sigset_t mask;
+sigset_t prev_mask;
+struct sigaction sa;
 
-/* Make sure that signals are delivered to the main thread */
 sigfillset(&mask);
-pthread_sigmask(SIG_BLOCK, &mask, NULL);
+
+/* If SIGSYS is currently unblocked and trapped
+ * then keep it unblocked. */
+if (!pthread_sigmask(SIG_SETMASK, NULL, &prev_mask) &&
+!sigismember(&prev_mask, SIGSYS) &&
+!sigaction(SIGSYS, NULL, &sa)
+&& sa.sa_handler != SIG_DFL) {
+sigdelset(&mask, SIGSYS);
+}
+
+/* Make sure that signals are delivered to the main thread.
+ * Use SIG_SETMASK because SIG_BLOCK does an union with current set.*/
+pthread_sigmask(SIG_SETMASK, &mask, NULL);
 #endif
 
 pa_mutex_lock(m->mutex);
-- 
1.9.1

___
pulseaudio-discuss mailing list
pulseaudio-discuss@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/pulseaudio-discuss

Re: [pulseaudio-discuss] Device reservation improvement suggestions

2015-10-19 Thread Zeno Endemann

On 19 Oct 2015 09:37:05 +0200, David Henningsson wrote:
> On 2015-10-04 17:05, Zeno Endemann wrote:
>> Hi,
>>
>> I have two suggestions how the device reservation can be improved for
>> app developers wanting to have exclusive access to an alsa card:
>>
>> 1.
>> I knew about this scheme only by accident, so I think the
>> discoverability should be improved. Adding a few lines to the PA
>> developer documentation site ([1]) with a link to [2] would already be
>> good (it took me quite a while to even find [2]).
> 
> Good idea, added now.

Thanks, much better now.

> 
>> Since the most important sound server (PA and jack, right?) are using it
>> and thus it having a good de facto adoption I furthermore suggest adding
>> the spec to the freedesktop.org specifications page. Should I ask about
>> this on the freedesktop mailing list?
>>
>> 2.
>> The spec implies that the reservation is for a whole device (ie. alsa
>> card), so I believe a well-behaving client should not touch a reserved
>> alsa card in any way. But PA only uses the reservation for PCM streams,
>> not for the mixer controls of the card. My suggestion is either fixing
>> PA (my preference) so that it reserves the card before writing any
>> changes to the mixer, or at least making it clear in the spec that the
>> reservation is only for all PCM devices on the corresponding card.
> 
> I agree that PA should reserve the card before opening the mixer. 
> Patches welcome :-) Although you might want to synchronize with Tanu who 
> recently talked about closing the mixer when we lost access to it, that 
> sounds like something that touches similar parts of the code at least.

I had a quick look, but for me to patch it I'd first need to understand
the PA module system better. The module-alsa-card.c (assuming that is
the correct place) already reserves the card in pa__init, but gives up
the reserve after initialization. Extending that so it reserves until
pa__done is easy, but one would also need to deal with how to make that
module properly give up and reacquire a card. Doing that cleanly seems
not that easy without knowing how exactly the module interacts with the
core. Also there are questions like should a reserved card temporarily
remove all profiles, so they cannot be switched to etc.

Maybe I'll look more into it, but no promises ;)

> 
> (Side note: I would have liked to see the device reservation protocol on 
> the system bus instead of the session one, but that's likely too late to 
> change now)
> 

I actually considered also suggesting that change, as for a global
resource it makes logically more sense, but I can also see the practical
advantage of not having to rely on D-Bus being properly configured for that.


Cheers,
Zeno Endemann

___
pulseaudio-discuss mailing list
pulseaudio-discuss@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/pulseaudio-discuss

Re: [pulseaudio-discuss] [PATCH] alsa-mixer: Ignore volume with unexpected number of channels

2015-10-19 Thread Raymond Yau

>>
>> We currently only support one and two channels for volumes, and
>> bail out otherwise. This makes Xonar users unhappy because they
>> have a volume with eight channels, and bailing out means they
>> don't have a path/port at all.
>>
>> This way they will at least have a port, which will in turn make
>> the gnome/unity UI behave better.
>>
>> BugLink: https://bugs.freedesktop.org/show_bug.cgi?id=84983
>> BugLink: https://bugzilla.gnome.org/show_bug.cgi?id=745017
>> Signed-off-by: David Henningsson 
>> ---
>>
>> This is a resend of https://bugs.freedesktop.org/attachment.cgi?id=114250
>> There is a competing patch by Raymond in that bug, but I think his
>> patch is broken, so I'm going to push mine in a week if there are no
>> reviews.
>
>
> Well, here is my review :)
>
> While this should work, this patch means that all those users are
essentially limited to software-based volume inside of PulseAudio. Also,
the master (or whatever multichannel) slider will not be touched by
PulseAudio at all. Which means that it will stay at whatever position that
alsactl init (or alsactl restore) left it at. I.e. a limited volume range
(note: this is not a regression). Which is still better than what we have
now.
>
> Counterproposal: if that's easier to implement than full multi-channel
volume, treat all multichannel volume controls as mono internally,
duplicate the volume across all channels when setting it, and use the first
channel when getting. I.e., instead of full software volume, make "software
balance".
>
> As this counterproposal comes without a patch, and especially since the
"counterprpopsed" improvement can be done later, this should not block the
"going to push in one week" statement.
>

What is the usage of  e->merged_mask  since it is not initialised ?

for (t = 0; t < PA_CHANNEL_POSITION_MAX; t++)
if (PA_CHANNEL_POSITION_MASK(t) & e->merged_mask) {
min_dB[t] = e->min_dB;
max_dB[t] = e->max_dB;
path_volume_channels |=
PA_CHANNEL_POSITION_MASK(t);
}

It seem  the logic try to skip the first control ( virtual master )

https://bugs.launchpad.net/ubuntu/+source/alsa-driver/+bug/1497666

For snd-ca0106 , there is also a virutal master with slaves "Analog xxx
Playback Volume" controls which are ignored by pulseaudio
___
pulseaudio-discuss mailing list
pulseaudio-discuss@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/pulseaudio-discuss

Re: [pulseaudio-discuss] [PATCH 2/8] card: Add variables for splitting up a profile

On Mon, 2015-10-19 at 17:11 +0200, David Henningsson wrote:
> 
> On 2015-10-19 16:32, Tanu Kaskinen wrote:
> > On Mon, 2015-10-19 at 16:11 +0200, David Henningsson wrote:
> > > 
> > > On 2015-10-19 15:56, Tanu Kaskinen wrote:
> > > > On Tue, 2015-05-05 at 17:01 +0200, David Henningsson wrote:
> > > > > It can be useful for routing modules to know if a profile consists
> > > > > of an output and input part, in order to e g change output profile
> > > > > while keeping the input profile unchanged.
> > > > 
> > > > n_sinks and n_sources already tell if a profile consists of an output
> > > > and input part.
> > > 
> > > It's not only *if* but also *what* the input and output parts are.
> > 
> > Ok, so routing modules may want to know what particular sinks and
> > sources the profile contains. Since a profile may contain multiple
> > sinks or sources, I think the fields should contain all sink/source
> > names, and not only one that is selected according to some unobvious
> > logic.
> 
> Yes. Although more than one sink is currently not implemented (it leaves 
> the field as NULL instead - see later patches).
> 
> > Currently the fields don't contain the actual sink/source name, but an
> > alias (derived from the alsa mapping name) - are there good reasons to
> > prefer an alias rather than the real sink/source name?
> 
> Consistency with the name field, I suppose. Or plain simplicity. I 
> didn't really think of the option of using the real sink/source names 
> instead.
> 
> The sink(s) are not created until the profile is activated, so I haven't 
> checked if it's easy or difficult to use the sink/source names instead.
> 
> The idea is just to make it possible for a routing module to keep "the 
> other side" unchanged - e g, when hdmi is unplugged and the routing 
> module needs to swap to analog speakers (and thus an analog profile), it 
> should try to select a profile which has the same input_name as the 
> current one.
> 
> In the future maybe input_name/output_name can be improved and used for 
> more things.

After reading your reply, I'm not sure if you're planning to implement
any of the proposed changes. I'd really like you to change the single-
name fields to lists (hashmaps or whatever) that contain the ids of all
sinks and sources. Populating the lists should be easy. If the policy
modules can't deal with multi-sink profiles, they can just ignore such
profiles.

Since the names refer to sources and sinks, I suggest that you rename
input_name/output_name to source_names/sink_names, or if it turns out
to be too complicated to use the actual source/sink name, maybe the
field names should be source_ids and sink_ids to not suggest too
strongly that the strings would be actual source/sink names.

-- 
Tanu
___
pulseaudio-discuss mailing list
pulseaudio-discuss@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/pulseaudio-discuss

Re: [pulseaudio-discuss] flat volumes for privileged apps

2015-10-19 Thread Alexander E. Patrakov

19.10.2015 19:38, Wim Taymans wrote:

Hi all,

Now that we are talking about flat volumes again (but I don't want to
hijack the other thread), I would like to present another alternative to fix
the problems with flat-volumes.

The idea is that all apps, by default, operate in non-flat volume mode.
This means all volume control done from the app is relative to the master
volume.

Privileged apps can see flat-volumes and thus (indirectly) change the
master volume. One such privileged app is the volume control applet but
it could be possible to manually enable trusted apps (maybe with a switch
in the volume control next to the app stream).

I made a little hack to let you try this, gnome-control-center is a
hardcoded
privileged app but you can see how we can store that in the database later
or how we can hook this into the security framework.

http://cgit.freedesktop.org/~wtay/pulseaudio/commit/?h=flat-volume-privilege-hack&id=1b203fe6bcc8bba1db1911fd4dbf225f36a6dbb9

I like this idea because:

1) it does not need any new api or changes to apps
2) sets a default that will not cause 100% master volume with misbehaving
apps
3) has the master/app volume separation that people understand and that is
also exposed in apps (volume in totem, master in gnome-shell header).
4) still exposes the flat-volume model if needed, which is IMHO the only way
to sanely increase the volume of just 1 single app (when it needs
adjusting
the master volume).
5) minimal code changes.

What do you think?

I have looked into this patch idea, and I think that even more minimal
changes on pulseaudio side (but not minimal overall) are possible.
However, your approach has an advantage of actually having a patch :)

Please note that, under your proposal, and also without any patch, any
application can introspect and set any sink volume directly, but no
application except dedicated mixer applications currently does this.

So here is a strawman counterproposal that should have a similar effect.
Feel free to test whether the idea is implementable, and compare.

1. All apps operate with non-flat volumes. Thus, we don't have to
implement any policy in pulseaudio, and can even remove all the
flat-volume code.

2. Alter the logic for presenting sink input volumes to the user in
gnome-control-center and all other mixer applications. That is, it
should introspect both sink and sink input volumes, and move the slider
to the correct position according to the product of them. If a user
moves the slider, adjust the sink input volume if possible (i.e. if it
is left of the sink volume). If not, adjust both the sink volume and
volumes of all sink inputs connected there.

I.e., move all flat volume logic from pulseaudio into mixer
applications, like it is done in Windows 7. Then we would have a very
simple rule to enforce if we want to implement privileged/unprivileged
app separation: "privileged app" = "has access to sink volumes".

Yes, I understand that this proposal looks at odds with my previous
"ack" to Arun's idea. That "ack" is still in force.

--
Alexander E. Patrakov
___
pulseaudio-discuss mailing list
pulseaudio-discuss@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/pulseaudio-discuss

Re: [pulseaudio-discuss] [PATCH 2/8] card: Add variables for splitting up a profile




On 2015-10-19 16:32, Tanu Kaskinen wrote:

On Mon, 2015-10-19 at 16:11 +0200, David Henningsson wrote:


On 2015-10-19 15:56, Tanu Kaskinen wrote:

On Tue, 2015-05-05 at 17:01 +0200, David Henningsson wrote:

It can be useful for routing modules to know if a profile consists
of an output and input part, in order to e g change output profile
while keeping the input profile unchanged.


n_sinks and n_sources already tell if a profile consists of an output
and input part.


It's not only *if* but also *what* the input and output parts are.


Ok, so routing modules may want to know what particular sinks and
sources the profile contains. Since a profile may contain multiple
sinks or sources, I think the fields should contain all sink/source
names, and not only one that is selected according to some unobvious
logic.


Yes. Although more than one sink is currently not implemented (it leaves 
the field as NULL instead - see later patches).



Currently the fields don't contain the actual sink/source name, but an
alias (derived from the alsa mapping name) - are there good reasons to
prefer an alias rather than the real sink/source name?


Consistency with the name field, I suppose. Or plain simplicity. I 
didn't really think of the option of using the real sink/source names 
instead.


The sink(s) are not created until the profile is activated, so I haven't 
checked if it's easy or difficult to use the sink/source names instead.


The idea is just to make it possible for a routing module to keep "the 
other side" unchanged - e g, when hdmi is unplugged and the routing 
module needs to swap to analog speakers (and thus an analog profile), it 
should try to select a profile which has the same input_name as the 
current one.


In the future maybe input_name/output_name can be improved and used for 
more things.


--
David Henningsson, Canonical Ltd.
https://launchpad.net/~diwic
___
pulseaudio-discuss mailing list
pulseaudio-discuss@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/pulseaudio-discuss

[pulseaudio-discuss] flat volumes for privileged apps

2015-10-19 Thread Wim Taymans

Hi all,

Now that we are talking about flat volumes again (but I don't want to
hijack the other thread), I would like to present another alternative to fix
the problems with flat-volumes.

The idea is that all apps, by default, operate in non-flat volume mode.
This means all volume control done from the app is relative to the master
volume.

http://cgit.freedesktop.org/~wtay/pulseaudio/commit/?h=flat-volume-privilege-hack&id=1b203fe6bcc8bba1db1911fd4dbf225f36a6dbb9

I like this idea because:

What do you think?

Wim
___
pulseaudio-discuss mailing list
pulseaudio-discuss@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/pulseaudio-discuss

Re: [pulseaudio-discuss] [PATCH 2/8] card: Add variables for splitting up a profile

On Mon, 2015-10-19 at 16:11 +0200, David Henningsson wrote:
> 
> On 2015-10-19 15:56, Tanu Kaskinen wrote:
> > On Tue, 2015-05-05 at 17:01 +0200, David Henningsson wrote:
> > > It can be useful for routing modules to know if a profile consists
> > > of an output and input part, in order to e g change output profile
> > > while keeping the input profile unchanged.
> > 
> > n_sinks and n_sources already tell if a profile consists of an output
> > and input part.
> 
> It's not only *if* but also *what* the input and output parts are.

Ok, so routing modules may want to know what particular sinks and
sources the profile contains. Since a profile may contain multiple
sinks or sources, I think the fields should contain all sink/source
names, and not only one that is selected according to some unobvious
logic.

Currently the fields don't contain the actual sink/source name, but an
alias (derived from the alsa mapping name) - are there good reasons to
prefer an alias rather than the real sink/source name?

-- 
Tanu
___
pulseaudio-discuss mailing list
pulseaudio-discuss@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/pulseaudio-discuss

Re: [pulseaudio-discuss] [PATCH 2/8] card: Add variables for splitting up a profile




On 2015-10-19 15:56, Tanu Kaskinen wrote:

On Tue, 2015-05-05 at 17:01 +0200, David Henningsson wrote:

It can be useful for routing modules to know if a profile consists
of an output and input part, in order to e g change output profile
while keeping the input profile unchanged.


n_sinks and n_sources already tell if a profile consists of an output
and input part.


It's not only *if* but also *what* the input and output parts are.


For now filling in these fields is optional and a routing module
must be able to handle NULL in these fields.

Signed-off-by: David Henningsson 
---
  src/pulsecore/card.c | 2 ++
  src/pulsecore/card.h | 6 ++
  2 files changed, 8 insertions(+)

diff --git a/src/pulsecore/card.c b/src/pulsecore/card.c
index f9a5ddc..dd33847 100644
--- a/src/pulsecore/card.c
+++ b/src/pulsecore/card.c
@@ -52,6 +52,8 @@ pa_card_profile *pa_card_profile_new(const char *name, const 
char *description,
  void pa_card_profile_free(pa_card_profile *c) {
  pa_assert(c);

+pa_xfree(c->input_name);
+pa_xfree(c->output_name);
  pa_xfree(c->name);
  pa_xfree(c->description);
  pa_xfree(c);
diff --git a/src/pulsecore/card.h b/src/pulsecore/card.h
index 3e2c004..8f85000 100644
--- a/src/pulsecore/card.h
+++ b/src/pulsecore/card.h
@@ -40,6 +40,12 @@ typedef struct pa_card_profile {
  char *name;
  char *description;

+/* Indication in case the profile is built from an output and an input 
part.
+   Can be NULL (and in case of an input- or output- only profile, the 
other direction
+   will be NULL). */
+char *input_name;
+char *output_name;


The comment doesn't explain what the names are used for, other than
figuring out whether the profile has inputs and outputs, which is
already available in the n_sinks and n_sources fields. What's the
relation to the name field?


Ok, this could perhaps be explained better; if name is 
"output:digital-stereo+input:analog-stereo" then input_name is supposed 
to be "analog-stereo" and output_name is supposed to be 
"digital-stereo". But the combination of profiles like that is ALSA 
specific and this could potentially be used for other backends too, so I 
tried to write it in a more generic way.


--
David Henningsson, Canonical Ltd.
https://launchpad.net/~diwic
___
pulseaudio-discuss mailing list
pulseaudio-discuss@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/pulseaudio-discuss

Re: [pulseaudio-discuss] [PATCH 2/8] card: Add variables for splitting up a profile

On Tue, 2015-05-05 at 17:01 +0200, David Henningsson wrote:
> It can be useful for routing modules to know if a profile consists
> of an output and input part, in order to e g change output profile
> while keeping the input profile unchanged.

n_sinks and n_sources already tell if a profile consists of an output
and input part.

> For now filling in these fields is optional and a routing module
> must be able to handle NULL in these fields.
> 
> Signed-off-by: David Henningsson 
> ---
>  src/pulsecore/card.c | 2 ++
>  src/pulsecore/card.h | 6 ++
>  2 files changed, 8 insertions(+)
> 
> diff --git a/src/pulsecore/card.c b/src/pulsecore/card.c
> index f9a5ddc..dd33847 100644
> --- a/src/pulsecore/card.c
> +++ b/src/pulsecore/card.c
> @@ -52,6 +52,8 @@ pa_card_profile *pa_card_profile_new(const char *name, 
> const char *description,
>  void pa_card_profile_free(pa_card_profile *c) {
>  pa_assert(c);
>  
> +pa_xfree(c->input_name);
> +pa_xfree(c->output_name);
>  pa_xfree(c->name);
>  pa_xfree(c->description);
>  pa_xfree(c);
> diff --git a/src/pulsecore/card.h b/src/pulsecore/card.h
> index 3e2c004..8f85000 100644
> --- a/src/pulsecore/card.h
> +++ b/src/pulsecore/card.h
> @@ -40,6 +40,12 @@ typedef struct pa_card_profile {
>  char *name;
>  char *description;
>  
> +/* Indication in case the profile is built from an output and an input 
> part.
> +   Can be NULL (and in case of an input- or output- only profile, the 
> other direction
> +   will be NULL). */
> +char *input_name;
> +char *output_name;

The comment doesn't explain what the names are used for, other than
figuring out whether the profile has inputs and outputs, which is
already available in the n_sinks and n_sources fields. What's the
relation to the name field?

-- 
Tanu
___
pulseaudio-discuss mailing list
pulseaudio-discuss@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/pulseaudio-discuss

Re: [pulseaudio-discuss] Device reservation improvement suggestions

2015-10-19 Thread Felipe Sateler

On 19 October 2015 at 05:41, Tanu Kaskinen  wrote:
>> (Side note: I would have liked to see the device reservation protocol on
>> the system bus instead of the session one, but that's likely too late to
>> change now)
>
> I think it's probably possible to do the transition to the system bus,
> if we just can find someone to write patches for both PulseAudio and
> Jack. During a transition phase we would have to use both buses, to
> cooperate with applications using only the session bus.

Note that now that everyone is moving to a user instead of session
bus, the benefits of moving to the system bus are reduced. Also, who
would ship the dbus/polkit policy to allow logged in users to own the
name?

-- 

Saludos,
Felipe Sateler
___
pulseaudio-discuss mailing list
pulseaudio-discuss@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/pulseaudio-discuss

Re: [pulseaudio-discuss] Flat volumes and programmatic volume access (again)

On Mon, 2015-10-19 at 13:30 +0300, Tanu Kaskinen wrote:
> On Mon, 2015-10-19 at 15:19 +0530, Arun Raghavan wrote:
> > * I'm inclined to keep the implementation of the relative volume
> > calculation server-side,
> 
> If we want a policy module to be able to enforce the behaviour at the
> server side, I don't see any other way.
> 
> > in order to keep the client library simple. To
> > do this, pa_stream_volume_get/set() could reuse the same protocol as
> > the pa_context_* API but set a flag to let the server know the client
> > wants relative volumes.
> 
> How is this compatible with the mixer-app-in-browser use case? How can
> the mixer app get the absolute volume, if the browser has set the
> relative volume flag for a video app? I think the introspection API
> should stay unaffected by the relative volume flag.

I probably misunderstood. When you mentioned a flag, I assumed you
meant the stream flag, but maybe you meant using another flag in the
set volume message. That should work. One consideration is that is it
easier to do access control based on the message than based on the
message parameters - we might want to disallow the introspection API
for an application, but still allow the application to set its own
volume. If there's only one "set volume" message used to set both maybe
-absolute and always-relative volumes, an access control policy has to
inspect the message parameters. I would somewhat prefer having separate
messages rather than differentiating with a flag, but that's not a
strong opinion.

Note that there's no "get volume" message anywhere in the protocol.
It's not currently needed, since the "get stream info" message covers
that. If the pa_stream_get_volume() implementation uses a cached
volume, the "get volume" message is not needed in the future either,
because the server will be sending volume update notifications all the
time, so the client never needs to explicitly request the volume.

-- 
Tanu
___
pulseaudio-discuss mailing list
pulseaudio-discuss@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/pulseaudio-discuss

Re: [pulseaudio-discuss] Flat volumes and programmatic volume access (again)

On Mon, 2015-10-19 at 15:19 +0530, Arun Raghavan wrote:
> Hi folks,
> Thought I'd restart this thread since it's been a while. Let me
> summarise the discussion so far.
> 
> The decision to use flat-volumes has certainly been controversial.
> However, I'm convinced it provides a better user experience than the
> standard model of having separate per-app and device volumes (even
> though this is more familiar).
>
> The problem

I think you meant "a problem" :) Badly behaving applications are not
the only issue with flat volumes.

> we face is that some applications set the volume to 100%
> arbitrarily, and we'd like to avoid having that affect the system
> volume. Most of the cases where this complaint comes from seems related
> to applications that allow programmatic access to volume controls
> (browsers, gnash, etc.).
> 
> We discussed a number of solutions in the past, but the one I'd like to
> take forward is what I'd proposed originally -- a per-stream flag to
> allow clients to opt out of participating in flat-volume logic. In this
> way, programs that know they can't make guarantees of being well
> -behaved. We also need a mechanism to force this on clients that we
> know aren't well-behaved that we can't change (proprietary and legacy
> applications).
> 
> The idea is that only controlling the stream within the application's
> volume slider would have this non-flat behaviour. Mixer applications
> such as pavucontrol would not distinguish this stream from other
> streams, and changing the volume from there would behave just as any
> other stream. This should minimise confusion from users' perspective,
> while disabling the mechanism for rogue applications unexpectedly bump
> the volume.
> 
> That's how I'd like to see the behaviour work. Now let's talk about
> implementation. The previous RFC patch I'd sent communicates the stream
> flag for disabling flat volumes to the server, which always disables
> flat volumes for that stream. This doesn't work with the behaviour I
> described above. So what I think we should do is:
> 
> * Streams set a PA_STREAM_DISABLE_FLAT_VOLUME (or
> PA_STREAM_PROG_VOLUME_CONTROL or whatever) on the streams for which
> they want the new behaviour.
> 
> * We add a new stream volume API -- pa_stream_get_volume(),
> pa_stream_set_volume(), pa_stream_set_volume_callback(). I think this
> is good to have in general, to have a simpler client API. In the
> context of this proposal, this allows us to know when an application is
> concerned about the stream volume in the stream context vs. a global
> context. (this follow's from Tanu's proposal to deal with applications
> that play streams as well as implement their own system mixer -- such
> as a browser-based system UI might).
> 
> * It is not clear to me at the moment whether the new volume API should
> be synchronous. pa_stream_volume_get() should be. I think, but I'm
> undecided on pa_stream_set_volume().

pa_stream_set_volume() should be asynchronous like everything else that
requires a round-trip. pa_stream_get_volume() can be synchronous,
because we can cache the stream volume in the client, but we need to
think about what should happen when the client doesn't yet know its
volume. If the server sends the initial volume in the stream creation
reply, this problem doesn't exist, but what should happen with old
servers that don't support that? Could the new API be entirely
unsupported with older servers? That would mean no volume support in
clients that don't fall back to the introspection API... Then again,
maybe it's reasonable to recommend all applications to fall back to the
introspection API for some time, if they don't have guarantees about
the server version.

> * I'm inclined to keep the implementation of the relative volume
> calculation server-side,

If we want a policy module to be able to enforce the behaviour at the
server side, I don't see any other way.

> in order to keep the client library simple. To
> do this, pa_stream_volume_get/set() could reuse the same protocol as
> the pa_context_* API but set a flag to let the server know the client
> wants relative volumes.

How is this compatible with the mixer-app-in-browser use case? How can
the mixer app get the absolute volume, if the browser has set the
relative volume flag for a video app? I think the introspection API
should stay unaffected by the relative volume flag.

> * I'd also like to add a policy module to allow blacklisting specific
> applications, and forcing this behaviour on them. This will need a
> protocol update to set a stream flag after the stream is connected.

I'm not against adding the protocol change just for completeness, but I
also think that the protocol update isn't really necessary. I suppose
you mean that clients should get notified if the volume flag changes,
but do clients really need to know? What would they do with the
knowledge?

An implementation note: if the flag is altered during the stream
creation, the final flag value should

Re: [pulseaudio-discuss] Flat volumes and programmatic volume access (again)

2015-10-19 Thread Alexander E. Patrakov


19.10.2015 14:49, Arun Raghavan wrote:

Hi folks,
Thought I'd restart this thread since it's been a while. Let me
summarise the discussion so far.

The idea is that only controlling the stream within the application's
volume slider would have this non-flat behaviour. Mixer applications
such as pavucontrol would not distinguish this stream from other
streams, and changing the volume from there would behave just as any
other stream. This should minimise confusion from users' perspective,
while disabling the mechanism for rogue applications unexpectedly bump
the volume.


This looks like a good proposal overall.



That's how I'd like to see the behaviour work. Now let's talk about
implementation. The previous RFC patch I'd sent communicates the stream
flag for disabling flat volumes to the server, which always disables
flat volumes for that stream. This doesn't work with the behaviour I
described above. So what I think we should do is:

* Streams set a PA_STREAM_DISABLE_FLAT_VOLUME (or
PA_STREAM_PROG_VOLUME_CONTROL or whatever) on the streams for which
they want the new behaviour.


OK.


* We add a new stream volume API -- pa_stream_get_volume(),
pa_stream_set_volume(), pa_stream_set_volume_callback(). I think this
is good to have in general, to have a simpler client API. In the
context of this proposal, this allows us to know when an application is
concerned about the stream volume in the stream context vs. a global
context. (this follow's from Tanu's proposal to deal with applications
that play streams as well as implement their own system mixer -- such
as a browser-based system UI might).

* It is not clear to me at the moment whether the new volume API should
be synchronous. pa_stream_volume_get() should be. I think, but I'm
undecided on pa_stream_set_volume().


This indeed looks simpler than the existing introspection API.


* I'm inclined to keep the implementation of the relative volume
calculation server-side, in order to keep the client library simple. To
do this, pa_stream_volume_get/set() could reuse the same protocol as
the pa_context_* API but set a flag to let the server know the client
wants relative volumes.


I have no opinion here.


* I'd also like to add a policy module to allow blacklisting specific
applications, and forcing this behaviour on them. This will need a
protocol update to set a stream flag after the stream is connected.

* For legacy apps that are not covered by the blacklist we could add an
environment variable that makes all stream and sink-input volume
-related bits to use the new behaviour.


The question here is whether we would later want to force this upon all 
sandboxed applications (xdg-app).


--
Alexander E. Patrakov
___
pulseaudio-discuss mailing list
pulseaudio-discuss@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/pulseaudio-discuss

[pulseaudio-discuss] Flat volumes and programmatic volume access (again)

Hi folks,
Thought I'd restart this thread since it's been a while. Let me
summarise the discussion so far.

The decision to use flat-volumes has certainly been controversial.
However, I'm convinced it provides a better user experience than the
standard model of having separate per-app and device volumes (even
though this is more familiar).

The problem we face is that some applications set the volume to 100%
arbitrarily, and we'd like to avoid having that affect the system
volume. Most of the cases where this complaint comes from seems related
to applications that allow programmatic access to volume controls
(browsers, gnash, etc.).

We discussed a number of solutions in the past, but the one I'd like to
take forward is what I'd proposed originally -- a per-stream flag to
allow clients to opt out of participating in flat-volume logic. In this
way, programs that know they can't make guarantees of being well
-behaved. We also need a mechanism to force this on clients that we
know aren't well-behaved that we can't change (proprietary and legacy
applications).

The idea is that only controlling the stream within the application's
volume slider would have this non-flat behaviour. Mixer applications
such as pavucontrol would not distinguish this stream from other
streams, and changing the volume from there would behave just as any
other stream. This should minimise confusion from users' perspective,
while disabling the mechanism for rogue applications unexpectedly bump
the volume.

That's how I'd like to see the behaviour work. Now let's talk about
implementation. The previous RFC patch I'd sent communicates the stream
flag for disabling flat volumes to the server, which always disables
flat volumes for that stream. This doesn't work with the behaviour I
described above. So what I think we should do is:

* Streams set a PA_STREAM_DISABLE_FLAT_VOLUME (or
PA_STREAM_PROG_VOLUME_CONTROL or whatever) on the streams for which
they want the new behaviour.

* We add a new stream volume API -- pa_stream_get_volume(),
pa_stream_set_volume(), pa_stream_set_volume_callback(). I think this
is good to have in general, to have a simpler client API. In the
context of this proposal, this allows us to know when an application is
concerned about the stream volume in the stream context vs. a global
context. (this follow's from Tanu's proposal to deal with applications
that play streams as well as implement their own system mixer -- such
as a browser-based system UI might).

* It is not clear to me at the moment whether the new volume API should
be synchronous. pa_stream_volume_get() should be. I think, but I'm
undecided on pa_stream_set_volume().

* I'm inclined to keep the implementation of the relative volume
calculation server-side, in order to keep the client library simple. To
do this, pa_stream_volume_get/set() could reuse the same protocol as
the pa_context_* API but set a flag to let the server know the client
wants relative volumes.

* I'd also like to add a policy module to allow blacklisting specific
applications, and forcing this behaviour on them. This will need a
protocol update to set a stream flag after the stream is connected.

* For legacy apps that are not covered by the blacklist we could add an
environment variable that makes all stream and sink-input volume
-related bits to use the new behaviour.

I've tried to distill out the relevant parts of the previous discussion
to take this forwards. Feel free to bring up anything I might've
missed.

Cheers,
Arun
___
pulseaudio-discuss mailing list
pulseaudio-discuss@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/pulseaudio-discuss

Re: [pulseaudio-discuss] [PATCH] alsa-mixer: Ignore volume with unexpected number of channels

2015-10-19 Thread Alexander E. Patrakov


19.10.2015 14:16, David Henningsson wrote:

We currently only support one and two channels for volumes, and
bail out otherwise. This makes Xonar users unhappy because they
have a volume with eight channels, and bailing out means they
don't have a path/port at all.

This way they will at least have a port, which will in turn make
the gnome/unity UI behave better.

BugLink: https://bugs.freedesktop.org/show_bug.cgi?id=84983
BugLink: https://bugzilla.gnome.org/show_bug.cgi?id=745017
Signed-off-by: David Henningsson 
---

This is a resend of https://bugs.freedesktop.org/attachment.cgi?id=114250
There is a competing patch by Raymond in that bug, but I think his
patch is broken, so I'm going to push mine in a week if there are no
reviews.


Well, here is my review :)

While this should work, this patch means that all those users are 
essentially limited to software-based volume inside of PulseAudio. Also, 
the master (or whatever multichannel) slider will not be touched by 
PulseAudio at all. Which means that it will stay at whatever position 
that alsactl init (or alsactl restore) left it at. I.e. a limited volume 
range (note: this is not a regression). Which is still better than what 
we have now.


Counterproposal: if that's easier to implement than full multi-channel 
volume, treat all multichannel volume controls as mono internally, 
duplicate the volume across all channels when setting it, and use the 
first channel when getting. I.e., instead of full software volume, make 
"software balance".


As this counterproposal comes without a patch, and especially since the 
"counterprpopsed" improvement can be done later, this should not block 
the "going to push in one week" statement.


--
Alexander E. Patrakov



  src/modules/alsa/alsa-mixer.c | 6 +++---
  1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/src/modules/alsa/alsa-mixer.c b/src/modules/alsa/alsa-mixer.c
index 9e06ba4..486da83 100644
--- a/src/modules/alsa/alsa-mixer.c
+++ b/src/modules/alsa/alsa-mixer.c
@@ -1743,10 +1743,10 @@ static int element_probe(pa_alsa_element *e, 
snd_mixer_t *m) {
  
  if (e->n_channels <= 0) {

  pa_log_warn("Volume element %s with no channels?", 
e->alsa_name);
-return -1;
+e->volume_use = PA_ALSA_VOLUME_IGNORE;
  }
  
-if (e->n_channels > 2) {

+else if (e->n_channels > 2) {
  /* FIXME: In some places code like this is used:
   *
   * e->masks[alsa_channel_ids[p]][e->n_channels-1]
@@ -1759,7 +1759,7 @@ static int element_probe(pa_alsa_element *e, snd_mixer_t 
*m) {
   * don't support elements with more than two
   * channels... */
  pa_log_warn("Volume element %s has %u channels. That's too much! 
I can't handle that!", e->alsa_name, e->n_channels);
-return -1;
+e->volume_use = PA_ALSA_VOLUME_IGNORE;
  }
  
  if (!e->override_map) {


___
pulseaudio-discuss mailing list
pulseaudio-discuss@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/pulseaudio-discuss

[pulseaudio-discuss] [PATCH] alsa-mixer: Ignore volume with unexpected number of channels

We currently only support one and two channels for volumes, and
bail out otherwise. This makes Xonar users unhappy because they
have a volume with eight channels, and bailing out means they
don't have a path/port at all.

This way they will at least have a port, which will in turn make
the gnome/unity UI behave better.

BugLink: https://bugs.freedesktop.org/show_bug.cgi?id=84983
BugLink: https://bugzilla.gnome.org/show_bug.cgi?id=745017
Signed-off-by: David Henningsson 
---

This is a resend of https://bugs.freedesktop.org/attachment.cgi?id=114250
There is a competing patch by Raymond in that bug, but I think his
patch is broken, so I'm going to push mine in a week if there are no
reviews.

 src/modules/alsa/alsa-mixer.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/src/modules/alsa/alsa-mixer.c b/src/modules/alsa/alsa-mixer.c
index 9e06ba4..486da83 100644
--- a/src/modules/alsa/alsa-mixer.c
+++ b/src/modules/alsa/alsa-mixer.c
@@ -1743,10 +1743,10 @@ static int element_probe(pa_alsa_element *e, 
snd_mixer_t *m) {
 
 if (e->n_channels <= 0) {
 pa_log_warn("Volume element %s with no channels?", 
e->alsa_name);
-return -1;
+e->volume_use = PA_ALSA_VOLUME_IGNORE;
 }
 
-if (e->n_channels > 2) {
+else if (e->n_channels > 2) {
 /* FIXME: In some places code like this is used:
  *
  * e->masks[alsa_channel_ids[p]][e->n_channels-1]
@@ -1759,7 +1759,7 @@ static int element_probe(pa_alsa_element *e, snd_mixer_t 
*m) {
  * don't support elements with more than two
  * channels... */
 pa_log_warn("Volume element %s has %u channels. That's 
too much! I can't handle that!", e->alsa_name, e->n_channels);
-return -1;
+e->volume_use = PA_ALSA_VOLUME_IGNORE;
 }
 
 if (!e->override_map) {
-- 
1.9.1

___
pulseaudio-discuss mailing list
pulseaudio-discuss@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/pulseaudio-discuss

Re: [pulseaudio-discuss] [PATCH] make availability of ports visible over dbus


Pushed now. Thanks for the contribution!

On 2015-04-19 16:29, John Horan wrote:

---
  src/modules/dbus/iface-device-port.c | 65 ++--
  1 file changed, 63 insertions(+), 2 deletions(-)

diff --git a/src/modules/dbus/iface-device-port.c 
b/src/modules/dbus/iface-device-port.c
index 1b26756..4892443 100644
--- a/src/modules/dbus/iface-device-port.c
+++ b/src/modules/dbus/iface-device-port.c
@@ -34,6 +34,7 @@ static void handle_get_index(DBusConnection *conn, 
DBusMessage *msg, void *userd
  static void handle_get_name(DBusConnection *conn, DBusMessage *msg, void 
*userdata);
  static void handle_get_description(DBusConnection *conn, DBusMessage *msg, 
void *userdata);
  static void handle_get_priority(DBusConnection *conn, DBusMessage *msg, void 
*userdata);
+static void handle_get_available(DBusConnection *conn, DBusMessage *msg, void 
*userdata);

  static void handle_get_all(DBusConnection *conn, DBusMessage *msg, void 
*userdata);

@@ -41,6 +42,9 @@ struct pa_dbusiface_device_port {
  uint32_t index;
  pa_device_port *port;
  char *path;
+
+pa_hook_slot *available_changed_slot;
+
  pa_dbus_protocol *dbus_protocol;
  };

@@ -49,6 +53,7 @@ enum property_handler_index {
  PROPERTY_HANDLER_NAME,
  PROPERTY_HANDLER_DESCRIPTION,
  PROPERTY_HANDLER_PRIORITY,
+PROPERTY_HANDLER_AVAILABLE,
  PROPERTY_HANDLER_MAX
  };

@@ -57,6 +62,18 @@ static pa_dbus_property_handler 
property_handlers[PROPERTY_HANDLER_MAX] = {
  [PROPERTY_HANDLER_NAME]= { .property_name = "Name",.type = 
"s", .get_cb = handle_get_name,.set_cb = NULL },
  [PROPERTY_HANDLER_DESCRIPTION] = { .property_name = "Description", .type = 
"s", .get_cb = handle_get_description, .set_cb = NULL },
  [PROPERTY_HANDLER_PRIORITY]= { .property_name = "Priority",.type = 
"u", .get_cb = handle_get_priority,.set_cb = NULL },
+[PROPERTY_HANDLER_AVAILABLE]   = { .property_name = "Available",   .type = 
"u", .get_cb = handle_get_available,   .set_cb = NULL }
+};
+
+enum signal_index {
+SIGNAL_AVAILABLE_CHANGED,
+SIGNAL_MAX
+};
+
+static pa_dbus_arg_info available_changed_args[] = { { "available", "u", NULL 
} };
+
+static pa_dbus_signal_info signals[SIGNAL_MAX] = {
+[SIGNAL_AVAILABLE_CHANGED] = { .name = "AvailableChanged", .arguments = 
available_changed_args, .n_arguments = 1 }
  };

  static pa_dbus_interface_info port_interface_info = {
@@ -66,8 +83,8 @@ static pa_dbus_interface_info port_interface_info = {
  .property_handlers = property_handlers,
  .n_property_handlers = PROPERTY_HANDLER_MAX,
  .get_all_properties_cb = handle_get_all,
-.signals = NULL,
-.n_signals = 0
+.signals = signals,
+.n_signals = SIGNAL_MAX
  };

  static void handle_get_index(DBusConnection *conn, DBusMessage *msg, void 
*userdata) {
@@ -113,6 +130,20 @@ static void handle_get_priority(DBusConnection *conn, 
DBusMessage *msg, void *us
  pa_dbus_send_basic_variant_reply(conn, msg, DBUS_TYPE_UINT32, &priority);
  }

+static void handle_get_available(DBusConnection *conn, DBusMessage *msg, void 
*userdata) {
+pa_dbusiface_device_port *p = userdata;
+dbus_uint32_t available = 0;
+
+pa_assert(conn);
+pa_assert(msg);
+pa_assert(p);
+
+available = p->port->available;
+
+pa_dbus_send_basic_variant_reply(conn, msg, DBUS_TYPE_UINT32, &available);
+}
+
+
  static void handle_get_all(DBusConnection *conn, DBusMessage *msg, void 
*userdata) {
  pa_dbusiface_device_port *p = userdata;
  DBusMessage *reply = NULL;
@@ -135,6 +166,7 @@ static void handle_get_all(DBusConnection *conn, 
DBusMessage *msg, void *userdat
  pa_dbus_append_basic_variant_dict_entry(&dict_iter, 
property_handlers[PROPERTY_HANDLER_NAME].property_name, DBUS_TYPE_STRING, 
&p->port->name);
  pa_dbus_append_basic_variant_dict_entry(&dict_iter, 
property_handlers[PROPERTY_HANDLER_DESCRIPTION].property_name, DBUS_TYPE_STRING, 
&p->port->description);
  pa_dbus_append_basic_variant_dict_entry(&dict_iter, 
property_handlers[PROPERTY_HANDLER_PRIORITY].property_name, DBUS_TYPE_UINT32, 
&priority);
+pa_dbus_append_basic_variant_dict_entry(&dict_iter, 
property_handlers[PROPERTY_HANDLER_AVAILABLE].property_name, DBUS_TYPE_UINT32, 
&p->port->available);

  pa_assert_se(dbus_message_iter_close_container(&msg_iter, &dict_iter));

@@ -142,6 +174,32 @@ static void handle_get_all(DBusConnection *conn, 
DBusMessage *msg, void *userdat
  dbus_message_unref(reply);
  }

+static pa_hook_result_t available_changed_cb(void *hook_data, void *call_data, 
void *slot_data) {
+pa_dbusiface_device_port *p = slot_data;
+pa_device_port *port = call_data;
+DBusMessage *signal_msg;
+uint32_t available;
+
+pa_assert(p);
+pa_assert(port);
+
+if(p->port != port)
+return PA_HOOK_OK;
+
+available = port->available;
+
+pa_assert_se(signal_msg = dbus_message_new_signal(p->path,
+

Re: [pulseaudio-discuss] [PATCH 19/19] dbus: Add card profile availability info to API




On 2015-05-21 12:36, Tanu Kaskinen wrote:

On Thu, May 21, 2015, at 11:46, David Henningsson wrote:

This patch looks broken: Profile availability is not a boolean, it's a
tristate (yes/no/unkown).


In the libpulse API it's a boolean, however. I think it should be
boolean in the D-Bus API too.


(Answering to a very old email while going through old stuff)

It's kind of weird/broken that libpulse does not expose the tristate but 
now that it doesn't, I guess it's okay to have just a boolean here too.


The calculation of that boolean was still wrong, though. Fixed it up and 
pushed.





On 2015-03-19 12:51, Juho Hämäläinen wrote:

---

Noticed that profile availability was missing from D-Bus API so added while
at it. Didn't check whether there was other stuff missing elsewhere as well.


There most likely is other stuff missing as well, because new features
are generally only added to the libpulse API. The D-Bus API is still in
work-in-progress state, although no progress has been made for a long
time. The API still has no stability guarantees, so it's not recommended
for wide use.



--
David Henningsson, Canonical Ltd.
https://launchpad.net/~diwic
___
pulseaudio-discuss mailing list
pulseaudio-discuss@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/pulseaudio-discuss

Re: [pulseaudio-discuss] Device reservation improvement suggestions

On Mon, 2015-10-19 at 09:37 +0200, David Henningsson wrote:
> 
> On 2015-10-04 17:05, Zeno Endemann wrote:
> > 2.
> > The spec implies that the reservation is for a whole device (ie. alsa
> > card), so I believe a well-behaving client should not touch a reserved
> > alsa card in any way. But PA only uses the reservation for PCM streams,
> > not for the mixer controls of the card. My suggestion is either fixing
> > PA (my preference) so that it reserves the card before writing any
> > changes to the mixer, or at least making it clear in the spec that the
> > reservation is only for all PCM devices on the corresponding card.
> 
> I agree that PA should reserve the card before opening the mixer. 
> Patches welcome :-) Although you might want to synchronize with Tanu who 
> recently talked about closing the mixer when we lost access to it, that 
> sounds like something that touches similar parts of the code at least.

If Zeno sends patches that conflict with my work, I can do the required
rebasing.

> (Side note: I would have liked to see the device reservation protocol on 
> the system bus instead of the session one, but that's likely too late to 
> change now)

I think it's probably possible to do the transition to the system bus,
if we just can find someone to write patches for both PulseAudio and
Jack. During a transition phase we would have to use both buses, to
cooperate with applications using only the session bus.

-- 
Tanu
___
pulseaudio-discuss mailing list
pulseaudio-discuss@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/pulseaudio-discuss

Re: [pulseaudio-discuss] [PATCH 2/2] thread-mainloop: unblock SIGSYS on sandbox

2015-10-19 Thread Julien Isorce

Thx for the review. Indeed I double checked and prctl should be blocked. It
seems in chromium gpu process and plugin process, they enable it that why
we did too. Though there is a todo saying to block it :)
In our case we can directly block it.

For my patch it means the condition "prctl(PR_GET_SECCOMP" should go away
and the todo too.

I am also wondering why pulseaudio block signals
"pthread_sigmask(SIG_BLOCK, &mask, NULL);"

AFAIK glib does not do that. Also is pa_threaded_mainloop_start called in
pa sever too ? Maybe it should be applied to the server only. Worth to
mention that pthread_sigmask(SIG_BLOCK, &mask, NULL); has last been touched
in 2006. I am not saying this a reason at all but just in case :)

Also note that glib only does it for explicit unix source:
"g_unix_signal_source_new". Though they do it differently, see
"g_get_worker_context", they block everything and then restore previous
mask. Note that "pthread_sigmask (SIG_SETMASK" is similar to
"pthread_sigmask(SIG_BLOCK", see man for the details.
Maybe pulseaudio should restore the previous mask too ?

In chromium the block everything by default and unblock SIGSYG whenever a
handler is attached to a system call, see:
chromium/src/sandbox/linux/seccomp-bpf/trap.cc::Trap::Trap

Anyway I think for the pulse audio client, the mask should be inherited
from application.
So that the patch would become:

sigset_t mask;
sigset_t prev_mask;

/* Make sure that signals are delivered to the main thread */
sigfillset(&mask);
pthread_sigmask(SIG_BLOCK, &mask, &prev_mask);

if (!sigismember(&prev_mask, SIGSYS)) {  // <- check if parent has
unblocked the SIGSYS signal .
int r = sigemptyset(&mask) || sigaddset(&mask, SIGSYS) ||
pthread_sigmask(SIG_UNBLOCK, &mask, NULL);
pa_assert(!r);
}

Problem is that if I replace SIGSYS by any other signal, it still goes into
the block. So it means no signal are blocked in prev_mask which is wrong.
Any idea ?

Also I'll add what you suggested header presence or not, and configure
check.

Thx
Julien

On 19 October 2015 at 04:35, Arun Raghavan  wrote:

> On Sat, 2015-10-10 at 20:11 +0100, Julien Isorce wrote:
> > Seccomp-BPF actually uses SIGSYS to trigger
> > the trap handler attached to sys_open.
> > If the signal is blocked then the kernel kills
> > the process whenever pulse audio calls 'open'.
> > The result backtrace is terminating in sys_open.
> >
> > This is required to have pulse audio working
> > in a sandbox.
> > ---
> >  src/pulse/thread-mainloop.c | 10 ++
> >  1 file changed, 10 insertions(+)
> >
> > diff --git a/src/pulse/thread-mainloop.c b/src/pulse/thread
> > -mainloop.c
> > index afd0581..93582d2 100644
> > --- a/src/pulse/thread-mainloop.c
> > +++ b/src/pulse/thread-mainloop.c
> > @@ -28,6 +28,8 @@
> >
> >  #include 
> >  #include 
> > +#include 
>
> This needs to be in a #ifdef HAVE_SYS_PRCTL_H (which is already
> defined).
>
> > +#include 
>
> You need to add a configure-time header check for this one and then
> make the include conditional, as we need to make sure we build on
> machines without seccomp (which includes non-Linux systems too).
>
> >
> >  #include 
> >  #include 
> > @@ -81,6 +83,14 @@ static void thread(void *userdata) {
> >  /* Make sure that signals are delivered to the main thread */
> >  sigfillset(&mask);
> >  pthread_sigmask(SIG_BLOCK, &mask, NULL);
> > +
> > +/* If seccomp is in use, only filter mode has a chance to work.
> > + * Because pa requires sys_open. */
> > +if (prctl(PR_GET_SECCOMP, SECCOMP_MODE_FILTER, NULL) == 2) {
>
> Is the second argument of PR_GET_SETCOMP ever used? The man page
> suggests that it takes no arguments.
>
> Also, I see that if prctl() is not allowed, the process will be killed.
> Is there any condition where prctl() might not be allowed by seccomp,
> but we might still be able to function correctly?
>
> > +/* TODO: unblock SIGSYS only if a trap is attached to
> > sys_open. */
>
> Could you clarify what needs to be done for this TODO to go away?
>
> > +int r = sigemptyset(&mask) || sigaddset(&mask, SIGSYS) ||
> > pthread_sigmask(SIG_UNBLOCK, &mask, NULL);
> > +pa_assert(!r);
> > +}
>
> This entire condition would then be within a conditional for the
> presence of prctl.h and seccomp.h.
>
> >  #endif
> >
> >  pa_mutex_lock(m->mutex);
>
> -- Arun
> ___
> pulseaudio-discuss mailing list
> pulseaudio-discuss@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/pulseaudio-discuss
>
___
pulseaudio-discuss mailing list
pulseaudio-discuss@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/pulseaudio-discuss

Re: [pulseaudio-discuss] [PATCH 1/2] pa_make_secure_dir: avoid calling fchmod if already right mode

2015-10-19 Thread Julien Isorce

Thx for review and changes, looks better indeed. I also confirm it still
works.
Cheers, Julien

On 19 October 2015 at 04:23, Arun Raghavan  wrote:

> On Sat, 2015-10-10 at 20:11 +0100, Julien Isorce wrote:
> > fchmod is denied in chromium sandbox.
> > ---
> >  src/pulsecore/core-util.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/src/pulsecore/core-util.c b/src/pulsecore/core-util.c
> > index 278ad63..f694347 100644
> > --- a/src/pulsecore/core-util.c
> > +++ b/src/pulsecore/core-util.c
> > @@ -343,7 +343,7 @@ again:
> >  #endif
> >
> >  #ifdef HAVE_FCHMOD
> > -if (fchmod(fd, m) < 0) {
> > +if ((st.st_mode & 0x0FFF) != m && fchmod(fd, m) < 0) {
> >  pa_assert_se(pa_close(fd) >= 0);
> >  goto fail;
> >  };
>
> I'm pushing this out with some minor changes. The commit log is made to
> be more consistent with previous commits.
>
> I also switched 0x to 0 since that's the more common convention
> with mode-related variables (this really is a nitpick, though!)
>
> -- Arun
> ___
> pulseaudio-discuss mailing list
> pulseaudio-discuss@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/pulseaudio-discuss
>
___
pulseaudio-discuss mailing list
pulseaudio-discuss@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/pulseaudio-discuss

Re: [pulseaudio-discuss] Device reservation improvement suggestions