Skip to content
Snippets Groups Projects
Commit a8b4e1f4 authored by Frederic Danis's avatar Frederic Danis Committed by Apertis CI
Browse files

d/patches: Fix audio sound in VirtualBox


When starting VirtualBox image, there's no audio output and application
like `pw-cat` freeze when playing a sound file.

Signed-off-by: default avatarFrédéric Danis <frederic.danis@collabora.com>
parent 82c7858c
No related branches found
No related tags found
1 merge request!6Import Upstream version 0.4.0
From 8e5448fd0cb597b6bc2e5994c2d4f5bb7246203b Mon Sep 17 00:00:00 2001
From: George Kiagiadakis <george.kiagiadakis@collabora.com>
Date: Fri, 18 Jun 2021 12:21:29 +0300
Subject: [PATCH] si-audio-adapter/endpoint: do not sync() in loops, use
ports-changed instead
Content-Type: text/plain; charset="utf-8"
Content-Transfer-Encoding: 8bit
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.
---
modules/module-si-audio-adapter.c | 63 ++++++++++++------------------
modules/module-si-audio-endpoint.c | 62 ++++++++++++-----------------
2 files changed, 48 insertions(+), 77 deletions(-)
diff --git a/modules/module-si-audio-adapter.c b/modules/module-si-audio-adapter.c
index 305898c..3a1cae6 100644
--- a/modules/module-si-audio-adapter.c
+++ b/modules/module-si-audio-adapter.c
@@ -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
diff --git a/modules/module-si-audio-endpoint.c b/modules/module-si-audio-endpoint.c
index 9f74ce4..f17c049 100644
--- a/modules/module-si-audio-endpoint.c
+++ b/modules/module-si-audio-endpoint.c
@@ -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
--
2.25.1
apertis/0001-si-audio-adapter-endpoint-do-not-sync-in-loops-use-p.patch
0% Loading or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment