summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorGeorge Kiagiadakis <george.kiagiadakis@collabora.com>2019-12-24 11:36:06 +0200
committerGeorge Kiagiadakis <george.kiagiadakis@collabora.com>2019-12-24 19:54:36 +0200
commit242d8a1dcd3f15d5d7c173f45cdbf820d7b173d6 (patch)
treeefac0b11459c04d4ba850771dfb33e5f0edf98eb
parentf2358d170b1ce4a46a136b02f3aa708c3221ba2f (diff)
audiomixer: rework event handling to avoid race conditionsicefish_8.99.5icefish/8.99.58.99.5
Previously there was a race condition that would happen when audiomixer was the first client to connect to pipewire. It would get a session first and then a default endpoint id from the session, but at the time the default endpoint id would be known, the WpProxyEndpoints would still not be ready. This would cause the populate_controls() algorithm to end up with no controls and the binding would therefore report that no controls were exposed. Now we also handle objects-changed from the endpoints object manager. This is an additional trigger, so that when we end up in the situation described above, the objects-changed from the endpoints om will trigger the controls to be exposed eventually. In order to avoid signalling control changes all the time, there are now some checks to change controls only under certain conditions. Change-Id: Ied705592f889a0262465ed5efa711233a66d579b Signed-off-by: George Kiagiadakis <george.kiagiadakis@collabora.com>
-rw-r--r--binding/audiomixer.c163
1 files changed, 114 insertions, 49 deletions
diff --git a/binding/audiomixer.c b/binding/audiomixer.c
index 6ef3210..aceaefe 100644
--- a/binding/audiomixer.c
+++ b/binding/audiomixer.c
@@ -19,11 +19,12 @@ struct audiomixer
GCond cond;
WpRemoteState state;
- WpObjectManager *endpoints_om;
GPtrArray *mixer_controls;
+ GWeakRef session;
- /* just pointer, no ref */
- WpSession *session;
+ /* ref held in the thread function */
+ WpObjectManager *sessions_om;
+ WpObjectManager *endpoints_om;
const struct audiomixer_events *events;
void *events_data;
@@ -91,41 +92,95 @@ control_changed (WpEndpoint * ep, guint32 control_id, struct audiomixer * self)
/* called with self->lock locked */
static void
-populate_controls (struct audiomixer * self, WpSession * session)
+check_and_populate_controls (struct audiomixer * self)
{
- guint32 def_id = wp_session_get_default_endpoint (session,
- WP_DEFAULT_ENDPOINT_TYPE_AUDIO_SINK);
- g_autoptr (GPtrArray) arr = wp_object_manager_get_objects (
- self->endpoints_om, 0);
-
- for (guint i = 0; i < arr->len; i++) {
- WpEndpoint *ep = g_ptr_array_index (arr, i);
- guint32 id = wp_proxy_get_global_id (WP_PROXY (ep));
+ g_autoptr (WpSession) session = NULL;
+ g_autoptr (WpEndpoint) def_ep = NULL;
+ guint32 def_id = 0;
+ struct mixer_control_impl *master_ctl = NULL;
+
+ /* find the session */
+ session = g_weak_ref_get (&self->session);
+
+ /* find the default audio sink endpoint */
+ if (session) {
+ g_autoptr (GPtrArray) arr =
+ wp_object_manager_get_objects (self->endpoints_om, 0);
+ def_id = wp_session_get_default_endpoint (session,
+ WP_DEFAULT_ENDPOINT_TYPE_AUDIO_SINK);
+
+ for (guint i = 0; i < arr->len; i++) {
+ WpEndpoint *ep = g_ptr_array_index (arr, i);
+ guint32 id = wp_proxy_get_global_id (WP_PROXY (ep));
+ if (id != def_id)
+ continue;
+ def_ep = g_object_ref (ep);
+ }
+ }
+
+ /* find the audio sink endpoint that was the default before */
+ for (guint i = 0; i < self->mixer_controls->len; i++) {
+ struct mixer_control_impl *ctl = (struct mixer_control_impl *)
+ g_ptr_array_index (self->mixer_controls, i);
+ if (ctl->is_master) {
+ master_ctl = ctl;
+ break;
+ }
+ }
+
+ g_debug ("check_and_populate: session:%p, def_ep:%p, def_id:%u, "
+ "master_ctl:%p, master_id:%u", session, def_ep, def_id,
+ master_ctl, master_ctl ? master_ctl->id : 0);
+
+ /* case 1: there is a default endpoint but it doesn't match
+ what we currently expose -> clear and expose the new one */
+ if (def_ep && (!master_ctl || master_ctl->id != def_id)) {
struct mixer_control_impl *mixctl = NULL;
gfloat volume;
gboolean mute;
- if (id != def_id)
- continue;
+ /* clear previous */
+ if (master_ctl)
+ g_signal_handlers_disconnect_by_data (master_ctl->endpoint, self);
+ g_ptr_array_set_size (self->mixer_controls, 0);
- wp_endpoint_get_control_float (ep, WP_ENDPOINT_CONTROL_VOLUME, &volume);
- wp_endpoint_get_control_boolean (ep, WP_ENDPOINT_CONTROL_MUTE, &mute);
+ /* get current master values */
+ wp_endpoint_get_control_float (def_ep, WP_ENDPOINT_CONTROL_VOLUME, &volume);
+ wp_endpoint_get_control_boolean (def_ep, WP_ENDPOINT_CONTROL_MUTE, &mute);
+ /* create the Master control */
mixctl = g_new0 (struct mixer_control_impl, 1);
strncpy (mixctl->pub.name, "Master", sizeof (mixctl->pub.name));
mixctl->pub.volume = volume;
mixctl->pub.mute = mute;
mixctl->is_master = TRUE;
- mixctl->id = id;
- mixctl->endpoint = g_object_ref (ep);
+ mixctl->id = def_id;
+ mixctl->endpoint = g_object_ref (def_ep);
g_ptr_array_add (self->mixer_controls, mixctl);
- g_signal_connect (ep, "control-changed", (GCallback) control_changed,
+ /* track changes */
+ g_signal_connect (def_ep, "control-changed", (GCallback) control_changed,
self);
/* wake up audiomixer_ensure_controls() */
g_cond_broadcast (&self->cond);
- break;
+
+ g_debug ("controls changed");
+
+ /* notify subscribers */
+ if (self->events && self->events->controls_changed)
+ self->events->controls_changed (self->events_data);
+ }
+ /* case 2: there is no default endpoint but something is exposed -> clear */
+ else if (!def_ep && master_ctl) {
+ g_signal_handlers_disconnect_by_data (master_ctl->endpoint, self);
+ g_ptr_array_set_size (self->mixer_controls, 0);
+
+ g_debug ("controls cleared");
+
+ /* notify subscribers */
+ if (self->events && self->events->controls_changed)
+ self->events->controls_changed (self->events_data);
}
}
@@ -133,44 +188,49 @@ static void
default_endpoint_changed (WpSession * session, WpDefaultEndpointType type,
guint32 new_id, struct audiomixer * self)
{
- if (session != self->session || type != WP_DEFAULT_ENDPOINT_TYPE_AUDIO_SINK)
+ if (type != WP_DEFAULT_ENDPOINT_TYPE_AUDIO_SINK)
return;
g_autoptr (GMutexLocker) locker = g_mutex_locker_new (&self->lock);
-
- g_ptr_array_set_size (self->mixer_controls, 0);
- populate_controls (self, session);
-
- if (self->events && self->events->controls_changed)
- self->events->controls_changed (self->events_data);
+ check_and_populate_controls (self);
}
static void
-sessions_changed (WpObjectManager * om, struct audiomixer * self)
+sessions_changed (WpObjectManager * sessions_om, struct audiomixer * self)
{
g_autoptr (GMutexLocker) locker = g_mutex_locker_new (&self->lock);
+ g_autoptr (WpSession) old_session = NULL;
g_autoptr (WpSession) session = NULL;
g_autoptr (GPtrArray) arr = NULL;
- g_ptr_array_set_size (self->mixer_controls, 0);
+ old_session = g_weak_ref_get (&self->session);
/* normally there is only 1 session */
- arr = wp_object_manager_get_objects (om, 0);
+ arr = wp_object_manager_get_objects (sessions_om, 0);
if (arr->len > 0)
session = WP_SESSION (g_object_ref (g_ptr_array_index (arr, 0)));
- if (session)
- populate_controls (self, session);
+ g_debug ("sessions changed, count:%d, session:%p, old_session:%p",
+ arr->len, session, old_session);
- /* just to be safe in case there are more sessions around */
- if (session != self->session) {
- g_signal_connect (session, "default-endpoint-changed",
- (GCallback) default_endpoint_changed, self);
- self->session = session;
+ if (session != old_session) {
+ if (old_session)
+ g_signal_handlers_disconnect_by_data (old_session, self);
+ if (session)
+ g_signal_connect (session, "default-endpoint-changed",
+ (GCallback) default_endpoint_changed, self);
+
+ g_weak_ref_set (&self->session, session);
}
- if (self->events && self->events->controls_changed)
- self->events->controls_changed (self->events_data);
+ check_and_populate_controls (self);
+}
+
+static void
+endpoints_changed (WpObjectManager * endpoints_om, struct audiomixer * self)
+{
+ g_autoptr (GMutexLocker) locker = g_mutex_locker_new (&self->lock);
+ check_and_populate_controls (self);
}
static void
@@ -182,10 +242,18 @@ remote_state_changed (WpCore *core, WpRemoteState state,
g_cond_broadcast (&self->cond);
}
+static gboolean
+connect_in_idle (struct audiomixer * self)
+{
+ wp_core_connect (self->core);
+ return G_SOURCE_REMOVE;
+}
+
static void *
audiomixer_thread (struct audiomixer * self)
{
- g_autoptr (WpObjectManager) sessions_om = wp_object_manager_new ();
+ g_autoptr (WpObjectManager) sessions_om =
+ self->sessions_om = wp_object_manager_new ();
g_autoptr (WpObjectManager) endpoints_om =
self->endpoints_om = wp_object_manager_new ();
GVariantBuilder b;
@@ -210,11 +278,13 @@ audiomixer_thread (struct audiomixer * self)
g_variant_new_string ("Audio/Sink"));
g_variant_builder_close (&b);
- wp_object_manager_add_proxy_interest (self->endpoints_om,
+ wp_object_manager_add_proxy_interest (endpoints_om,
PW_TYPE_INTERFACE_Endpoint,
g_variant_builder_end (&b),
WP_PROXY_FEATURE_INFO | WP_PROXY_ENDPOINT_FEATURE_CONTROLS);
- wp_core_install_object_manager (self->core, self->endpoints_om);
+ g_signal_connect (endpoints_om, "objects-changed",
+ (GCallback) endpoints_changed, self);
+ wp_core_install_object_manager (self->core, endpoints_om);
g_signal_connect (self->core, "remote-state-changed",
(GCallback) remote_state_changed, self);
@@ -249,6 +319,7 @@ audiomixer_new (void)
self->state = WP_REMOTE_STATE_UNCONNECTED;
self->mixer_controls = g_ptr_array_new_with_free_func (
(GDestroyNotify) mixer_control_impl_free);
+ g_weak_ref_init (&self->session, NULL);
self->thread = g_thread_new ("audiomixer", (GThreadFunc) audiomixer_thread,
self);
@@ -262,6 +333,7 @@ audiomixer_free(struct audiomixer *self)
g_main_loop_quit (self->loop);
g_thread_join (self->thread);
+ g_weak_ref_clear (&self->session);
g_ptr_array_unref (self->mixer_controls);
g_object_unref (self->core);
g_main_loop_unref (self->loop);
@@ -284,13 +356,6 @@ audiomixer_unlock(struct audiomixer *self)
g_mutex_unlock (&self->lock);
}
-static gboolean
-connect_in_idle (struct audiomixer * self)
-{
- wp_core_connect (self->core);
- return G_SOURCE_REMOVE;
-}
-
int
audiomixer_ensure_connected(struct audiomixer *self, int timeout_sec)
{