Re: [pulseaudio-discuss] Handling of port latency offsets
On 16.02.2017 12:59, Tanu Kaskinen wrote:
On Fri, 2017-02-10 at 14:38 +0100, Georg Chini wrote:
Hi,
during my work on module-loopback I came upon an issue regarding the
handling
of the port latency offsets.
The functions pa_{source,sink}_get_latency_within_thread() return the
latencies
including the offset. Those functions are used to determine the amount
that has
to be rewound during a sink move and also to calculate the time a volume
change
is delayed.
I think it is wrong in those situations to include the offset,
especially if a user sets
very large or very negative offsets. If you have a large negative offset
for example,
the sink will never be rewound on a move because get_latency_in_thread()
always
returns 0. If the offset is large and positive, you might end up
rewinding too much,
although this is limited by max_rewind.
A similar thought applies to the delay of the volume changes, although
it is surely
less critical there.
Am I right or do I miss something?
You're right. The total latency is not the same thing as the rewindable
amount. I think there's a FIXME comment about this somewhere in the
stream moving code.
Could this not be fixed quite easily by subtracting the offset again
in these places? Should I send a patch?
Would it be a good idea to let the functions return a structure
containing the
actual latency and the offset in separate variables and let the caller
sort out
if it needs the offset or not?
The term "actual latency" includes the offset, if the offset is
correct. The important distinction is between "total latency" and
"rewindable amount". And yes, it would be a good idea to report these
separately.
I think it is not really necessary, because there is the port_latency_offset
variable, which can be directly accessed in all places where you might
need the offset. (And if there will be some sink_latency_offset in the
future, the same applies)
___
pulseaudio-discuss mailing list
pulseaudio-discuss@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/pulseaudio-discuss
[pulseaudio-discuss] [PATCH v3 0/2] Improve default device policy
The first patch refactors the default sink/source handling code to make it easier to add new conditions to how the default sink and source are chosen. It's not pure refactoring, though: the patch also fixes some cases where change notifications were not sent, and the D-Bus protocol's default sink/source functionality should work much better now, and there are new log messages for default device changes. The second patch fixes improves the default sink/source selection by avoiding devices whose active port is unavailable. Changes in v3: - Fixed a crash in module-dbus-protocol: the module would crash if a new sink became immediately the default, because the module assumed that the sink put hook would always be fired before the default sink changed hook. - Changed the default source selection logic so that if we're comparing two monitor sources, the monitored sinks are compared before resorting to checking whether one of the sources is already the default. Changes in v2: - Added a note about the D-Bus protocol bug to the first patch's commit message. - Changed the default sink/source selection logic to prefer the current default device when comparing two devices with the same priority. - Removed the redundant call to pa_core_update_default_sink/source() when unlinking the current configured default sink or source. pa_core_set_configured_default_sink/source() will call that function anyway. - Made the comment on pa_core.default_sink/source a bit more verbose. - Dropped the patch that uses the active port's priority as the sink/source priority. The discussion is ongoing about what to do about that, so I'll submit that patch separately later, or a different patch that achieves the same goal. Tanu Kaskinen (2): refactor default sink/source handling core, device-port: check availability when choosing the default device src/modules/dbus/iface-core.c | 114 +++-- src/modules/dbus/iface-sample.c | 10 +- src/modules/module-default-device-restore.c | 14 +-- src/modules/module-intended-roles.c | 37 +++--- src/modules/module-rescue-streams.c | 20 ++- src/modules/module-switch-on-connect.c | 30 ++--- src/pulsecore/cli-command.c | 20 ++- src/pulsecore/cli-text.c| 12 +- src/pulsecore/core.c| 186 src/pulsecore/core.h| 28 - src/pulsecore/device-port.c | 8 ++ src/pulsecore/namereg.c | 115 + src/pulsecore/namereg.h | 6 - src/pulsecore/protocol-native.c | 19 ++- src/pulsecore/sink.c| 7 ++ src/pulsecore/source.c | 7 ++ 16 files changed, 394 insertions(+), 239 deletions(-) -- 2.11.0 ___ pulseaudio-discuss mailing list pulseaudio-discuss@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/pulseaudio-discuss
[pulseaudio-discuss] [PATCH v3 2/2] core, device-port: check availability when choosing the default device
It doesn't make sense to use a sink or source whose active port is
unavailable, so let's take this into account when choosing the default
sink and source.
---
No changes in v2 or v3.
src/pulsecore/core.c| 16
src/pulsecore/device-port.c | 8
2 files changed, 24 insertions(+)
diff --git a/src/pulsecore/core.c b/src/pulsecore/core.c
index 16fd040a4..52e51db1a 100644
--- a/src/pulsecore/core.c
+++ b/src/pulsecore/core.c
@@ -266,6 +266,14 @@ static int compare_sinks(pa_sink *a, pa_sink *b) {
core = a->core;
+/* Available sinks always beat unavailable sinks. */
+if (a->active_port && a->active_port->available == PA_AVAILABLE_NO
+&& (!b->active_port || b->active_port->available !=
PA_AVAILABLE_NO))
+return -1;
+if (b->active_port && b->active_port->available == PA_AVAILABLE_NO
+&& (!a->active_port || a->active_port->available !=
PA_AVAILABLE_NO))
+return 1;
+
/* The configured default sink is preferred over any other sink. */
if (b == core->configured_default_sink)
return -1;
@@ -332,6 +340,14 @@ static int compare_sources(pa_source *a, pa_source *b) {
core = a->core;
+/* Available sources always beat unavailable sources. */
+if (a->active_port && a->active_port->available == PA_AVAILABLE_NO
+&& (!b->active_port || b->active_port->available !=
PA_AVAILABLE_NO))
+return -1;
+if (b->active_port && b->active_port->available == PA_AVAILABLE_NO
+&& (!a->active_port || a->active_port->available !=
PA_AVAILABLE_NO))
+return 1;
+
/* The configured default source is preferred over any other source. */
if (b == core->configured_default_source)
return -1;
diff --git a/src/pulsecore/device-port.c b/src/pulsecore/device-port.c
index 7c9ddf325..76a7e80a1 100644
--- a/src/pulsecore/device-port.c
+++ b/src/pulsecore/device-port.c
@@ -93,6 +93,14 @@ void pa_device_port_set_available(pa_device_port *p,
pa_available_t status) {
* be created before port objects, and then p->card could be non-NULL for
* the whole lifecycle of pa_device_port. */
if (p->card) {
+/* A sink or source whose active port is unavailable can't be the
+ * default sink/source, so port availability changes may affect the
+ * default sink/source choice. */
+if (p->direction == PA_DIRECTION_OUTPUT)
+pa_core_update_default_sink(p->core);
+else
+pa_core_update_default_source(p->core);
+
pa_subscription_post(p->core,
PA_SUBSCRIPTION_EVENT_CARD|PA_SUBSCRIPTION_EVENT_CHANGE, p->card->index);
pa_hook_fire(>core->hooks[PA_CORE_HOOK_PORT_AVAILABLE_CHANGED], p);
}
--
2.11.0
___
pulseaudio-discuss mailing list
pulseaudio-discuss@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/pulseaudio-discuss
[pulseaudio-discuss] [PATCH v3 1/2] refactor default sink/source handling
Currently the default sink policy is simple: either the user has configured it explicitly, in which case we always use that as the default, or we pick the sink with the highest priority. The sink priorities are currently static, so there's no need to worry about updating the default sink when sink priorities change. I intend to make things a bit more complex: if the active port of a sink is unavailable, the sink should not be the default sink, and I also want to make sink priorities dependent on the active port, so changing the port should cause re-evaluation of which sink to choose as the default. Currently the default sink choice is done only when someone calls pa_namereg_get_default_sink(), and change notifications are only sent when a sink is created or destroyed. That makes it hard to add new rules to the default sink selection policy. This patch moves the default sink selection to pa_core_update_default_sink(), which is called whenever something happens that can affect the default sink choice. That function needs to know the previous choice in order to send change notifications as appropriate, but previously pa_core.default_sink was only set when the user had configured it explicitly. Now pa_core.default_sink is always set (unless there are no sinks at all), so pa_core_update_default_sink() can use that to get the previous choice. The user configuration is saved in a new variable, pa_core.configured_default_sink. pa_namereg_get_default_sink() is now unnecessary, because pa_core.default_sink can be used directly to get the currently-considered-best sink. pa_namereg_set_default_sink() is replaced by pa_core_set_configured_default_sink(). I haven't confirmed it, but I expect that this patch will fix problems in the D-Bus protocol related to default sink handling. The D-Bus protocol used to get confused when the current default sink gets removed. It would incorrectly think that if there's no explicitly configured default sink, then there's no default sink at all. Even worse, when the D-Bus thinks that there's no default sink, it concludes that there are no sinks at all, which made it impossible to configure the default sink via the D-Bus interface. Now that pa_core.default_sink is always set, except when there really aren't any sinks, the D-Bus protocol should behave correctly. BugLink: https://bugs.freedesktop.org/show_bug.cgi?id=99425 --- Changes in v3: - Fixed a crash in module-dbus-protocol: the module would crash if a new sink became immediately the default, because the module assumed that the sink put hook would always be fired before the default sink changed hook. - Changed the default source selection logic so that if we're comparing two monitor sources, the monitored sinks are compared before resorting to checking whether one of the sources is already the default. Changes in v2: - Added a note about the D-Bus protocol bug to the commit message. - Changed the default sink/source selection logic to prefer the current default device when comparing two devices with the same priority. - Removed the redundant call to pa_core_update_default_sink/source() when unlinking the current configured default sink or source. pa_core_set_configured_default_sink/source() will call that function anyway. - Made the comment on pa_core.default_sink/source a bit more verbose. - Dropped the patch that uses the active port's priority as the sink/source priority. The discussion is ongoing about what to do about that, so I'll submit that patch separately later, or a different patch that achieves the same goal. src/modules/dbus/iface-core.c | 114 +-- src/modules/dbus/iface-sample.c | 10 +- src/modules/module-default-device-restore.c | 14 +-- src/modules/module-intended-roles.c | 37 +++--- src/modules/module-rescue-streams.c | 20 ++-- src/modules/module-switch-on-connect.c | 30 ++--- src/pulsecore/cli-command.c | 20 ++-- src/pulsecore/cli-text.c| 12 +- src/pulsecore/core.c| 170 src/pulsecore/core.h| 28 - src/pulsecore/namereg.c | 115 +-- src/pulsecore/namereg.h | 6 - src/pulsecore/protocol-native.c | 19 ++-- src/pulsecore/sink.c| 7 ++ src/pulsecore/source.c | 7 ++ 15 files changed, 370 insertions(+), 239 deletions(-) diff --git a/src/modules/dbus/iface-core.c b/src/modules/dbus/iface-core.c index 508913de1..3f368ab46 100644 --- a/src/modules/dbus/iface-core.c +++ b/src/modules/dbus/iface-core.c @@ -721,7 +721,7 @@ static void handle_set_fallback_sink(DBusConnection *conn, DBusMessage *msg, DBu return; } -pa_namereg_set_default_sink(c->core, pa_dbusiface_device_get_sink(fallback_sink)); +
Re: [pulseaudio-discuss] Handling of port latency offsets
On 16.02.2017 13:10, Georg Chini wrote:
On 16.02.2017 12:59, Tanu Kaskinen wrote:
On Fri, 2017-02-10 at 14:38 +0100, Georg Chini wrote:
Hi,
during my work on module-loopback I came upon an issue regarding the
handling
of the port latency offsets.
The functions pa_{source,sink}_get_latency_within_thread() return the
latencies
including the offset. Those functions are used to determine the amount
that has
to be rewound during a sink move and also to calculate the time a
volume
change
is delayed.
I think it is wrong in those situations to include the offset,
especially if a user sets
very large or very negative offsets. If you have a large negative
offset
for example,
the sink will never be rewound on a move because
get_latency_in_thread()
always
returns 0. If the offset is large and positive, you might end up
rewinding too much,
although this is limited by max_rewind.
A similar thought applies to the delay of the volume changes, although
it is surely
less critical there.
Am I right or do I miss something?
You're right. The total latency is not the same thing as the rewindable
amount. I think there's a FIXME comment about this somewhere in the
stream moving code.
Could this not be fixed quite easily by subtracting the offset again
in these places? Should I send a patch?
I just noticed that this is only possible after the loopback patches
have been merged,
because they change the behavior of the
pa_{source,sink}_get_latency_in_thread()
functions ...
___
pulseaudio-discuss mailing list
pulseaudio-discuss@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/pulseaudio-discuss
Re: [pulseaudio-discuss] combined sink does not work with pulseaudio-dlna
On Wed, 2017-02-15 at 17:25 +0100, Ivan Kanis wrote: > The author of the dlna plugin has told me that the combined sink feature > is not implemented in his module. He has given me an alternative with > the loopback module: > > load-module module-loopback sink=alsa_output.pci-_00_1b.0.analog-stereo > > Then pickup the loopback on the Chromecast. It works but, as expected, > it is not in sync. So the source of the loopback is the monitor source of the chromecast sink? > I am now looking for a way to delay the local playback but I can't > figure out how. > > I tried latency_msec=3000 but it set's the latency on the Chromecast. If the source of the loopback is the monitor source of the chromecast sink, increasing the latency of the loopback module should increase the latency of what is played to the alsa sink, not the chromecast sink. It's a known problem that something is wrong in module-loopback related to the latency_msec paramter: https://bugs.freedesktop.org/show_bug.cgi?id=80770 The loopback patches that Georg Chini submitted recently probably fix the latency_msec parameter behaviour. -- Tanu https://www.patreon.com/tanuk ___ pulseaudio-discuss mailing list pulseaudio-discuss@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/pulseaudio-discuss
Re: [pulseaudio-discuss] [PATCH] bluez5-device: Use correct constants for fixed latency in PA_{SINK, SOURCE}_MESSAGE_GET_LATENCY
On Wed, 2017-02-15 at 11:32 +0100, Georg Chini wrote:
> The PA_{SINK,SOURCE}_GET_LATENCY message handlers falsely always added the
> A2DP latency as fixed
> latency instead of the profile specific constant.
>
> ---
> src/modules/bluetooth/module-bluez5-device.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
Thanks! Applied.
--
Tanu
https://www.patreon.com/tanuk
___
pulseaudio-discuss mailing list
pulseaudio-discuss@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/pulseaudio-discuss
Re: [pulseaudio-discuss] Handling of port latency offsets
On Fri, 2017-02-10 at 14:38 +0100, Georg Chini wrote:
> Hi,
>
> during my work on module-loopback I came upon an issue regarding the
> handling
> of the port latency offsets.
> The functions pa_{source,sink}_get_latency_within_thread() return the
> latencies
> including the offset. Those functions are used to determine the amount
> that has
> to be rewound during a sink move and also to calculate the time a volume
> change
> is delayed.
>
> I think it is wrong in those situations to include the offset,
> especially if a user sets
> very large or very negative offsets. If you have a large negative offset
> for example,
> the sink will never be rewound on a move because get_latency_in_thread()
> always
> returns 0. If the offset is large and positive, you might end up
> rewinding too much,
> although this is limited by max_rewind.
> A similar thought applies to the delay of the volume changes, although
> it is surely
> less critical there.
>
> Am I right or do I miss something?
You're right. The total latency is not the same thing as the rewindable
amount. I think there's a FIXME comment about this somewhere in the
stream moving code.
> Would it be a good idea to let the functions return a structure
> containing the
> actual latency and the offset in separate variables and let the caller
> sort out
> if it needs the offset or not?
The term "actual latency" includes the offset, if the offset is
correct. The important distinction is between "total latency" and
"rewindable amount". And yes, it would be a good idea to report these
separately.
--
Tanu
https://www.patreon.com/tanuk
___
pulseaudio-discuss mailing list
pulseaudio-discuss@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/pulseaudio-discuss
Re: [pulseaudio-discuss] combined sink does not work with pulseaudio-dlna
On 16.02.2017 11:50, Tanu Kaskinen wrote: On Wed, 2017-02-15 at 17:25 +0100, Ivan Kanis wrote: The author of the dlna plugin has told me that the combined sink feature is not implemented in his module. He has given me an alternative with the loopback module: load-module module-loopback sink=alsa_output.pci-_00_1b.0.analog-stereo Then pickup the loopback on the Chromecast. It works but, as expected, it is not in sync. So the source of the loopback is the monitor source of the chromecast sink? I am now looking for a way to delay the local playback but I can't figure out how. I tried latency_msec=3000 but it set's the latency on the Chromecast. If the source of the loopback is the monitor source of the chromecast sink, increasing the latency of the loopback module should increase the latency of what is played to the alsa sink, not the chromecast sink. It's a known problem that something is wrong in module-loopback related to the latency_msec paramter: https://bugs.freedesktop.org/show_bug.cgi?id=80770 The loopback patches that Georg Chini submitted recently probably fix the latency_msec parameter behaviour. The bug Tanu is referring to should already have been fixed with pulseaudio 10.0, although with the loopback module of 10.0 it will take quite a while until it has adjusted to the correct latency. You can check the latency of module-loopback if you enable debug logging. Regards Georg ___ pulseaudio-discuss mailing list pulseaudio-discuss@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/pulseaudio-discuss
Re: [pulseaudio-discuss] Access control
On Wed, 2017-02-15 at 11:34 +0100, Timothy Hobbs wrote: > Obviously, this question reveals my total ignorance of pulseaudio > architecture, but why are you implementing access control in pulseaudio > itself, rather than using a firewall wrapper that parses the info being > sent down the pulse audio socket and only lets allowed RPC calls through? If by a "firewall wrapper" you mean a separate process, I don't see what benefit that would have. As far as I can see, it would just complicate things. If you meant an in-process firewall, the first approach that Wim tried was kind of like that. One of the reasons why it wasn't liked that much was that it only works for the native protocol, and pulseaudio supports several communication protocols. While the native protocol is the most important one, it's nice to have support for all protocols without having to duplicate the effort. -- Tanu https://www.patreon.com/tanuk ___ pulseaudio-discuss mailing list pulseaudio-discuss@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/pulseaudio-discuss
Re: [pulseaudio-discuss] Pulse audio's UNIX sockets and other questions about pulse and containers
On Fri, 2017-02-10 at 19:08 +0100, Timothy Hobbs wrote:
> Hello,
>
> I'm trying to discover the best way to share pulse audio between linux
> containers. I have found a great deal of answers to this problem online.
> Some of them involve sharing UNIX sockets, while others suggest
> connecting via localhost using port tunneling or otherwise. I want to
> use UNIX sockets and take advantage of POSIX's existing and excelent
> naming, namespacing, and security mechanisms. I guess that shouldn't be
> a problem, because pulse audio seems to support unix sockets. But I'm
> having trouble understanding their layout or finding any documentation
> on what data gets transfered via which mechanism and which APIs are
> exposed via what protocol. So far I've found that there are unix sockets
> in the following places:
>
> - $HOME/.config/pulse/-runtime
>
> This is just a link to:
>
> - /tmp/-pulse/
>
> But how does this differ from the one in run?
>
> - /run/user/$UID/pulse/
Those two directories serve the exact same purpose.
/run/user/$UID/pulse is used when $XDG_RUNTIME_DIR is set, otherwise
pulseaudio creates the directory under /tmp.
> I seem to recall that there is also system wide socket, not associated
> with any single user of the system. However, I do not remember its location.
That socket only exists when pulseaudio runs in the system wide mode
(i.e. it doesn't exist by default). The location is
"${localstatedir}/run/pulse/native" (I think that usually gets expanded
to /var/run/pulse/native).
> There is also discussion of PULSE_AUDIO_COOKIE. Is this really needed
> when connecting to UNIX sockets? Are these sockets stand alone or does
> pulse audio also communicate over dbus or x11 or something?
In the most common setups the cookie is not used with unix sockets. In
the per-user setup the user who runs pulseaudio is always allowed to
connect, and other users aren't expected to connect anyway. In the
system-wide setup access is controlled via group membership (users not
in the "pulse-access" group are denied access).
It's possible to use the cookie authorization with unix sockets, but I
can't think of any situation where that would be the best solution.
Pulseaudio uses dbus and x11, but not for anything really important, so
I expect that you can live without them.
> For my usecase, the localhost option is a non-starter. It involves
> configuring networking in the container to allow for the tunneling of
> some ports which seems fiddly and like a can of worms from a security
> perspective. Even if I weren't useing containers, I don't like the idea
> of accessing services through localhost. Numbered ports with no human
> readable names? No namespacing between users? Security from the outside
> world ensured through conventions, manual checks, and hacks? Good greif!
>
> If you have other suggestions that don't involve networking, I'd be
> happy to hear them. I do hope, though, that if there isn't one already,
> we'll be able to create a page as informative as this one container page> for using pulse audio with containers.
Sharing the native protocol unix socket should work. If you want to use
a non-standard socket location, pass the "socket" option to module-
native-protocol-unix and set "default-server = /path/to/socket" in
client.conf so that clients find the socket.
I'd guess a container trying to connect to a socket from a different
container will get blocked by default. If you put the socket in a place
that only trusted users can access, then you can safely pass the auth-
anonymous option to module-native-protocol-unix.
--
Tanu
https://www.patreon.com/tanuk
___
pulseaudio-discuss mailing list
pulseaudio-discuss@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/pulseaudio-discuss
