Re: [pulseaudio-discuss] pulseaudio master is frozen
On Mon, 2019-07-01 at 11:29 +0530, Arun Raghavan wrote: > Hi folks, > As we were discussing, 13.0 is a bit overdue, so we're freezing > master now. Release tracker issue is at: > > https://gitlab.freedesktop.org/pulseaudio/pulseaudio/issues/174 > > Just as a refresher, this means we will not be landing any new > features to master without an explicit exception, and only critical > bug fixes should land outside of those changes. > > We used to keep a temporary 'next' branch for the times where the > freeze cycle went longer and we didn't want to hold reviewed patches > back. This may not be necessary any more with the Gitlab merge > request system > > We have discussed an exception for Pali's initial A2DP fixes, > Russell's info script, and the fixes for the pending bug on the > tracker relating to detecting when we are running in a VM. I had a look at where we are with Pali's patches, and it seems that the patches that we really want are reviewed, and there were some improvements requested. Therefore this part is currently blocked on waiting for new patch versions. Pali, do you have fixes for the first three (or even just the first two) patches? These: * bluetooth: Fix A2DP codec API to provide information about data buffer * bluetooth: Fix usage of RTP structures in SBC codec * bluetooth: Implement reading SO_TIMESTAMP for A2DP source -- Tanu https://www.patreon.com/tanuk https://liberapay.com/tanuk ___ pulseaudio-discuss mailing list pulseaudio-discuss@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/pulseaudio-discuss
Re: [pulseaudio-discuss] [PATCH v2] bluetooth: Fix crash in setup_stream()
On Tue, 2019-06-18 at 10:15 +0200, Frédéric Danis wrote:
> setup_stream() crashes when calling set_nonblock() with an invalid
> stream_fd.
>
> On a new call, the ofono backend gets notified of a new connection.
> The ofono backend sets the transport state to playing, and that triggers
> a profile change, which sets up the stream for the first time.
> Then module-bluetooth-policy sets up the loopbacks. The loopbacks get
> fully initialized before the crash.
>
> After module-bluetooth-policy has done its things, the execution
> continues in the transport state change hook. The next hook user is
> module-bluez5-device, whose handle_transport_state_change() function
> gets called. It will then set up the stream again even though it's
> already set up. I'm not sure if that's a some kind of a bug.
> setup_stream() can handle the case where it's unnecessarily called,
> though, so this second setup is not a big problem.
>
> The crash happens, because the connection died due to POLLHUP in the IO
> thread before the second setup_stream() call.
> ---
> Changes in v2:
> * Update comments and commit message
>
> src/modules/bluetooth/module-bluez5-device.c | 26 ++--
> 1 file changed, 24 insertions(+), 2 deletions(-)
>
> diff --git a/src/modules/bluetooth/module-bluez5-device.c
> b/src/modules/bluetooth/module-bluez5-device.c
> index 56c96054d..91c678223 100644
> --- a/src/modules/bluetooth/module-bluez5-device.c
> +++ b/src/modules/bluetooth/module-bluez5-device.c
> @@ -753,6 +753,8 @@ static void setup_stream(struct userdata *u) {
> struct pollfd *pollfd;
> int one;
>
> +pa_assert(u->stream_fd >= 0);
> +
> /* return if stream is already set up */
> if (u->stream_setup_done)
> return;
> @@ -829,7 +831,17 @@ static int source_process_msg(pa_msgobject *o, int code,
> void *data, int64_t off
> }
>
> case PA_SOURCE_MESSAGE_SETUP_STREAM:
> -setup_stream(u);
> +/* Skip stream setup if stream_fd has been invalidated.
> + This can occur if the stream has already been set up and
> + then immediately received POLLHUP. If the stream has
> + already been set up earlier, then this setup_stream()
> + call is redundant anyway, but currently the code
> + is such that this kind of unnecessary setup_stream()
> + calls can happen. */
> +if (u->stream_fd < 0)
> +pa_log_debug("Skip source stream setup while closing");
> +else
> +setup_stream(u);
> return 0;
>
> }
> @@ -1007,7 +1019,17 @@ static int sink_process_msg(pa_msgobject *o, int code,
> void *data, int64_t offse
> }
>
> case PA_SINK_MESSAGE_SETUP_STREAM:
> -setup_stream(u);
> +/* Skip stream setup if stream_fd has been invalidated.
> + This can occur if the stream has already been set up and
> + then immediately received POLLHUP. If the stream has
> + already been set up earlier, then this setup_stream()
> + call is redundant anyway, but currently the code
> + is such that this kind of unnecessary setup_stream()
> + calls can happen. */
> +if (u->stream_fd < 0)
> +pa_log_debug("Skip sink stream setup while closing");
> +else
> +setup_stream(u);
> return 0;
> }
>
Thanks! Applied.
--
Tanu
https://www.patreon.com/tanuk
https://liberapay.com/tanuk
___
pulseaudio-discuss mailing list
pulseaudio-discuss@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/pulseaudio-discuss
Re: [pulseaudio-discuss] Pavucontrol on Flathub
On Tue, 2 Jul 2019, at 10:28 AM, Tanu Kaskinen wrote: [...] > Once we've set up the procedures for maintaining the flatpak ourselves, > is there any reason to have a separate repository for it? Shouldn't the > flatpak stuff go to the main pavucontrol repo? Not that I have anything > against having push rights to the current github repo for now. (By "the > procedures" I mean documenting how to update the flatpak build files > and how to test them, and following that documentation when doing > releases.) I think it might be worth having it on flathub to be able to use all the existing CI infrastructure over there. Cheers, Arun ___ pulseaudio-discuss mailing list pulseaudio-discuss@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/pulseaudio-discuss
Re: [pulseaudio-discuss] Pavucontrol on Flathub
On Mon, 2019-07-01 at 11:34 +0530, Arun Raghavan wrote: > On Mon, 1 Jul 2019, at 11:26 AM, Tanu Kaskinen wrote: > > On Sat, 2019-06-29 at 10:37 +0530, Arun Raghavan wrote: > > > Hi folks, > > > Just a heads-up -- Will Thompson has nice enough to create a flatpak > > > for pavucontrol. It's on flathub, so you can see it at: > > > > > > https://flathub.org/apps/details/org.pulseaudio.pavucontrol > > > > > > Instructions on installing are also on that page. > > > > > > For those who are unfamiliar with it, flatpak provides a way to run > > > applications in a sandbox, against a standard distribution-neutral > > > runtime. Flathub is a repository for a number of such applications. > > > > > > The code for the flatpak of pavucontrol is at: > > > > > > https://github.com/flathub/org.pulseaudio.pavucontrol > > > > That's great! > > > > What are your views on what we should or shouldn't do with this in the > > future? Ideally we'd maintain it upstream, because as a user I'd prefer > > to trust the upstream rather than some third party when installing > > flatpaks (and if the upstream maintains the flatpak, it will probably > > be always up to date). It would just mean some extra work for us when > > doing releases. > > Yup, I think we should maintain this (Will had asked that we do that > while writing it, and and I'd signed up for that but more hands are > definitely welcome). I'll ask Nick Richards to give write permissions > to you and George (or ideally the PulseAudio organisation). Once we've set up the procedures for maintaining the flatpak ourselves, is there any reason to have a separate repository for it? Shouldn't the flatpak stuff go to the main pavucontrol repo? Not that I have anything against having push rights to the current github repo for now. (By "the procedures" I mean documenting how to update the flatpak build files and how to test them, and following that documentation when doing releases.) > > At least the appdata.xml file looks like something that we should > > maintain ourselves. > > Agreed, but it depends on dropping intltool apparently. Which you seem to have done in a merge request already. Thanks! -- Tanu https://www.patreon.com/tanuk https://liberapay.com/tanuk ___ pulseaudio-discuss mailing list pulseaudio-discuss@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/pulseaudio-discuss
Re: [pulseaudio-discuss] [PATCH v2 4/8] core: move sink-inputs conditionally when update default_sink
On Mon, 2019-07-01 at 08:48 +0200, Georg Chini wrote:
> On 01.07.19 08:03, Georg Chini wrote:
> > On 01.07.19 07:08, Tanu Kaskinen wrote:
> > > On Sun, 2019-06-30 at 13:52 +0200, Georg Chini wrote:
> > > > On 17.01.19 07:53, Hui Wang wrote:
> > > > > When the default sink changes, the streams from the old default sink
> > > > > should be moved to the new default sink, unless the preferred_sink
> > > > > string is set to the old default sink and the active port of the old
> > > > > default sink is not unavailable
>
>
> > > > >+
> > > > > +void pa_sink_move_streams_to_default_sink(pa_core *core, pa_sink
> > > > > *old_sink, bool from_user) {
> > > > > +pa_sink_input *i;
> > > > > +uint32_t idx;
> > > > > +bool old_sink_is_unavailable;
> > > > Does this not give a warning about using uninitialized variables?
> > > > > +
> > > > > +pa_assert(core);
> > > > > +pa_assert(old_sink);
> > > > > +
> > > > > +if (old_sink == core->default_sink)
> > > > > +return;
> > > > > +
> > > > > +if (old_sink->active_port && old_sink->active_port->available
> > > > > == PA_AVAILABLE_NO)
> > > > > +old_sink_is_unavailable = true;
> > > > This is not sufficient to determine if the old sink is still available.
> > > > There are sinks
> > > > that do not have ports (null-sink, virtual sinks). I think you will
> > > > need
> > > > another boolean
> > > > argument to the function which indicates availability of the old sink.
> > > The definition of an unavailable sink is that its active port is
> > > unavailable. I don't know what kind of sinks you're thinking about
> > > here, maybe virtual sinks that are on top of an unavailable hardware
> > > sink? There are many places where we check the availability of a sink
> > > this way, and I don't think it makes sense to be different here.
> >
> > I don't understand. Virtual sinks don't have ports. So checking only
> > sinks that have an active port excludes all sinks that don't have
> > ports like the null-sink and virtual sinks. Following your definition
> > above, those sinks are never available.
You have it reversed: following my definition above, those sinks are
always available.
> Checking the code, only alsa, bluetooth and raop sinks define ports and
> the "many places" you are referring to are compare_sinks() and
> compare_sources() in core.c and a check in module-switch-on-connect,
> which is used to determine the availability of the default sink.
At least module-stream-restore checks device availability too (although
probably not anymore after this patch set, because module-stream-
restore won't do the routing directly anymore, it will just restore the
preferred_sink variable, which can be done regardless of the sink
availability).
Extending the hardware sink availability to filter sinks probably makes
sense, but I think that's a topic for a separate patch (or patches).
You suggested an extra flag for pa_sink_move_streams_to_default_sink(),
which seems unnecessary to me. The function can in any case figure out
the availability by itself (possibly using a helper function
pa_sink_is_available() that can handle filter sinks too).
--
Tanu
https://www.patreon.com/tanuk
https://liberapay.com/tanuk
___
pulseaudio-discuss mailing list
pulseaudio-discuss@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/pulseaudio-discuss
Re: [pulseaudio-discuss] [PATCH v2 7/8] sink: move the streams to the default_sink when the sink is unlinked
On 17.01.19 07:53, Hui Wang wrote: When a sink is unlinked, all streams of this sink are moved to default_sink, this action is implemented in the core rather than modules now. Signed-off-by: Hui Wang --- src/modules/module-stream-restore.c | 50 - src/pulsecore/sink.c| 3 ++ 2 files changed, 3 insertions(+), 50 deletions(-) module-rescue-stream also has to be modified to reflect that change. It should now only rescue source streams. ___ pulseaudio-discuss mailing list pulseaudio-discuss@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/pulseaudio-discuss
Re: [pulseaudio-discuss] [PATCH v2 4/8] core: move sink-inputs conditionally when update default_sink
On 01.07.19 08:03, Georg Chini wrote:
On 01.07.19 07:08, Tanu Kaskinen wrote:
On Sun, 2019-06-30 at 13:52 +0200, Georg Chini wrote:
On 17.01.19 07:53, Hui Wang wrote:
When the default sink changes, the streams from the old default sink
should be moved to the new default sink, unless the preferred_sink
string is set to the old default sink and the active port of the old
default sink is not unavailable
+
+void pa_sink_move_streams_to_default_sink(pa_core *core, pa_sink
*old_sink, bool from_user) {
+ pa_sink_input *i;
+ uint32_t idx;
+ bool old_sink_is_unavailable;
Does this not give a warning about using uninitialized variables?
+
+ pa_assert(core);
+ pa_assert(old_sink);
+
+ if (old_sink == core->default_sink)
+ return;
+
+ if (old_sink->active_port && old_sink->active_port->available
== PA_AVAILABLE_NO)
+ old_sink_is_unavailable = true;
This is not sufficient to determine if the old sink is still available.
There are sinks
that do not have ports (null-sink, virtual sinks). I think you will
need
another boolean
argument to the function which indicates availability of the old sink.
The definition of an unavailable sink is that its active port is
unavailable. I don't know what kind of sinks you're thinking about
here, maybe virtual sinks that are on top of an unavailable hardware
sink? There are many places where we check the availability of a sink
this way, and I don't think it makes sense to be different here.
I don't understand. Virtual sinks don't have ports. So checking only
sinks that have an active port excludes all sinks that don't have
ports like the null-sink and virtual sinks. Following your definition
above, those sinks are never available.
Checking the code, only alsa, bluetooth and raop sinks define ports and
the "many places" you are referring to are compare_sinks() and
compare_sources() in core.c and a check in module-switch-on-connect,
which is used to determine the availability of the default sink.
___
pulseaudio-discuss mailing list
pulseaudio-discuss@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/pulseaudio-discuss
Re: [pulseaudio-discuss] [PATCH v2 4/8] core: move sink-inputs conditionally when update default_sink
On 01.07.19 07:08, Tanu Kaskinen wrote:
On Sun, 2019-06-30 at 13:52 +0200, Georg Chini wrote:
On 17.01.19 07:53, Hui Wang wrote:
When the default sink changes, the streams from the old default sink
should be moved to the new default sink, unless the preferred_sink
string is set to the old default sink and the active port of the old
default sink is not unavailable
Signed-off-by: Hui Wang
---
src/modules/dbus/iface-core.c | 2 +-
src/modules/module-default-device-restore.c | 2 +-
src/modules/module-switch-on-connect.c | 27 ++--
src/pulsecore/cli-command.c | 2 +-
src/pulsecore/core.c| 10 --
src/pulsecore/core.h| 4 +--
src/pulsecore/device-port.c | 2 +-
src/pulsecore/protocol-native.c | 2 +-
src/pulsecore/sink.c| 35 +++--
src/pulsecore/sink.h| 6
10 files changed, 54 insertions(+), 38 deletions(-)
diff --git a/src/modules/dbus/iface-core.c b/src/modules/dbus/iface-core.c
index 5229c0467..9763480d2 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_core_set_configured_default_sink(c->core, pa_dbusiface_device_get_sink(fallback_sink)->name);
+pa_core_set_configured_default_sink(c->core,
pa_dbusiface_device_get_sink(fallback_sink)->name, true);
pa_dbus_send_empty_reply(conn, msg);
}
diff --git a/src/modules/module-default-device-restore.c
b/src/modules/module-default-device-restore.c
index c4dbad99f..33e74c071 100644
--- a/src/modules/module-default-device-restore.c
+++ b/src/modules/module-default-device-restore.c
@@ -69,7 +69,7 @@ static void load(struct userdata *u) {
pa_log_warn("Invalid sink name: %s", ln);
else {
pa_log_info("Restoring default sink '%s'.", ln);
-pa_core_set_configured_default_sink(u->core, ln);
+pa_core_set_configured_default_sink(u->core, ln, false);
}
} else if (errno != ENOENT)
diff --git a/src/modules/module-switch-on-connect.c
b/src/modules/module-switch-on-connect.c
index f0cb29a7c..3ceac8b4f 100644
--- a/src/modules/module-switch-on-connect.c
+++ b/src/modules/module-switch-on-connect.c
@@ -54,9 +54,6 @@ struct userdata {
};
static pa_hook_result_t sink_put_hook_callback(pa_core *c, pa_sink *sink, void* userdata) {
-pa_sink_input *i;
-uint32_t idx;
-pa_sink *old_default_sink;
const char *s;
struct userdata *u = userdata;
@@ -88,7 +85,7 @@ static pa_hook_result_t sink_put_hook_callback(pa_core *c, pa_sink *sink, void*
/* No default sink, nothing to move away, just set the new default */
if (!c->default_sink) {
-pa_core_set_configured_default_sink(c, sink->name);
+pa_core_set_configured_default_sink(c, sink->name, false);
return PA_HOOK_OK;
}
@@ -103,28 +100,8 @@ static pa_hook_result_t sink_put_hook_callback(pa_core *c, pa_sink *sink, void*
return PA_HOOK_OK;
}
-old_default_sink = c->default_sink;
-
/* Actually do the switch to the new sink */
-pa_core_set_configured_default_sink(c, sink->name);
-
-/* Now move all old inputs over */
-if (pa_idxset_size(old_default_sink->inputs) <= 0) {
-pa_log_debug("No sink inputs to move away.");
-return PA_HOOK_OK;
-}
-
-PA_IDXSET_FOREACH(i, old_default_sink->inputs, idx) {
-if (pa_safe_streq(i->sink->name, i->preferred_sink) ||
!PA_SINK_INPUT_IS_LINKED(i->state))
-continue;
-
-if (pa_sink_input_move_to(i, sink, false) < 0)
-pa_log_info("Failed to move sink input %u \"%s\" to %s.", i->index,
-pa_strnull(pa_proplist_gets(i->proplist,
PA_PROP_APPLICATION_NAME)), sink->name);
-else
-pa_log_info("Successfully moved sink input %u \"%s\" to %s.",
i->index,
-pa_strnull(pa_proplist_gets(i->proplist,
PA_PROP_APPLICATION_NAME)), sink->name);
-}
+pa_core_set_configured_default_sink(c, sink->name, false);
I wonder if we could use something like
pa_core_set_temporary_default_sink() here
and have a variable core->temporary_default_sink that has even higher
priority
than core->configured_default_sink in the default sink selection
process. The temporary
default sink could be reset when the sink vanishes again or replaced
when another new
sink turns up. What module-switch-on-connect does is to temporarily
override the default
sink that the user configured. If we save this in another variable we
would not overwrite
the user configured default sink by a sink that is not expected to be
present the next
time PA is started. But that would be just nice to have.
I'm
Re: [pulseaudio-discuss] Pavucontrol on Flathub
On Mon, 1 Jul 2019, at 11:26 AM, Tanu Kaskinen wrote: > On Sat, 2019-06-29 at 10:37 +0530, Arun Raghavan wrote: > > Hi folks, > > Just a heads-up -- Will Thompson has nice enough to create a flatpak > > for pavucontrol. It's on flathub, so you can see it at: > > > > https://flathub.org/apps/details/org.pulseaudio.pavucontrol > > > > Instructions on installing are also on that page. > > > > For those who are unfamiliar with it, flatpak provides a way to run > > applications in a sandbox, against a standard distribution-neutral > > runtime. Flathub is a repository for a number of such applications. > > > > The code for the flatpak of pavucontrol is at: > > > > https://github.com/flathub/org.pulseaudio.pavucontrol > > That's great! > > What are your views on what we should or shouldn't do with this in the > future? Ideally we'd maintain it upstream, because as a user I'd prefer > to trust the upstream rather than some third party when installing > flatpaks (and if the upstream maintains the flatpak, it will probably > be always up to date). It would just mean some extra work for us when > doing releases. Yup, I think we should maintain this (Will had asked that we do that while writing it, and and I'd signed up for that but more hands are definitely welcome). I'll ask Nick Richards to give write permissions to you and George (or ideally the PulseAudio organisation). > At least the appdata.xml file looks like something that we should > maintain ourselves. Agreed, but it depends on dropping intltool apparently. > We should probably advertise this on the pavucontrol home page. Agreed! Cheers, Arun ___ pulseaudio-discuss mailing list pulseaudio-discuss@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/pulseaudio-discuss
[pulseaudio-discuss] pulseaudio master is frozen
Hi folks, As we were discussing, 13.0 is a bit overdue, so we're freezing master now. Release tracker issue is at: https://gitlab.freedesktop.org/pulseaudio/pulseaudio/issues/174 Just as a refresher, this means we will not be landing any new features to master without an explicit exception, and only critical bug fixes should land outside of those changes. We used to keep a temporary 'next' branch for the times where the freeze cycle went longer and we didn't want to hold reviewed patches back. This may not be necessary any more with the Gitlab merge request system We have discussed an exception for Pali's initial A2DP fixes, Russell's info script, and the fixes for the pending bug on the tracker relating to detecting when we are running in a VM. Cheers, Arun ___ pulseaudio-discuss mailing list pulseaudio-discuss@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/pulseaudio-discuss
