From 7966a8d45688ede44215d207518227fdbc665bf4 Mon Sep 17 00:00:00 2001
From: George Kiagiadakis <george.kiagiadakis@collabora.com>
Date: Tue, 5 May 2020 16:31:53 +0300
Subject: [PATCH] si-standard-link: fix endpoint linking bugs and add some
debug statements
---
lib/wp/endpoint-link.c | 2 +-
lib/wp/endpoint.c | 92 ++++++++++++++++++------------
lib/wp/session-item.c | 19 +++++--
modules/module-si-standard-link.c | 95 ++++++++-----------------------
4 files changed, 94 insertions(+), 114 deletions(-)
diff --git a/lib/wp/endpoint-link.c b/lib/wp/endpoint-link.c
index 2bba0711..9f77ccdb 100644
--- a/lib/wp/endpoint-link.c
+++ b/lib/wp/endpoint-link.c
@@ -443,7 +443,7 @@ on_si_link_flags_changed (WpSiLink * item, WpSiFlags flags,
{
enum pw_endpoint_link_state old_state = self->info.state;
- if (flags & WP_SI_FLAG_EXPORT_ERROR)
+ if (flags & WP_SI_FLAG_ACTIVATE_ERROR)
self->info.state = PW_ENDPOINT_LINK_STATE_ERROR;
else if (flags & WP_SI_FLAG_ACTIVE)
self->info.state = PW_ENDPOINT_LINK_STATE_ACTIVE;
diff --git a/lib/wp/endpoint.c b/lib/wp/endpoint.c
index 3f1bb234..3d6d0a93 100644
--- a/lib/wp/endpoint.c
+++ b/lib/wp/endpoint.c
@@ -677,10 +677,14 @@ impl_set_param (void *object, uint32_t id, uint32_t flags,
}
static void
-destroy_deconfigured_link (WpSessionItem * link, WpSiFlags flags, gpointer data)
+on_stream_flags_changed (WpSessionItem * stream, WpSiFlags flags,
+ WpSessionItem * link)
{
- if (!(flags & WP_SI_FLAG_CONFIGURED))
+ /* stream was deactivated; destroy the associated link */
+ if (!(flags & WP_SI_FLAG_ACTIVE)) {
+ wp_session_item_reset (link);
g_object_unref (link);
+ }
}
static void
@@ -692,11 +696,7 @@ on_si_link_exported (WpSessionItem * link, GAsyncResult * res, gpointer data)
if (!wp_session_item_export_finish (link, res, &error)) {
wp_warning_object (self, "failed to export link: %s", error->message);
g_object_unref (link);
- return;
}
-
- g_signal_connect (link, "flags-changed",
- G_CALLBACK (destroy_deconfigured_link), NULL);
}
static int
@@ -708,11 +708,14 @@ impl_create_link (void *object, const struct spa_dict *props)
WpSiStream *self_si_stream = NULL;
g_autoptr (WpSiStream) peer_si_stream = NULL;
g_autoptr (WpSession) session = NULL;
+ g_autoptr (WpEndpointStream) self_stream_proxy = NULL;
+ g_autoptr (WpEndpoint) peer_ep_proxy = NULL;
+ g_autoptr (WpEndpointStream) peer_stream_proxy = NULL;
/* find the session */
session = wp_session_item_get_associated_proxy (
WP_SESSION_ITEM (self->item), WP_TYPE_SESSION);
- g_return_val_if_fail (!session, -ENAVAIL);
+ g_return_val_if_fail (session, -ENAVAIL);
if (self->info.direction == PW_DIRECTION_OUTPUT) {
self_ep = spa_dict_lookup (props, PW_KEY_ENDPOINT_LINK_OUTPUT_ENDPOINT);
@@ -726,6 +729,9 @@ impl_create_link (void *object, const struct spa_dict *props)
peer_stream = spa_dict_lookup (props, PW_KEY_ENDPOINT_LINK_OUTPUT_STREAM);
}
+ wp_debug_object (self, "requested link between %s:%s [self] & %s:%s [peer]",
+ self_ep, self_stream, peer_ep, peer_stream);
+
/* verify arguments */
if (!peer_ep) {
wp_warning_object (self,
@@ -770,42 +776,49 @@ impl_create_link (void *object, const struct spa_dict *props)
return -EINVAL;
}
- /* find the peer stream */
- {
- g_autoptr (WpEndpoint) peer_ep_proxy = NULL;
- g_autoptr (WpEndpointStream) peer_stream_proxy = NULL;
-
- peer_ep_proxy = wp_session_lookup_endpoint (session,
- WP_CONSTRAINT_TYPE_G_PROPERTY, "bound-id", "=u", peer_ep_id, NULL);
- if (!peer_ep_proxy) {
- wp_warning_object (self, "endpoint %d not found in session", peer_ep_id);
- return -EINVAL;
- }
+ self_stream_proxy = wp_session_item_get_associated_proxy (
+ WP_SESSION_ITEM (self_si_stream), WP_TYPE_ENDPOINT_STREAM);
- if (peer_stream_id != SPA_ID_INVALID) {
- peer_stream_proxy = wp_endpoint_lookup_stream (peer_ep_proxy,
- WP_CONSTRAINT_TYPE_G_PROPERTY, "bound-id", "=u", peer_stream_id, NULL);
- } else {
- peer_stream_proxy = wp_endpoint_lookup_stream (peer_ep_proxy, NULL);
- }
+ /* find the peer stream */
+ peer_ep_proxy = wp_session_lookup_endpoint (session,
+ WP_CONSTRAINT_TYPE_G_PROPERTY, "bound-id", "=u", peer_ep_id, NULL);
+ if (!peer_ep_proxy) {
+ wp_warning_object (self, "endpoint %d not found in session", peer_ep_id);
+ return -EINVAL;
+ }
- if (!peer_stream_proxy) {
- wp_warning_object (self, "stream %d not found in %d", peer_stream_id,
- peer_ep_id);
- return -EINVAL;
- }
+ if (peer_stream_id != SPA_ID_INVALID) {
+ peer_stream_proxy = wp_endpoint_lookup_stream (peer_ep_proxy,
+ WP_CONSTRAINT_TYPE_G_PROPERTY, "bound-id", "=u", peer_stream_id, NULL);
+ } else {
+ peer_stream_proxy = wp_endpoint_lookup_stream (peer_ep_proxy, NULL);
+ }
- if (!WP_IS_IMPL_ENDPOINT_LINK (peer_stream_proxy)) {
- /* TODO - if the stream is not implemented by our session manager,
- we can still make things work by calling the peer endpoint's
- create_link() and negotiating ports, while creating a dummy
- WpSiEndpoint / WpSiStream on our end to satisfy the API */
- return -ENAVAIL;
- }
+ if (!peer_stream_proxy) {
+ wp_warning_object (self, "stream %d not found in %d", peer_stream_id,
+ peer_ep_id);
+ return -EINVAL;
+ }
- g_object_get (peer_stream_proxy, "item", &peer_si_stream, NULL);
+ if (!WP_IS_IMPL_ENDPOINT_STREAM (peer_stream_proxy)) {
+ /* TODO - if the stream is not implemented by our session manager,
+ we can still make things work by calling the peer endpoint's
+ create_link() and negotiating ports, while creating a dummy
+ WpSiEndpoint / WpSiStream on our end to satisfy the API */
+ return -ENAVAIL;
}
+ g_object_get (peer_stream_proxy, "item", &peer_si_stream, NULL);
+
+ wp_info_object (self, "creating endpoint link between "
+ "%s|%s " WP_OBJECT_FORMAT ", %s|%s " WP_OBJECT_FORMAT,
+ wp_endpoint_get_name (WP_ENDPOINT (self)),
+ wp_endpoint_stream_get_name (self_stream_proxy),
+ WP_OBJECT_ARGS (self_si_stream),
+ wp_endpoint_get_name (peer_ep_proxy),
+ wp_endpoint_stream_get_name (peer_stream_proxy),
+ WP_OBJECT_ARGS (peer_si_stream));
+
/* create the link */
{
g_autoptr (WpSessionItem) link = NULL;
@@ -838,6 +851,11 @@ impl_create_link (void *object, const struct spa_dict *props)
return -ENAVAIL;
}
+ g_signal_connect_object (self_si_stream, "flags-changed",
+ G_CALLBACK (on_stream_flags_changed), link, 0);
+ g_signal_connect_object (peer_si_stream, "flags-changed",
+ G_CALLBACK (on_stream_flags_changed), link, 0);
+
wp_session_item_export (link, session,
(GAsyncReadyCallback) on_si_link_exported, self);
link = NULL;
diff --git a/lib/wp/session-item.c b/lib/wp/session-item.c
index de3459e1..906ba69e 100644
--- a/lib/wp/session-item.c
+++ b/lib/wp/session-item.c
@@ -144,6 +144,7 @@ wp_session_item_default_get_associated_proxy (WpSessionItem * self,
GType proxy_type)
{
WpSessionItemPrivate *priv;
+ gpointer ret = NULL;
if (WP_IS_SI_STREAM (self)) {
g_autoptr (WpSiEndpoint) ep =
@@ -154,23 +155,26 @@ wp_session_item_default_get_associated_proxy (WpSessionItem * self,
}
if (proxy_type == WP_TYPE_SESSION) {
- return g_weak_ref_get (&priv->session);
+ ret = g_weak_ref_get (&priv->session);
}
else if (proxy_type == WP_TYPE_ENDPOINT) {
if (priv->impl_proxy && WP_IS_ENDPOINT (priv->impl_proxy))
- return g_object_ref (priv->impl_proxy);
+ ret = g_object_ref (priv->impl_proxy);
}
else if (proxy_type == WP_TYPE_ENDPOINT_LINK) {
if (priv->impl_proxy && WP_IS_ENDPOINT_LINK (priv->impl_proxy))
- return g_object_ref (priv->impl_proxy);
+ ret = g_object_ref (priv->impl_proxy);
}
else if (proxy_type == WP_TYPE_ENDPOINT_STREAM) {
gpointer impl_stream = priv->impl_streams ?
g_hash_table_lookup (priv->impl_streams, self) : NULL;
- return impl_stream ? g_object_ref (impl_stream) : NULL;
+ ret = impl_stream ? g_object_ref (impl_stream) : NULL;
}
- return NULL;
+ wp_trace_object (self, "associated %s: " WP_OBJECT_FORMAT,
+ g_type_name (proxy_type), WP_OBJECT_ARGS (ret));
+
+ return ret;
}
static guint
@@ -622,6 +626,8 @@ wp_session_item_activate (WpSessionItem * self,
g_signal_connect (transition, "notify::completed",
(GCallback) on_activate_transition_completed, self);
+ wp_debug_object (self, "activating item");
+
priv->flags &= ~WP_SI_FLAG_ACTIVATE_ERROR;
priv->flags |= WP_SI_FLAG_ACTIVATING;
g_signal_emit (self, signals[SIGNAL_FLAGS_CHANGED], 0, priv->flags);
@@ -750,6 +756,9 @@ wp_session_item_export (WpSessionItem * self, WpSession * session,
g_signal_connect (transition, "notify::completed",
(GCallback) on_export_transition_completed, self);
+ wp_debug_object (self, "exporting item on session " WP_OBJECT_FORMAT,
+ WP_OBJECT_ARGS (session));
+
priv->flags &= ~WP_SI_FLAG_EXPORT_ERROR;
priv->flags |= WP_SI_FLAG_EXPORTING;
g_signal_emit (self, signals[SIGNAL_FLAGS_CHANGED], 0, priv->flags);
diff --git a/modules/module-si-standard-link.c b/modules/module-si-standard-link.c
index 3168ef3d..e50d0afd 100644
--- a/modules/module-si-standard-link.c
+++ b/modules/module-si-standard-link.c
@@ -33,37 +33,6 @@ G_DECLARE_FINAL_TYPE (WpSiStandardLink, si_standard_link, WP, SI_STANDARD_LINK,
G_DEFINE_TYPE_WITH_CODE (WpSiStandardLink, si_standard_link, WP_TYPE_SESSION_ITEM,
G_IMPLEMENT_INTERFACE (WP_TYPE_SI_LINK, si_standard_link_link_init))
-static void
-on_stream_destroyed (gpointer data, GObject * stream)
-{
- WpSiStandardLink *self = WP_SI_STANDARD_LINK (data);
-
- if ((gpointer) self->out_stream == (gpointer) stream)
- self->out_stream = NULL;
- else if ((gpointer) self->in_stream == (gpointer) stream)
- self->in_stream = NULL;
-
- wp_session_item_reset (WP_SESSION_ITEM (self));
-}
-
-static void
-on_stream_flags_changed (WpSessionItem * stream, WpSiFlags flags,
- WpSiStandardLink *self)
-{
- /* stream was deactivated; treat it as destroyed and reset */
- if (!(flags & WP_SI_FLAG_ACTIVE))
- wp_session_item_reset (WP_SESSION_ITEM (self));
-}
-
-static inline void
-disconnect_stream (WpSiStandardLink *self, WpSiStream * stream)
-{
- if (stream) {
- g_signal_handlers_disconnect_by_data (stream, self);
- g_object_weak_unref (G_OBJECT (stream), on_stream_destroyed, self);
- }
-}
-
static void
si_standard_link_init (WpSiStandardLink * self)
{
@@ -76,8 +45,6 @@ si_standard_link_reset (WpSessionItem * item)
WP_SESSION_ITEM_CLASS (si_standard_link_parent_class)->reset (item);
- disconnect_stream (self, self->out_stream);
- disconnect_stream (self, self->in_stream);
self->out_stream = NULL;
self->in_stream = NULL;
@@ -124,19 +91,9 @@ si_standard_link_configure (WpSessionItem * item, GVariant * args)
!(wp_session_item_get_flags (in_stream) & WP_SI_FLAG_ACTIVE))
return FALSE;
- disconnect_stream (self, self->out_stream);
- disconnect_stream (self, self->in_stream);
-
self->out_stream = WP_SI_STREAM (out_stream);
self->in_stream = WP_SI_STREAM (in_stream);
- g_signal_connect_object (self->out_stream, "flags-changed",
- G_CALLBACK (on_stream_flags_changed), self, 0);
- g_signal_connect_object (self->in_stream, "flags-changed",
- G_CALLBACK (on_stream_flags_changed), self, 0);
- g_object_weak_ref (G_OBJECT (self->out_stream), on_stream_destroyed, self);
- g_object_weak_ref (G_OBJECT (self->in_stream), on_stream_destroyed, self);
-
wp_session_item_set_flag (item, WP_SI_FLAG_CONFIGURED);
return TRUE;
@@ -217,12 +174,12 @@ find_core (WpSiStandardLink * self)
}
static gboolean
-create_links (WpSiStandardLink * self, GVariant * out_ports, GVariant * in_ports)
+create_links (WpSiStandardLink * self, WpTransition * transition,
+ GVariant * out_ports, GVariant * in_ports)
{
g_autoptr (GPtrArray) in_ports_arr = NULL;
g_autoptr (WpCore) core = NULL;
- WpLink *link;
- GVariantIter *iter;
+ GVariantIter *iter = NULL;
GVariant *child;
guint32 out_node_id, in_node_id;
guint32 out_port_id, in_port_id;
@@ -235,9 +192,9 @@ create_links (WpSiStandardLink * self, GVariant * out_ports, GVariant * in_ports
uint32 port_id;
uint32 channel; // enum spa_audio_channel
*/
- if (!g_variant_is_of_type (out_ports, G_VARIANT_TYPE("a(uuu)")))
+ if (!out_ports || !g_variant_is_of_type (out_ports, G_VARIANT_TYPE("a(uuu)")))
return FALSE;
- if (!g_variant_is_of_type (in_ports, G_VARIANT_TYPE("a(uuu)")))
+ if (!in_ports || !g_variant_is_of_type (in_ports, G_VARIANT_TYPE("a(uuu)")))
return FALSE;
core = find_core (self);
@@ -253,8 +210,9 @@ create_links (WpSiStandardLink * self, GVariant * out_ports, GVariant * in_ports
g_ptr_array_set_size (in_ports_arr, i);
g_variant_get (in_ports, "a(uuu)", &iter);
- while ((child = g_variant_iter_next_value (iter)))
- g_ptr_array_insert (in_ports_arr, --i, child);
+ while ((child = g_variant_iter_next_value (iter))) {
+ g_ptr_array_index (in_ports_arr, --i) = child;
+ }
g_variant_iter_free (iter);
/* now loop over the out ports and figure out where they should be linked */
@@ -280,6 +238,7 @@ create_links (WpSiStandardLink * self, GVariant * out_ports, GVariant * in_ports
in_channel == SPA_AUDIO_CHANNEL_UNKNOWN)
{
g_autoptr (WpProperties) props = NULL;
+ WpLink *link;
/* Create the properties */
props = wp_properties_new_empty ();
@@ -288,7 +247,7 @@ create_links (WpSiStandardLink * self, GVariant * out_ports, GVariant * in_ports
wp_properties_setf (props, PW_KEY_LINK_INPUT_NODE, "%u", in_node_id);
wp_properties_setf (props, PW_KEY_LINK_INPUT_PORT, "%u", in_port_id);
- g_debug ("Create pw link: %u:%u (%s) -> %u:%u (%s)",
+ wp_debug_object (self, "create pw link: %u:%u (%s) -> %u:%u (%s)",
out_node_id, out_port_id,
spa_debug_type_find_name (spa_type_audio_channel, out_channel),
in_node_id, in_port_id,
@@ -302,7 +261,7 @@ create_links (WpSiStandardLink * self, GVariant * out_ports, GVariant * in_ports
/* augment to ensure it is created without errors */
self->n_async_ops_wait++;
wp_proxy_augment (WP_PROXY (link), WP_PROXY_FEATURES_STANDARD, NULL,
- (GAsyncReadyCallback) on_link_augmented, self);
+ (GAsyncReadyCallback) on_link_augmented, transition);
/* continue to link all input ports, if requested */
if (link_all)
@@ -367,7 +326,7 @@ si_standard_link_activate_execute_step (WpSessionItem * item,
in_ports = wp_si_port_info_get_ports (WP_SI_PORT_INFO (self->in_stream),
NULL);
- if (!create_links (self, out_ports, in_ports)) {
+ if (!create_links (self, transition, out_ports, in_ports)) {
wp_transition_return_error (transition, g_error_new (WP_DOMAIN_LIBRARY,
WP_LIBRARY_ERROR_INVARIANT,
"Bad port info returned from one of the streams"));
@@ -387,18 +346,19 @@ si_standard_link_activate_rollback (WpSessionItem * item)
g_autoptr (WpSiEndpoint) in_endpoint = NULL;
WpSiStreamAcquisition *out_acquisition, *in_acquisition;
- out_endpoint = wp_si_stream_get_parent_endpoint (self->out_stream);
- in_endpoint = wp_si_stream_get_parent_endpoint (self->in_stream);
- out_acquisition = wp_si_endpoint_get_stream_acquisition (out_endpoint);
- in_acquisition = wp_si_endpoint_get_stream_acquisition (in_endpoint);
-
- if (out_acquisition) {
- wp_si_stream_acquisition_release (out_acquisition, WP_SI_LINK (self),
- self->out_stream);
+ if (self->out_stream) {
+ out_endpoint = wp_si_stream_get_parent_endpoint (self->out_stream);
+ out_acquisition = wp_si_endpoint_get_stream_acquisition (out_endpoint);
+ if (out_acquisition)
+ wp_si_stream_acquisition_release (out_acquisition, WP_SI_LINK (self),
+ self->out_stream);
}
- if (in_acquisition) {
- wp_si_stream_acquisition_release (in_acquisition, WP_SI_LINK (self),
- self->in_stream);
+ if (self->in_stream) {
+ in_endpoint = wp_si_stream_get_parent_endpoint (self->in_stream);
+ in_acquisition = wp_si_endpoint_get_stream_acquisition (in_endpoint);
+ if (in_acquisition)
+ wp_si_stream_acquisition_release (in_acquisition, WP_SI_LINK (self),
+ self->in_stream);
}
g_clear_pointer (&self->node_links, g_ptr_array_unref);
@@ -425,12 +385,6 @@ si_standard_link_get_registration_info (WpSiLink * item)
return g_variant_builder_end (&b);
}
-static WpProperties *
-si_standard_link_get_properties (WpSiLink * item)
-{
- return wp_properties_new_empty ();
-}
-
static WpSiStream *
si_standard_link_get_out_stream (WpSiLink * item)
{
@@ -449,7 +403,6 @@ static void
si_standard_link_link_init (WpSiLinkInterface * iface)
{
iface->get_registration_info = si_standard_link_get_registration_info;
- iface->get_properties = si_standard_link_get_properties;
iface->get_out_stream = si_standard_link_get_out_stream;
iface->get_in_stream = si_standard_link_get_in_stream;
}
--
GitLab