From 3fc9582b6a9437f7c72c5227d998470cee2d2412 Mon Sep 17 00:00:00 2001 From: Julian Bouzas Date: Thu, 11 Jul 2019 09:55:15 -0400 Subject: [PATCH 1/8] dsp: use the new WpProxyLink API --- .../module-pw-audio-softdsp-endpoint/dsp.c | 26 ++++++++++++++----- 1 file changed, 20 insertions(+), 6 deletions(-) diff --git a/modules/module-pw-audio-softdsp-endpoint/dsp.c b/modules/module-pw-audio-softdsp-endpoint/dsp.c index 0c0f6b18..7c2494f6 100644 --- a/modules/module-pw-audio-softdsp-endpoint/dsp.c +++ b/modules/module-pw-audio-softdsp-endpoint/dsp.c @@ -58,7 +58,7 @@ struct _WpPwAudioDsp /* Proxies */ WpProxyNode *proxy; GPtrArray *port_proxies; - struct pw_proxy *link_proxy; + WpProxyLink *link_proxy; /* Listener */ struct spa_hook listener; @@ -218,11 +218,22 @@ on_audio_dsp_port_added(WpRemotePipewire *rp, guint id, guint parent_id, GUINT_TO_POINTER(parent_id)); } +static void +on_proxy_link_created(GObject *initable, GAsyncResult *res, gpointer data) +{ + WpPwAudioDsp *self = data; + + /* Get the link */ + self->link_proxy = wp_proxy_link_new_finish(initable, res, NULL); + g_return_if_fail (self->link_proxy); +} + static void on_audio_dsp_running(WpPwAudioDsp *self) { struct pw_properties *props; const struct pw_node_info *dsp_info = NULL; + struct pw_proxy *proxy = NULL; /* Return if the node has already been linked */ if (self->link_proxy) @@ -252,8 +263,10 @@ on_audio_dsp_running(WpPwAudioDsp *self) g_debug ("%p linking DSP to node", self); /* Create the link */ - self->link_proxy = wp_remote_pipewire_create_object(self->remote_pipewire, + proxy = wp_remote_pipewire_create_object(self->remote_pipewire, "link-factory", PW_TYPE_INTERFACE_Link, &props->dict); + wp_proxy_link_new (pw_proxy_get_id(proxy), proxy, on_proxy_link_created, + self); /* Clean up */ pw_properties_free(props); @@ -262,10 +275,8 @@ on_audio_dsp_running(WpPwAudioDsp *self) static void on_audio_dsp_idle (WpPwAudioDsp *self) { - if (self->link_proxy != NULL) { - pw_proxy_destroy (self->link_proxy); - self->link_proxy = NULL; - } + /* Clear the proxy */ + g_clear_object (&self->link_proxy); } static void @@ -413,6 +424,9 @@ wp_pw_audio_dsp_finalize (GObject * object) self->port_proxies = NULL; } + /* Destroy the link proxy */ + g_clear_object (&self->link_proxy); + G_OBJECT_CLASS (wp_pw_audio_dsp_parent_class)->finalize (object); } From dbd763bc9c5d787a8fa361edc082e5b68b97cf03 Mon Sep 17 00:00:00 2001 From: Julian Bouzas Date: Thu, 11 Jul 2019 12:06:04 -0400 Subject: [PATCH 2/8] proxy: throw an error if the proxy is destroyed during async constructions --- lib/wp/proxy-link.c | 22 +++++++++++++++++++++- lib/wp/proxy-node.c | 22 +++++++++++++++++++++- lib/wp/proxy-port.c | 20 ++++++++++++++++++++ lib/wp/proxy.c | 4 ++++ lib/wp/proxy.h | 3 +++ 5 files changed, 69 insertions(+), 2 deletions(-) diff --git a/lib/wp/proxy-link.c b/lib/wp/proxy-link.c index a0f6733f..c7b5adee 100644 --- a/lib/wp/proxy-link.c +++ b/lib/wp/proxy-link.c @@ -6,6 +6,7 @@ * SPDX-License-Identifier: MIT */ +#include "error.h" #include "proxy-link.h" #include @@ -15,7 +16,7 @@ struct _WpProxyLink /* The task to signal the proxy is initialized */ GTask *init_task; - + /* The link proxy listener */ struct spa_hook listener; @@ -69,6 +70,22 @@ wp_proxy_link_finalize (GObject * object) G_OBJECT_CLASS (wp_proxy_link_parent_class)->finalize (object); } +static void +wp_proxy_link_destroy (WpProxy * proxy) +{ + WpProxyLink *self = WP_PROXY_LINK(proxy); + GError *error = NULL; + + /* Return error if the pipewire destruction happened while the async creation + * of this proxy link object has not finished */ + if (self->init_task) { + g_set_error (&error, WP_DOMAIN_LIBRARY, WP_LIBRARY_ERROR_OPERATION_FAILED, + "pipewire link proxy destroyed before finishing"); + g_task_return_error (self->init_task, error); + g_clear_object (&self->init_task); + } +} + static void wp_proxy_link_init_async (GAsyncInitable *initable, int io_priority, GCancellable *cancellable, GAsyncReadyCallback callback, gpointer data) @@ -105,8 +122,11 @@ static void wp_proxy_link_class_init (WpProxyLinkClass * klass) { GObjectClass *object_class = (GObjectClass *) klass; + WpProxyClass *proxy_class = (WpProxyClass *) klass; object_class->finalize = wp_proxy_link_finalize; + + proxy_class->destroy = wp_proxy_link_destroy; } void diff --git a/lib/wp/proxy-node.c b/lib/wp/proxy-node.c index 89ca19e5..46569e31 100644 --- a/lib/wp/proxy-node.c +++ b/lib/wp/proxy-node.c @@ -6,6 +6,7 @@ * SPDX-License-Identifier: MIT */ +#include "error.h" #include "proxy-node.h" #include @@ -15,7 +16,7 @@ struct _WpProxyNode /* The task to signal the proxy is initialized */ GTask *init_task; - + /* The node proxy listener */ struct spa_hook listener; @@ -69,6 +70,22 @@ wp_proxy_node_finalize (GObject * object) G_OBJECT_CLASS (wp_proxy_node_parent_class)->finalize (object); } +static void +wp_proxy_node_destroy (WpProxy * proxy) +{ + WpProxyNode *self = WP_PROXY_NODE(proxy); + GError *error = NULL; + + /* Return error if the pipewire destruction happened while the async creation + * of this proxy node object has not finished */ + if (self->init_task) { + g_set_error (&error, WP_DOMAIN_LIBRARY, WP_LIBRARY_ERROR_OPERATION_FAILED, + "pipewire node proxy destroyed before finishing"); + g_task_return_error (self->init_task, error); + g_clear_object (&self->init_task); + } +} + static void wp_proxy_node_init_async (GAsyncInitable *initable, int io_priority, GCancellable *cancellable, GAsyncReadyCallback callback, gpointer data) @@ -105,8 +122,11 @@ static void wp_proxy_node_class_init (WpProxyNodeClass * klass) { GObjectClass *object_class = (GObjectClass *) klass; + WpProxyClass *proxy_class = (WpProxyClass *) klass; object_class->finalize = wp_proxy_node_finalize; + + proxy_class->destroy = wp_proxy_node_destroy; } void diff --git a/lib/wp/proxy-port.c b/lib/wp/proxy-port.c index 4255381b..c796e5a2 100644 --- a/lib/wp/proxy-port.c +++ b/lib/wp/proxy-port.c @@ -6,6 +6,7 @@ * SPDX-License-Identifier: MIT */ +#include "error.h" #include "proxy-port.h" #include #include @@ -99,6 +100,22 @@ wp_proxy_port_finalize (GObject * object) G_OBJECT_CLASS (wp_proxy_port_parent_class)->finalize (object); } +static void +wp_proxy_port_destroy (WpProxy * proxy) +{ + WpProxyPort *self = WP_PROXY_PORT(proxy); + GError *error = NULL; + + /* Return error if the pipewire destruction happened while the async creation + * of this proxy port object has not finished */ + if (self->init_task) { + g_set_error (&error, WP_DOMAIN_LIBRARY, WP_LIBRARY_ERROR_OPERATION_FAILED, + "pipewire port proxy destroyed before finishing"); + g_task_return_error (self->init_task, error); + g_clear_object (&self->init_task); + } +} + static void wp_proxy_port_init_async (GAsyncInitable *initable, int io_priority, GCancellable *cancellable, GAsyncReadyCallback callback, gpointer data) @@ -139,8 +156,11 @@ static void wp_proxy_port_class_init (WpProxyPortClass * klass) { GObjectClass *object_class = (GObjectClass *) klass; + WpProxyClass *proxy_class = (WpProxyClass *) klass; object_class->finalize = wp_proxy_port_finalize; + + proxy_class->destroy = wp_proxy_port_destroy; } void diff --git a/lib/wp/proxy.c b/lib/wp/proxy.c index 360b50df..b14608b4 100644 --- a/lib/wp/proxy.c +++ b/lib/wp/proxy.c @@ -50,6 +50,10 @@ proxy_event_destroy (void *data) /* Set the proxy to NULL */ self->proxy = NULL; + + /* Call the destroy method */ + if (WP_PROXY_GET_CLASS (data)->destroy) + WP_PROXY_GET_CLASS (data)->destroy (data); } static void diff --git a/lib/wp/proxy.h b/lib/wp/proxy.h index 5dab6a80..52889874 100644 --- a/lib/wp/proxy.h +++ b/lib/wp/proxy.h @@ -23,6 +23,9 @@ struct _WpProxyClass { GObjectClass parent_class; + /* Methods */ + void (*destroy) (WpProxy * self); + /* Signals */ void (*done)(WpProxy *wp_proxy, gpointer data); }; From 755f0bd862b79249ae95a3564a3c4b3bde4c4424 Mon Sep 17 00:00:00 2001 From: Julian Bouzas Date: Thu, 11 Jul 2019 13:50:37 -0400 Subject: [PATCH 3/8] modules: handle error if the endpoint proxies could not be created --- modules/module-pipewire/simple-endpoint.c | 49 +++++++++++++-- modules/module-pw-alsa-udev.c | 9 ++- modules/module-pw-audio-client.c | 9 ++- modules/module-pw-audio-softdsp-endpoint.c | 61 ++++++++++++++++--- .../module-pw-audio-softdsp-endpoint/dsp.c | 52 ++++++++++++++-- 5 files changed, 161 insertions(+), 19 deletions(-) diff --git a/modules/module-pipewire/simple-endpoint.c b/modules/module-pipewire/simple-endpoint.c index ea15eecc..cb8bc9bc 100644 --- a/modules/module-pipewire/simple-endpoint.c +++ b/modules/module-pipewire/simple-endpoint.c @@ -31,6 +31,7 @@ struct _WpPipewireSimpleEndpoint /* The task to signal the endpoint is initialized */ GTask *init_task; + gboolean init_abort; /* The remote pipewire */ WpRemotePipewire *remote_pipewire; @@ -74,6 +75,37 @@ G_DEFINE_TYPE_WITH_CODE (WpPipewireSimpleEndpoint, simple_endpoint, G_IMPLEMENT_INTERFACE (G_TYPE_ASYNC_INITABLE, wp_simple_endpoint_async_initable_init)) +typedef GObject* (*WpObjectNewFinishFunc)(GObject *initable, GAsyncResult *res, + GError **error); + +static GObject * +object_safe_new_finish(WpPipewireSimpleEndpoint * self, GObject *initable, + GAsyncResult *res, WpObjectNewFinishFunc new_finish_func) +{ + GObject *object = NULL; + GError *error = NULL; + + /* Return NULL if we are already aborting */ + if (self->init_abort) + return NULL; + + /* Get the object */ + object = G_OBJECT (new_finish_func (initable, res, &error)); + g_return_val_if_fail (object, NULL); + + /* Check for error */ + if (error) { + g_clear_object (&object); + g_warning ("WpPipewireSimpleEndpoint:%p Aborting construction", self); + self->init_abort = TRUE; + g_task_return_error (self->init_task, error); + g_clear_object (&self->init_task); + return NULL; + } + + return object; +} + static void node_proxy_param (void *object, int seq, uint32_t id, uint32_t index, uint32_t next, const struct spa_pod *param) @@ -146,8 +178,10 @@ on_proxy_port_created(GObject *initable, GAsyncResult *res, gpointer data) WpProxyPort *proxy_port = NULL; /* Get the proxy port */ - proxy_port = wp_proxy_port_new_finish(initable, res, NULL); - g_return_if_fail (proxy_port); + proxy_port = WP_PROXY_PORT (object_safe_new_finish (self, initable, res, + (WpObjectNewFinishFunc)wp_proxy_port_new_finish)); + if (!proxy_port) + return; /* Add the proxy port to the array */ g_return_if_fail (self->proxies_port); @@ -168,6 +202,10 @@ on_port_added(WpRemotePipewire *rp, guint id, guint parent_id, gconstpointer p, WpPipewireSimpleEndpoint *self = d; struct pw_port_proxy *port_proxy = NULL; + /* Don't do anything if we are aborting */ + if (self->init_abort) + return; + /* Only handle ports owned by this endpoint */ if (parent_id != self->global_id) return; @@ -222,8 +260,10 @@ on_proxy_node_created(GObject *initable, GAsyncResult *res, gpointer data) struct pw_node_proxy *node_proxy = NULL; /* Get the proxy node */ - self->proxy_node = wp_proxy_node_new_finish(initable, res, NULL); - g_return_if_fail (self->proxy_node); + self->proxy_node = WP_PROXY_NODE (object_safe_new_finish (self, initable, + res, (WpObjectNewFinishFunc)wp_proxy_node_new_finish)); + if (!self->proxy_node) + return; self->role = g_strdup (spa_dict_lookup ( wp_proxy_node_get_info (self->proxy_node)->props, "media.role")); @@ -319,6 +359,7 @@ wp_simple_endpoint_async_initable_init (gpointer iface, gpointer iface_data) static void simple_endpoint_init (WpPipewireSimpleEndpoint * self) { + self->init_abort = FALSE; } static void diff --git a/modules/module-pw-alsa-udev.c b/modules/module-pw-alsa-udev.c index ab29eaed..50d70904 100644 --- a/modules/module-pw-alsa-udev.c +++ b/modules/module-pw-alsa-udev.c @@ -29,11 +29,18 @@ on_endpoint_created(GObject *initable, GAsyncResult *res, gpointer d) struct impl *impl = d; WpEndpoint *endpoint = NULL; guint global_id = 0; + GError *error = NULL; /* Get the endpoint */ endpoint = wp_endpoint_new_finish(initable, res, NULL); - if (!endpoint) + g_return_if_fail (endpoint); + + /* Check for error */ + if (error) { + g_clear_object (&endpoint); + g_warning ("Failed to create alsa endpoint: %s", error->message); return; + } /* Get the endpoint global id */ g_object_get (endpoint, "global-id", &global_id, NULL); diff --git a/modules/module-pw-audio-client.c b/modules/module-pw-audio-client.c index 03fcdac6..9a923c31 100644 --- a/modules/module-pw-audio-client.c +++ b/modules/module-pw-audio-client.c @@ -27,11 +27,18 @@ on_endpoint_created(GObject *initable, GAsyncResult *res, gpointer d) struct module_data *data = d; WpEndpoint *endpoint = NULL; guint global_id = 0; + GError *error = NULL; /* Get the endpoint */ endpoint = wp_endpoint_new_finish(initable, res, NULL); - if (!endpoint) + g_return_if_fail (endpoint); + + /* Check for error */ + if (error) { + g_clear_object (&endpoint); + g_warning ("Failed to create client endpoint: %s", error->message); return; + } /* Get the endpoint global id */ g_object_get (endpoint, "global-id", &global_id, NULL); diff --git a/modules/module-pw-audio-softdsp-endpoint.c b/modules/module-pw-audio-softdsp-endpoint.c index 496e1300..91117135 100644 --- a/modules/module-pw-audio-softdsp-endpoint.c +++ b/modules/module-pw-audio-softdsp-endpoint.c @@ -38,6 +38,7 @@ struct _WpPwAudioSoftdspEndpoint /* The task to signal the endpoint is initialized */ GTask *init_task; + gboolean init_abort; /* The remote pipewire */ WpRemotePipewire *remote_pipewire; @@ -71,6 +72,37 @@ G_DEFINE_TYPE_WITH_CODE (WpPwAudioSoftdspEndpoint, endpoint, WP_TYPE_ENDPOINT, G_IMPLEMENT_INTERFACE (G_TYPE_ASYNC_INITABLE, wp_endpoint_async_initable_init)) +typedef GObject* (*WpObjectNewFinishFunc)(GObject *initable, GAsyncResult *res, + GError **error); + +static GObject * +object_safe_new_finish(WpPwAudioSoftdspEndpoint * self, GObject *initable, + GAsyncResult *res, WpObjectNewFinishFunc new_finish_func) +{ + GObject *object = NULL; + GError *error = NULL; + + /* Return NULL if we are already aborting */ + if (self->init_abort) + return NULL; + + /* Get the object */ + object = G_OBJECT (new_finish_func (initable, res, &error)); + g_return_val_if_fail (object, NULL); + + /* Check for error */ + if (error) { + g_clear_object (&object); + g_warning ("WpPwAudioSoftdspEndpoint:%p Aborting construction", self); + self->init_abort = TRUE; + g_task_return_error (self->init_task, error); + g_clear_object (&self->init_task); + return NULL; + } + + return object; +} + static gboolean endpoint_prepare_link (WpEndpoint * ep, guint32 stream_id, WpEndpointLink * link, GVariant ** properties, GError ** error) @@ -110,8 +142,10 @@ on_audio_dsp_stream_created(GObject *initable, GAsyncResult *res, gpointer data) g_autofree gchar *name = NULL; /* Get the stream */ - dsp = wp_pw_audio_dsp_new_finish(initable, res, NULL); - g_return_if_fail (dsp); + dsp = WP_PW_AUDIO_DSP (object_safe_new_finish (self, initable, res, + (WpObjectNewFinishFunc)wp_pw_audio_dsp_new_finish)); + if (!dsp) + return; /* Get the stream id */ g_object_get (dsp, "id", &stream_id, "name", &name, NULL); @@ -142,8 +176,10 @@ on_audio_dsp_converter_created(GObject *initable, GAsyncResult *res, int i; /* Get the proxy dsp converter */ - self->converter = wp_pw_audio_dsp_new_finish(initable, res, NULL); - g_return_if_fail (self->converter); + self->converter = WP_PW_AUDIO_DSP (object_safe_new_finish (self, initable, + res, (WpObjectNewFinishFunc)wp_pw_audio_dsp_new_finish)); + if (!self->converter) + return; /* Get the target and format */ target = wp_pw_audio_dsp_get_info (self->converter); @@ -177,8 +213,10 @@ on_proxy_node_created(GObject *initable, GAsyncResult *res, gpointer data) const struct spa_audio_info_raw *format = NULL; /* Get the proxy node */ - self->proxy_node = wp_proxy_node_new_finish(initable, res, NULL); - g_return_if_fail (self->proxy_node); + self->proxy_node = WP_PROXY_NODE (object_safe_new_finish (self, initable, + res, (WpObjectNewFinishFunc)wp_proxy_node_new_finish)); + if (!self->proxy_node) + return; /* Give a proper name to this endpoint based on ALSA properties */ props = wp_proxy_node_get_info (self->proxy_node)->props; @@ -208,8 +246,10 @@ on_proxy_port_created(GObject *initable, GAsyncResult *res, gpointer data) struct pw_node_proxy *node_proxy = NULL; /* Get the proxy port */ - self->proxy_port = wp_proxy_port_new_finish(initable, res, NULL); - g_return_if_fail (self->proxy_port); + self->proxy_port = WP_PROXY_PORT (object_safe_new_finish (self, initable, res, + (WpObjectNewFinishFunc)wp_proxy_port_new_finish)); + if (!self->proxy_port) + return; /* Create the proxy node async */ node_proxy = wp_remote_pipewire_proxy_bind (self->remote_pipewire, @@ -225,6 +265,10 @@ on_port_added(WpRemotePipewire *rp, guint id, guint parent_id, gconstpointer p, WpPwAudioSoftdspEndpoint *self = d; struct pw_port_proxy *port_proxy = NULL; + /* Don't do anything if we are aborting */ + if (self->init_abort) + return; + /* Check if it is a node port and handle it */ if (self->global_id != parent_id) return; @@ -407,6 +451,7 @@ wp_endpoint_async_initable_init (gpointer iface, gpointer iface_data) static void endpoint_init (WpPwAudioSoftdspEndpoint * self) { + self->init_abort = FALSE; self->dsps = g_ptr_array_new_with_free_func (g_object_unref); } diff --git a/modules/module-pw-audio-softdsp-endpoint/dsp.c b/modules/module-pw-audio-softdsp-endpoint/dsp.c index 7c2494f6..8faa8273 100644 --- a/modules/module-pw-audio-softdsp-endpoint/dsp.c +++ b/modules/module-pw-audio-softdsp-endpoint/dsp.c @@ -39,6 +39,7 @@ struct _WpPwAudioDsp /* The task to signal the audio dsp is initialized */ GTask *init_task; + gboolean init_abort; /* The remote pipewire */ WpRemotePipewire *remote_pipewire; @@ -75,6 +76,37 @@ G_DEFINE_TYPE_WITH_CODE (WpPwAudioDsp, wp_pw_audio_dsp, G_TYPE_OBJECT, G_IMPLEMENT_INTERFACE (G_TYPE_ASYNC_INITABLE, wp_pw_audio_dsp_async_initable_init)) +typedef GObject* (*WpObjectNewFinishFunc)(GObject *initable, GAsyncResult *res, + GError **error); + +static GObject * +object_safe_new_finish(WpPwAudioDsp * self, GObject *initable, + GAsyncResult *res, WpObjectNewFinishFunc new_finish_func) +{ + GObject *object = NULL; + GError *error = NULL; + + /* Return NULL if we are already aborting */ + if (self->init_abort) + return NULL; + + /* Get the object */ + object = G_OBJECT (new_finish_func (initable, res, &error)); + g_return_val_if_fail (object, NULL); + + /* Check for error */ + if (error) { + g_clear_object (&object); + g_warning ("WpPwAudioDsp:%p Aborting construction", self); + self->init_abort = TRUE; + g_task_return_error (self->init_task, error); + g_clear_object (&self->init_task); + return NULL; + } + + return object; +} + guint wp_pw_audio_dsp_id_encode (guint stream_id, guint control_id) { @@ -147,8 +179,10 @@ on_audio_dsp_port_created(GObject *initable, GAsyncResult *res, WpProxyPort *port_proxy = NULL; /* Get the proxy port */ - port_proxy = wp_proxy_port_new_finish(initable, res, NULL); - g_return_if_fail (port_proxy); + port_proxy = WP_PROXY_PORT (object_safe_new_finish (self, initable, res, + (WpObjectNewFinishFunc)wp_proxy_port_new_finish)); + if (!port_proxy) + return; /* Add the proxy port to the array */ g_return_if_fail (self->port_proxies); @@ -164,6 +198,10 @@ handled_ports_foreach_func (gpointer key, gpointer value, gpointer data) const guint id = GPOINTER_TO_INT (key); const guint parent_id = GPOINTER_TO_INT (value); + /* Don't do anything if we are aborting */ + if (self->init_abort) + return; + /* Get the dsp info */ g_return_if_fail (self->proxy); dsp_info = wp_proxy_node_get_info(self->proxy); @@ -224,7 +262,8 @@ on_proxy_link_created(GObject *initable, GAsyncResult *res, gpointer data) WpPwAudioDsp *self = data; /* Get the link */ - self->link_proxy = wp_proxy_link_new_finish(initable, res, NULL); + self->link_proxy = WP_PROXY_LINK (object_safe_new_finish (self, initable, + res, (WpObjectNewFinishFunc)wp_proxy_link_new_finish)); g_return_if_fail (self->link_proxy); } @@ -366,8 +405,10 @@ on_audio_dsp_proxy_created(GObject *initable, GAsyncResult *res, struct spa_pod *param; /* Get the audio dsp proxy */ - self->proxy = wp_proxy_node_new_finish(initable, res, NULL); - g_return_if_fail (self->proxy); + self->proxy = WP_PROXY_NODE (object_safe_new_finish (self, initable, + res, (WpObjectNewFinishFunc)wp_proxy_node_new_finish)); + if (!self->proxy) + return; /* Add a custom dsp listener */ pw_proxy = wp_proxy_get_pw_proxy(WP_PROXY(self->proxy)); @@ -575,6 +616,7 @@ wp_pw_audio_dsp_async_initable_init (gpointer iface, gpointer iface_data) static void wp_pw_audio_dsp_init (WpPwAudioDsp * self) { + self->init_abort = FALSE; } static void From efbefe9e0e5a5d27f31ae74224418a1a8abaa881 Mon Sep 17 00:00:00 2001 From: Julian Bouzas Date: Mon, 15 Jul 2019 11:00:11 -0400 Subject: [PATCH 4/8] simple-policy: fix bug when finding endpoints --- modules/module-simple-policy.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/modules/module-simple-policy.c b/modules/module-simple-policy.c index 120b90ef..8eeb3fda 100644 --- a/modules/module-simple-policy.c +++ b/modules/module-simple-policy.c @@ -471,7 +471,7 @@ simple_policy_find_endpoint (WpPolicy *policy, GVariant *props, } /* If not found, return the first endpoint */ - ep = (ptr_array->len > 1) ? + ep = (ptr_array->len > 0) ? g_object_ref (g_ptr_array_index (ptr_array, 0)) : NULL; select_stream: From 9f07ba229cc94ff33a1c2d8c163bb46f95c4a62f Mon Sep 17 00:00:00 2001 From: Julian Bouzas Date: Thu, 25 Jul 2019 12:26:09 +0300 Subject: [PATCH 5/8] dsp: removed unneeded format property and always use the default format --- modules/module-pipewire/simple-endpoint.c | 6 ++--- modules/module-pw-audio-softdsp-endpoint.c | 15 +++-------- .../module-pw-audio-softdsp-endpoint/dsp.c | 26 ++++++------------- .../module-pw-audio-softdsp-endpoint/dsp.h | 4 +-- 4 files changed, 16 insertions(+), 35 deletions(-) diff --git a/modules/module-pipewire/simple-endpoint.c b/modules/module-pipewire/simple-endpoint.c index cb8bc9bc..b1e2e072 100644 --- a/modules/module-pipewire/simple-endpoint.c +++ b/modules/module-pipewire/simple-endpoint.c @@ -230,13 +230,13 @@ emit_endpoint_ports(WpPipewireSimpleEndpoint *self) node_proxy = wp_proxy_get_pw_proxy(WP_PROXY(self->proxy_node)); g_return_if_fail (node_proxy); - /* TODO: Assume all clients have this format for now */ + /* The default format for audio clients */ format.format = SPA_AUDIO_FORMAT_F32P; format.flags = 1; format.rate = 48000; format.channels = 2; - format.position[0] = 0; - format.position[1] = 0; + format.position[0] = SPA_AUDIO_CHANNEL_FL; + format.position[1] = SPA_AUDIO_CHANNEL_FR; /* Build the param profile */ spa_pod_builder_init(&pod_builder, buf, sizeof(buf)); diff --git a/modules/module-pw-audio-softdsp-endpoint.c b/modules/module-pw-audio-softdsp-endpoint.c index 91117135..f7c74452 100644 --- a/modules/module-pw-audio-softdsp-endpoint.c +++ b/modules/module-pw-audio-softdsp-endpoint.c @@ -169,7 +169,6 @@ on_audio_dsp_converter_created(GObject *initable, GAsyncResult *res, WpPwAudioSoftdspEndpoint *self = data; g_autoptr (WpCore) core = wp_endpoint_get_core(WP_ENDPOINT(self)); const struct pw_node_info *target = NULL; - const struct spa_audio_info_raw *format = NULL; GVariantDict d; GVariantIter iter; const gchar *stream; @@ -184,14 +183,12 @@ on_audio_dsp_converter_created(GObject *initable, GAsyncResult *res, /* Get the target and format */ target = wp_pw_audio_dsp_get_info (self->converter); g_return_if_fail (target); - g_object_get (self->converter, "format", &format, NULL); - g_return_if_fail (format); /* Create the audio dsp streams */ g_variant_iter_init (&iter, self->streams); for (i = 0; g_variant_iter_next (&iter, "&s", &stream); i++) { - wp_pw_audio_dsp_new (WP_ENDPOINT(self), i, stream, self->direction, - FALSE, target, format, on_audio_dsp_stream_created, self); + wp_pw_audio_dsp_new (WP_ENDPOINT(self), i, stream, self->direction, FALSE, + target, on_audio_dsp_stream_created, self); /* Register the stream */ g_variant_dict_init (&d, NULL); @@ -210,7 +207,6 @@ on_proxy_node_created(GObject *initable, GAsyncResult *res, gpointer data) g_autofree gchar *name = NULL; const struct spa_dict *props; const struct pw_node_info *target = NULL; - const struct spa_audio_info_raw *format = NULL; /* Get the proxy node */ self->proxy_node = WP_PROXY_NODE (object_safe_new_finish (self, initable, @@ -230,13 +226,8 @@ on_proxy_node_created(GObject *initable, GAsyncResult *res, gpointer data) /* Create the converter proxy */ target = wp_proxy_node_get_info (self->proxy_node); g_return_if_fail (target); - format = wp_proxy_port_get_format (self->proxy_port); - g_return_if_fail (format); - /* TODO: For now we create convert as a stream because convert mode does not - * generate any ports, not sure why */ wp_pw_audio_dsp_new (WP_ENDPOINT(self), WP_STREAM_ID_NONE, "master", - self->direction, TRUE, target, format, on_audio_dsp_converter_created, - self); + self->direction, TRUE, target, on_audio_dsp_converter_created, self); } static void diff --git a/modules/module-pw-audio-softdsp-endpoint/dsp.c b/modules/module-pw-audio-softdsp-endpoint/dsp.c index 8faa8273..f3282a8b 100644 --- a/modules/module-pw-audio-softdsp-endpoint/dsp.c +++ b/modules/module-pw-audio-softdsp-endpoint/dsp.c @@ -24,7 +24,6 @@ enum { PROP_DIRECTION, PROP_CONVERT, PROP_TARGET, - PROP_FORMAT, }; enum { @@ -51,7 +50,6 @@ struct _WpPwAudioDsp enum pw_direction direction; gboolean convert; const struct pw_node_info *target; - const struct spa_audio_info_raw *format; /* All ports handled by the port added callback */ GHashTable *handled_ports; @@ -420,9 +418,13 @@ on_audio_dsp_proxy_created(GObject *initable, GAsyncResult *res, pw_node_proxy_enum_params (pw_proxy, 0, SPA_PARAM_Props, 0, -1, NULL); if (!self->convert) { - /* Get the port format */ - g_return_if_fail (self->format); - format = *self->format; + /* Use the default format */ + format.format = SPA_AUDIO_FORMAT_F32P; + format.flags = 1; + format.rate = 48000; + format.channels = 2; + format.position[0] = SPA_AUDIO_CHANNEL_FL; + format.position[1] = SPA_AUDIO_CHANNEL_FR; /* Emit the ports */ spa_pod_builder_init(&pod_builder, buf, sizeof(buf)); @@ -496,9 +498,6 @@ wp_pw_audio_dsp_set_property (GObject * object, guint property_id, case PROP_TARGET: self->target = g_value_get_pointer(value); break; - case PROP_FORMAT: - self->format = g_value_get_pointer(value); - break; default: G_OBJECT_WARN_INVALID_PROPERTY_ID (object, property_id, pspec); break; @@ -530,9 +529,6 @@ wp_pw_audio_dsp_get_property (GObject * object, guint property_id, case PROP_TARGET: g_value_set_pointer (value, (gpointer)self->target); break; - case PROP_FORMAT: - g_value_set_pointer (value, (gpointer)self->format); - break; default: G_OBJECT_WARN_INVALID_PROPERTY_ID (object, property_id, pspec); break; @@ -651,17 +647,12 @@ wp_pw_audio_dsp_class_init (WpPwAudioDspClass * klass) g_param_spec_pointer ("target", "target", "The target node info of the audio DSP", G_PARAM_READWRITE | G_PARAM_CONSTRUCT_ONLY | G_PARAM_STATIC_STRINGS)); - g_object_class_install_property (object_class, PROP_FORMAT, - g_param_spec_pointer ("format", "format", - "The format of the audio DSP ports", - G_PARAM_READWRITE | G_PARAM_CONSTRUCT_ONLY | G_PARAM_STATIC_STRINGS)); } void wp_pw_audio_dsp_new (WpEndpoint *endpoint, guint id, const char *name, enum pw_direction direction, gboolean convert, - const struct pw_node_info *target, const struct spa_audio_info_raw *format, - GAsyncReadyCallback callback, + const struct pw_node_info *target, GAsyncReadyCallback callback, gpointer user_data) { g_async_initable_new_async ( @@ -673,7 +664,6 @@ wp_pw_audio_dsp_new (WpEndpoint *endpoint, guint id, const char *name, "direction", direction, "convert", convert, "target", target, - "format", format, NULL); } diff --git a/modules/module-pw-audio-softdsp-endpoint/dsp.h b/modules/module-pw-audio-softdsp-endpoint/dsp.h index 5d7c6f44..f6bb7550 100644 --- a/modules/module-pw-audio-softdsp-endpoint/dsp.h +++ b/modules/module-pw-audio-softdsp-endpoint/dsp.h @@ -20,8 +20,8 @@ void wp_pw_audio_dsp_id_decode (guint id, guint *stream_id, guint *control_id); void wp_pw_audio_dsp_new (WpEndpoint *endpoint, guint id, const char *name, enum pw_direction direction, gboolean convert, - const struct pw_node_info *target, const struct spa_audio_info_raw *format, - GAsyncReadyCallback callback, gpointer user_data); + const struct pw_node_info *target, GAsyncReadyCallback callback, + gpointer user_data); WpPwAudioDsp * wp_pw_audio_dsp_new_finish (GObject *initable, GAsyncResult *res, GError **error); From 4b6ea0de6e6741aa5e3f9c98f651e765f2971666 Mon Sep 17 00:00:00 2001 From: George Kiagiadakis Date: Thu, 25 Jul 2019 12:27:03 +0300 Subject: [PATCH 6/8] modules: initialize some spa_pod_builders inline --- modules/module-pipewire/simple-endpoint.c | 3 +-- modules/module-pw-audio-softdsp-endpoint/dsp.c | 3 +-- 2 files changed, 2 insertions(+), 4 deletions(-) diff --git a/modules/module-pipewire/simple-endpoint.c b/modules/module-pipewire/simple-endpoint.c index b1e2e072..06f5833a 100644 --- a/modules/module-pipewire/simple-endpoint.c +++ b/modules/module-pipewire/simple-endpoint.c @@ -223,8 +223,8 @@ emit_endpoint_ports(WpPipewireSimpleEndpoint *self) struct pw_node_proxy* node_proxy = NULL; struct spa_audio_info_raw format = { 0, }; struct spa_pod *param; - struct spa_pod_builder pod_builder = { 0, }; char buf[1024]; + struct spa_pod_builder pod_builder = SPA_POD_BUILDER_INIT(buf, sizeof(buf)); /* Get the pipewire node proxy */ node_proxy = wp_proxy_get_pw_proxy(WP_PROXY(self->proxy_node)); @@ -239,7 +239,6 @@ emit_endpoint_ports(WpPipewireSimpleEndpoint *self) format.position[1] = SPA_AUDIO_CHANNEL_FR; /* Build the param profile */ - spa_pod_builder_init(&pod_builder, buf, sizeof(buf)); param = spa_format_audio_raw_build(&pod_builder, SPA_PARAM_Format, &format); param = spa_pod_builder_add_object(&pod_builder, SPA_TYPE_OBJECT_ParamProfile, SPA_PARAM_Profile, diff --git a/modules/module-pw-audio-softdsp-endpoint/dsp.c b/modules/module-pw-audio-softdsp-endpoint/dsp.c index f3282a8b..c90a11cd 100644 --- a/modules/module-pw-audio-softdsp-endpoint/dsp.c +++ b/modules/module-pw-audio-softdsp-endpoint/dsp.c @@ -399,7 +399,7 @@ on_audio_dsp_proxy_created(GObject *initable, GAsyncResult *res, struct pw_node_proxy *pw_proxy = NULL; struct spa_audio_info_raw format; uint8_t buf[1024]; - struct spa_pod_builder pod_builder = { 0, }; + struct spa_pod_builder pod_builder = SPA_POD_BUILDER_INIT(buf, sizeof(buf)); struct spa_pod *param; /* Get the audio dsp proxy */ @@ -427,7 +427,6 @@ on_audio_dsp_proxy_created(GObject *initable, GAsyncResult *res, format.position[1] = SPA_AUDIO_CHANNEL_FR; /* Emit the ports */ - spa_pod_builder_init(&pod_builder, buf, sizeof(buf)); param = spa_format_audio_raw_build(&pod_builder, SPA_PARAM_Format, &format); param = spa_pod_builder_add_object(&pod_builder, SPA_TYPE_OBJECT_ParamProfile, SPA_PARAM_Profile, From c27c020a6621930075af8aaaa99da71fa497014c Mon Sep 17 00:00:00 2001 From: Julian Bouzas Date: Thu, 18 Jul 2019 11:03:19 -0400 Subject: [PATCH 7/8] simple-endpoint-link: skip already output linked ports --- modules/module-pipewire/simple-endpoint-link.c | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/modules/module-pipewire/simple-endpoint-link.c b/modules/module-pipewire/simple-endpoint-link.c index 895d8063..a64b3b5f 100644 --- a/modules/module-pipewire/simple-endpoint-link.c +++ b/modules/module-pipewire/simple-endpoint-link.c @@ -229,9 +229,11 @@ simple_endpoint_link_create (WpEndpointLink * epl, GVariant * src_data, if (in_direction == PW_DIRECTION_OUTPUT) continue; - /* Skip the port if it is already linked */ + /* Skip the ports if they are already linked */ if (g_hash_table_contains (linked_ports, GUINT_TO_POINTER(in_id))) continue; + if (g_hash_table_contains (linked_ports, GUINT_TO_POINTER(out_id))) + continue; /* Create the properties */ props = pw_properties_new(NULL, NULL); @@ -248,8 +250,9 @@ simple_endpoint_link_create (WpEndpointLink * epl, GVariant * src_data, self); self->link_count++; - /* Insert the port id in the hash table to know it is linked */ + /* Insert the port ids in the hash tables to know they are linked */ g_hash_table_insert (linked_ports, GUINT_TO_POINTER(in_id), NULL); + g_hash_table_insert (linked_ports, GUINT_TO_POINTER(out_id), NULL); /* Clean up */ pw_properties_free(props); From baaccc92f7c42475285386ada922611c615f81a2 Mon Sep 17 00:00:00 2001 From: George Kiagiadakis Date: Thu, 25 Jul 2019 12:18:54 +0300 Subject: [PATCH 8/8] proxy: fix the naming of variables that point to the instance and private structures --- lib/wp/proxy.c | 35 ++++++++++++++++++----------------- 1 file changed, 18 insertions(+), 17 deletions(-) diff --git a/lib/wp/proxy.c b/lib/wp/proxy.c index b14608b4..e3d612a4 100644 --- a/lib/wp/proxy.c +++ b/lib/wp/proxy.c @@ -46,14 +46,15 @@ G_DEFINE_ABSTRACT_TYPE_WITH_CODE (WpProxy, wp_proxy, G_TYPE_OBJECT, static void proxy_event_destroy (void *data) { - WpProxyPrivate *self = wp_proxy_get_instance_private (WP_PROXY(data)); + WpProxy *self = WP_PROXY (data); + WpProxyPrivate *priv = wp_proxy_get_instance_private (self); /* Set the proxy to NULL */ - self->proxy = NULL; + priv->proxy = NULL; /* Call the destroy method */ - if (WP_PROXY_GET_CLASS (data)->destroy) - WP_PROXY_GET_CLASS (data)->destroy (data); + if (WP_PROXY_GET_CLASS (self)->destroy) + WP_PROXY_GET_CLASS (self)->destroy (self); } static void @@ -72,24 +73,24 @@ static const struct pw_proxy_events proxy_events = { static void wp_proxy_constructed (GObject * object) { - WpProxyPrivate *self = wp_proxy_get_instance_private (WP_PROXY(object)); + WpProxyPrivate *priv = wp_proxy_get_instance_private (WP_PROXY(object)); /* Add the event listener */ - pw_proxy_add_listener (self->proxy, &self->listener, &proxy_events, object); + pw_proxy_add_listener (priv->proxy, &priv->listener, &proxy_events, object); } static void wp_proxy_finalize (GObject * object) { - WpProxyPrivate *self = wp_proxy_get_instance_private (WP_PROXY(object)); + WpProxyPrivate *priv = wp_proxy_get_instance_private (WP_PROXY(object)); g_debug ("%s:%p destroyed (pw proxy %p)", G_OBJECT_TYPE_NAME (object), - object, self->proxy); + object, priv->proxy); /* Destroy the proxy */ - if (self->proxy) { - pw_proxy_destroy (self->proxy); - self->proxy = NULL; + if (priv->proxy) { + pw_proxy_destroy (priv->proxy); + priv->proxy = NULL; } G_OBJECT_CLASS (wp_proxy_parent_class)->finalize (object); @@ -99,14 +100,14 @@ static void wp_proxy_set_property (GObject * object, guint property_id, const GValue * value, GParamSpec * pspec) { - WpProxyPrivate *self = wp_proxy_get_instance_private (WP_PROXY(object)); + WpProxyPrivate *priv = wp_proxy_get_instance_private (WP_PROXY(object)); switch (property_id) { case PROP_GLOBAL_ID: - self->global_id = g_value_get_uint (value); + priv->global_id = g_value_get_uint (value); break; case PROP_PROXY: - self->proxy = g_value_get_pointer (value); + priv->proxy = g_value_get_pointer (value); break; default: G_OBJECT_WARN_INVALID_PROPERTY_ID (object, property_id, pspec); @@ -118,14 +119,14 @@ static void wp_proxy_get_property (GObject * object, guint property_id, GValue * value, GParamSpec * pspec) { - WpProxyPrivate *self = wp_proxy_get_instance_private (WP_PROXY(object)); + WpProxyPrivate *priv = wp_proxy_get_instance_private (WP_PROXY(object)); switch (property_id) { case PROP_GLOBAL_ID: - g_value_set_uint (value, self->global_id); + g_value_set_uint (value, priv->global_id); break; case PROP_PROXY: - g_value_set_pointer (value, self->proxy); + g_value_set_pointer (value, priv->proxy); break; default: G_OBJECT_WARN_INVALID_PROPERTY_ID (object, property_id, pspec);