si-audio-adapter/endpoint: do not sync() in loops, use ports-changed instead

Calling sync() in loops may end up looping forever if the operating
system's scheduling allows pipewire to reply to this sync() before
wireplumber's event loop has a chance to become idle.

In order for the new ports to appear, the object manager that monitors
them needs to emit "objects-changed", which only ever happens in an
idle callback. If the reply to sync() arrives before the idle callback,
it gets prioritized and processed, which causes another sync(), and so on...

Since setting PortFormat in the adapter always changes the ports,
watching for "ports-changed" feels like a better solution. Still,
there is more room for improvement.
This commit is contained in:
George Kiagiadakis 2021-06-18 12:21:29 +03:00
parent 87741e39ec
commit 8e5448fd0c
2 changed files with 48 additions and 77 deletions

View file

@ -261,12 +261,21 @@ on_node_enum_format_done (WpPipewireObject * proxy, GAsyncResult * res,
"dsp", on_format_set, transition);
}
static void
on_node_ports_changed (WpObject * node, WpSiAudioAdapter *self)
{
/* finish the task started by _set_ports_format() */
if (self->format_task && wp_node_get_n_ports (self->node) > 0) {
g_autoptr (GTask) t = g_steal_pointer (&self->format_task);
g_task_return_boolean (t, TRUE);
}
}
static void
on_feature_ports_ready (WpObject * node, GAsyncResult * res,
WpTransition * transition)
{
WpSiAudioAdapter *self = wp_transition_get_source_object (transition);
g_autoptr (WpCore) core = wp_object_get_core (WP_OBJECT (self));
g_autoptr (GError) error = NULL;
if (!wp_object_activate_finish (node, res, &error)) {
@ -274,6 +283,9 @@ on_feature_ports_ready (WpObject * node, GAsyncResult * res,
return;
}
g_signal_connect_object (node, "ports-changed",
(GCallback) on_node_ports_changed, self, 0);
/* If device node, enum available formats and set one of them */
if (self->is_device || self->dont_remix || !self->is_autoconnect)
wp_pipewire_object_enum_params (WP_PIPEWIRE_OBJECT (self->node),
@ -342,42 +354,6 @@ si_audio_adapter_get_ports_format (WpSiAdapter * item, const gchar **mode)
return self->format ? wp_spa_pod_ref (self->format) : NULL;
}
static void
on_sync_done (WpCore * core, GAsyncResult * res, WpSiAudioAdapter *self)
{
g_autoptr (GError) error = NULL;
guint32 active = 0;
if (!wp_core_sync_finish (core, res, &error)) {
g_autoptr (GTask) t = g_steal_pointer (&self->format_task);
g_task_return_error (t, g_steal_pointer (&error));
return;
}
active = wp_object_get_active_features (WP_OBJECT (self->node));
if (!(active & WP_NODE_FEATURE_PORTS)) {
g_autoptr (GTask) t = g_steal_pointer (&self->format_task);
g_task_return_new_error (t, WP_DOMAIN_LIBRARY,
WP_LIBRARY_ERROR_OPERATION_FAILED,
"node feature ports is not enabled, aborting set format operation");
return;
}
/* The task might be destroyed by set_ports_format before sync is finished.
* The set_ports_format API returns a task error if there is a pending task
* so we don't need to do anything here */
if (!self->format_task)
return;
/* make sure ports are available */
if (wp_node_get_n_ports (self->node) > 0) {
g_autoptr (GTask) t = g_steal_pointer (&self->format_task);
g_task_return_boolean (t, TRUE);
} else {
wp_core_sync (core, NULL, (GAsyncReadyCallback) on_sync_done, self);
}
}
static gboolean
parse_adapter_format (WpSpaPod *format, gint *channels,
WpSpaPod **position)
@ -461,6 +437,7 @@ si_audio_adapter_set_ports_format (WpSiAdapter * item, WpSpaPod *f,
g_autoptr (WpCore) core = wp_object_get_core (WP_OBJECT (self));
g_autoptr (WpSpaPod) format = f;
g_autoptr (WpSpaPod) new_format = NULL;
guint32 active = 0;
g_return_if_fail (core);
@ -484,6 +461,15 @@ si_audio_adapter_set_ports_format (WpSiAdapter * item, WpSpaPod *f,
return;
}
active = wp_object_get_active_features (WP_OBJECT (self->node));
if (G_UNLIKELY (!(active & WP_NODE_FEATURE_PORTS))) {
g_autoptr (GTask) t = g_steal_pointer (&self->format_task);
g_task_return_new_error (t, WP_DOMAIN_LIBRARY,
WP_LIBRARY_ERROR_OPERATION_FAILED,
"node feature ports is not enabled, aborting set format operation");
return;
}
/* set format and mode */
g_clear_pointer (&self->format, wp_spa_pod_unref);
self->format = g_steal_pointer (&new_format);
@ -500,8 +486,7 @@ si_audio_adapter_set_ports_format (WpSiAdapter * item, WpSpaPod *f,
"format", "P", self->format,
NULL));
/* sync until new ports are available */
wp_core_sync (core, NULL, (GAsyncReadyCallback) on_sync_done, self);
/* the task finishes with new ports being emitted -> on_node_ports_changed */
}
static gboolean

View file

@ -158,6 +158,16 @@ si_audio_endpoint_disable_exported (WpSessionItem *si)
WP_SESSION_ITEM_FEATURE_EXPORTED);
}
static void
on_node_ports_changed (WpObject * node, WpSiAudioEndpoint *self)
{
/* finish the task started by _set_ports_format() */
if (self->format_task && wp_node_get_n_ports (self->node) > 0) {
g_autoptr (GTask) t = g_steal_pointer (&self->format_task);
g_task_return_boolean (t, TRUE);
}
}
static void
on_node_activate_done (WpObject * node, GAsyncResult * res,
WpTransition * transition)
@ -170,6 +180,9 @@ on_node_activate_done (WpObject * node, GAsyncResult * res,
return;
}
g_signal_connect_object (node, "ports-changed",
(GCallback) on_node_ports_changed, self, 0);
wp_object_update_features (WP_OBJECT (self),
WP_SESSION_ITEM_FEATURE_ACTIVE, 0);
}
@ -372,42 +385,6 @@ si_audio_endpoint_get_ports_format (WpSiAdapter * item, const gchar **mode)
return self->format ? wp_spa_pod_ref (self->format) : NULL;
}
static void
on_sync_done (WpCore * core, GAsyncResult * res, WpSiAudioEndpoint *self)
{
g_autoptr (GError) error = NULL;
guint32 active = 0;
if (!wp_core_sync_finish (core, res, &error)) {
g_autoptr (GTask) t = g_steal_pointer (&self->format_task);
g_task_return_error (t, g_steal_pointer (&error));
return;
}
active = wp_object_get_active_features (WP_OBJECT (self->node));
if (!(active & WP_NODE_FEATURE_PORTS)) {
g_autoptr (GTask) t = g_steal_pointer (&self->format_task);
g_task_return_new_error (t, WP_DOMAIN_LIBRARY,
WP_LIBRARY_ERROR_OPERATION_FAILED,
"node feature ports is not enabled, aborting set format operation");
return;
}
/* The task might be destroyed by set_ports_format before sync is finished.
* The set_ports_format API returns a task error if there is a pending task
* so we don't need to do anything here */
if (!self->format_task)
return;
/* make sure ports are available */
if (wp_node_get_n_ports (self->node) > 0) {
g_autoptr (GTask) t = g_steal_pointer (&self->format_task);
g_task_return_boolean (t, TRUE);
} else {
wp_core_sync (core, NULL, (GAsyncReadyCallback) on_sync_done, self);
}
}
static gboolean
parse_adapter_format (WpSpaPod *format, gint *channels,
WpSpaPod **position)
@ -491,6 +468,7 @@ si_audio_endpoint_set_ports_format (WpSiAdapter * item, WpSpaPod *f,
g_autoptr (WpCore) core = wp_object_get_core (WP_OBJECT (self));
g_autoptr (WpSpaPod) format = f;
g_autoptr (WpSpaPod) new_format = NULL;
guint32 active = 0;
g_return_if_fail (core);
@ -514,6 +492,15 @@ si_audio_endpoint_set_ports_format (WpSiAdapter * item, WpSpaPod *f,
return;
}
active = wp_object_get_active_features (WP_OBJECT (self->node));
if (G_UNLIKELY (!(active & WP_NODE_FEATURE_PORTS))) {
g_autoptr (GTask) t = g_steal_pointer (&self->format_task);
g_task_return_new_error (t, WP_DOMAIN_LIBRARY,
WP_LIBRARY_ERROR_OPERATION_FAILED,
"node feature ports is not enabled, aborting set format operation");
return;
}
/* set format and mode */
g_clear_pointer (&self->format, wp_spa_pod_unref);
self->format = g_steal_pointer (&new_format);
@ -529,8 +516,7 @@ si_audio_endpoint_set_ports_format (WpSiAdapter * item, WpSpaPod *f,
"format", "P", self->format,
NULL));
/* sync until new ports are available */
wp_core_sync (core, NULL, (GAsyncReadyCallback) on_sync_done, self);
/* the task finishes with new ports being emitted -> on_node_ports_changed */
}
static gboolean