Skip to content

Commit

Permalink
pipewire: Destroy registry object with remote
Browse files Browse the repository at this point in the history
This practically reintroduces commit fbe2653 which got reverted
in ff8a8de, which I only discovered after coming up with this
solution on its own. The main difference is that
`discover_node_factory_sync()` gets removed as it doesn't really
make much sense to keep around, as well small clean up improvements.

The commit message of fbe2653 (by msizanoen1
<[email protected]>)>
```
Currently, the PipeWire registry object is destroyed when
discover_node_factory_sync finishes its functionality. This causes
registry events to not be delivered to the global_removed_cb and
global_added_cb callbacks after discover_node_factory_sync, breaking
the functionality of the
org.freedesktop.portal.Camera.IsCameraPresent property in case of
camera device hot(un)plug.

This changes the discover_node_factory_sync function to store the
registry in the PipeWireRemote structure and destroy it in
pipewire_remote_destroy.
```

This holds true to this day and causes issues for the camera portal,
most importantly causing a race condition where cameras that were not
yet available during initialization would never get recognized.

Whatever crashes it caused at some point I can't reproduce - and
Gnome-Shell, which has a very similar implementation for its camera
monitor doesn't seem to suffer from crashes as well.
  • Loading branch information
rmader authored and GeorgesStavracas committed Apr 1, 2024
1 parent 956b514 commit 24d29bf
Show file tree
Hide file tree
Showing 2 changed files with 20 additions and 27 deletions.
46 changes: 19 additions & 27 deletions src/pipewire.c
Original file line number Diff line number Diff line change
Expand Up @@ -122,32 +122,6 @@ pipewire_remote_roundtrip (PipeWireRemote *remote)
FALSE);
}

static gboolean
discover_node_factory_sync (PipeWireRemote *remote,
GError **error)
{
struct pw_registry *registry;

registry = pw_core_get_registry (remote->core, PW_VERSION_REGISTRY, 0);
pw_registry_add_listener (registry,
&remote->registry_listener,
&registry_events,
remote);

pipewire_remote_roundtrip (remote);

pw_proxy_destroy((struct pw_proxy*)registry);

if (remote->node_factory_id == 0)
{
g_set_error (error, G_IO_ERROR, G_IO_ERROR_FAILED,
"No node factory discovered");
return FALSE;
}

return TRUE;
}

static void
core_event_error (void *user_data,
uint32_t id,
Expand Down Expand Up @@ -245,7 +219,13 @@ pipewire_remote_destroy (PipeWireRemote *remote)
pw_loop_destroy_source (loop, g_steal_pointer (&remote->roundtrip_timeout));
}

/* This check is a workaround for older PW versions */
if (remote->registry)
spa_hook_remove (&remote->registry_listener);
g_clear_pointer (&remote->registry, pw_proxy_destroy);
g_clear_pointer (&remote->globals, g_hash_table_destroy);
if (remote->core)
spa_hook_remove (&remote->core_listener);
g_clear_pointer (&remote->core, pw_core_disconnect);
g_clear_pointer (&remote->context, pw_context_destroy);
g_clear_pointer (&remote->loop, pw_main_loop_destroy);
Expand Down Expand Up @@ -346,8 +326,20 @@ pipewire_remote_new_sync (struct pw_properties *pipewire_properties,
&core_events,
remote);

if (!discover_node_factory_sync (remote, error))
remote->registry = (struct pw_proxy*) pw_core_get_registry (remote->core,
PW_VERSION_REGISTRY,
0);
pw_registry_add_listener (remote->registry,
&remote->registry_listener,
&registry_events,
remote);

pipewire_remote_roundtrip (remote);

if (remote->node_factory_id == 0)
{
g_set_error (error, G_IO_ERROR, G_IO_ERROR_FAILED,
"No node factory discovered");
pipewire_remote_destroy (remote);
return NULL;
}
Expand Down
1 change: 1 addition & 0 deletions src/pipewire.h
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,7 @@ struct _PipeWireRemote

int sync_seq;

struct pw_proxy *registry;
struct spa_hook registry_listener;

GHashTable *globals;
Expand Down

0 comments on commit 24d29bf

Please sign in to comment.