[Spice-devel] [PATCH spice-gtk 1/2] audio: use weak references to channel

Marc-André Lureau marcandre.lureau at redhat.com
Fri Nov 7 11:41:58 PST 2014


The audio channels are currently referenced and released on channel
close events. However, this event may not happen if the channel never
was connected. Keeping channels alive also prevent session from
finishing.

By not holding the ref, the channel can go to dispose
when it is no longer needed, and the session can be disposed too.
---
 gtk/spice-gstaudio.c | 81 ++++++++++++++++++++--------------------------------
 gtk/spice-pulse.c    | 65 ++++++++++++++++-------------------------
 2 files changed, 56 insertions(+), 90 deletions(-)

diff --git a/gtk/spice-gstaudio.c b/gtk/spice-gstaudio.c
index e6cf6a9..d784aa1 100644
--- a/gtk/spice-gstaudio.c
+++ b/gtk/spice-gstaudio.c
@@ -53,9 +53,8 @@ struct _SpiceGstaudioPrivate {
     guint                   mmtime_id;
 };
 
-static void channel_event(SpiceChannel *channel, SpiceChannelEvent event,
-                          gpointer data);
 static gboolean connect_channel(SpiceAudio *audio, SpiceChannel *channel);
+static void channel_weak_notified(gpointer data, GObject *where_the_object_was);
 
 static void spice_gstaudio_finalize(GObject *obj)
 {
@@ -91,27 +90,21 @@ static void spice_gstaudio_dispose(GObject *obj)
     stream_dispose(&p->playback);
     stream_dispose(&p->record);
 
-    if (p->pchannel != NULL) {
-        g_signal_handlers_disconnect_by_func(p->pchannel,
-                                             channel_event, obj);
-        g_object_unref(p->pchannel);
-        p->pchannel = NULL;
-    }
+    if (p->pchannel)
+        g_object_weak_unref(G_OBJECT(p->pchannel), channel_weak_notified, gstaudio);
+    p->pchannel = NULL;
 
-    if (p->rchannel != NULL) {
-        g_signal_handlers_disconnect_by_func(p->rchannel,
-                                             channel_event, obj);
-        g_object_unref(p->rchannel);
-        p->rchannel = NULL;
-    }
+    if (p->rchannel)
+        g_object_weak_unref(G_OBJECT(p->rchannel), channel_weak_notified, gstaudio);
+    p->rchannel = NULL;
 
     if (G_OBJECT_CLASS(spice_gstaudio_parent_class)->dispose)
         G_OBJECT_CLASS(spice_gstaudio_parent_class)->dispose(obj);
 }
 
-static void spice_gstaudio_init(SpiceGstaudio *pulse)
+static void spice_gstaudio_init(SpiceGstaudio *gstaudio)
 {
-    pulse->priv = SPICE_GSTAUDIO_GET_PRIVATE(pulse);
+    gstaudio->priv = SPICE_GSTAUDIO_GET_PRIVATE(gstaudio);
 }
 
 static void spice_gstaudio_class_init(SpiceGstaudioClass *klass)
@@ -144,9 +137,8 @@ static void record_new_buffer(GstAppSink *appsink, gpointer data)
     gst_element_post_message(p->record.pipe, msg);
 }
 
-static void record_stop(SpiceRecordChannel *channel, gpointer data)
+static void record_stop(SpiceGstaudio *gstaudio)
 {
-    SpiceGstaudio *gstaudio = data;
     SpiceGstaudioPrivate *p = gstaudio->priv;
 
     SPICE_DEBUG("%s", __FUNCTION__);
@@ -230,7 +222,7 @@ static void record_start(SpiceRecordChannel *channel, gint format, gint channels
     if (p->record.pipe &&
         (p->record.rate != frequency ||
          p->record.channels != channels)) {
-        record_stop(channel, data);
+        record_stop(gstaudio);
         gst_object_unref(p->record.pipe);
         p->record.pipe = NULL;
     }
@@ -285,31 +277,6 @@ lerr:
         gst_element_set_state(p->record.pipe, GST_STATE_PLAYING);
 }
 
-static void channel_event(SpiceChannel *channel, SpiceChannelEvent event,
-                          gpointer data)
-{
-    SpiceGstaudio *gstaudio = data;
-    SpiceGstaudioPrivate *p = gstaudio->priv;
-
-    switch (event) {
-    case SPICE_CHANNEL_OPENED:
-        break;
-    case SPICE_CHANNEL_CLOSED:
-        if (channel == p->pchannel) {
-            p->pchannel = NULL;
-            g_object_unref(channel);
-        } else if (channel == p->rchannel) {
-            record_stop(SPICE_RECORD_CHANNEL(channel), gstaudio);
-            p->rchannel = NULL;
-            g_object_unref(channel);
-        } else /* if (p->pchannel || p->rchannel) */
-            g_warn_if_reached();
-        break;
-    default:
-        break;
-    }
-}
-
 static void playback_stop(SpicePlaybackChannel *channel, gpointer data)
 {
     SpiceGstaudio *gstaudio = data;
@@ -542,6 +509,22 @@ static void record_mute_changed(GObject *object, GParamSpec *pspec, gpointer dat
     g_object_unref(e);
 }
 
+static void
+channel_weak_notified(gpointer data,
+                      GObject *where_the_object_was)
+{
+    SpiceGstaudio *gstaudio = SPICE_GSTAUDIO(data);
+    SpiceGstaudioPrivate *p = gstaudio->priv;
+
+    if (where_the_object_was == (GObject *)p->pchannel) {
+        p->pchannel = NULL;
+    } else if (where_the_object_was == (GObject *)p->rchannel) {
+        SPICE_DEBUG("record closed");
+        record_stop(gstaudio);
+        p->rchannel = NULL;
+    }
+}
+
 static gboolean connect_channel(SpiceAudio *audio, SpiceChannel *channel)
 {
     SpiceGstaudio *gstaudio = SPICE_GSTAUDIO(audio);
@@ -550,15 +533,14 @@ static gboolean connect_channel(SpiceAudio *audio, SpiceChannel *channel)
     if (SPICE_IS_PLAYBACK_CHANNEL(channel)) {
         g_return_val_if_fail(p->pchannel == NULL, FALSE);
 
-        p->pchannel = g_object_ref(channel);
+        p->pchannel = channel;
+        g_object_weak_ref(G_OBJECT(p->pchannel), channel_weak_notified, audio);
         spice_g_signal_connect_object(channel, "playback-start",
                                       G_CALLBACK(playback_start), gstaudio, 0);
         spice_g_signal_connect_object(channel, "playback-data",
                                       G_CALLBACK(playback_data), gstaudio, 0);
         spice_g_signal_connect_object(channel, "playback-stop",
                                       G_CALLBACK(playback_stop), gstaudio, 0);
-        spice_g_signal_connect_object(channel, "channel-event",
-                                      G_CALLBACK(channel_event), gstaudio, 0);
         spice_g_signal_connect_object(channel, "notify::volume",
                                       G_CALLBACK(playback_volume_changed), gstaudio, 0);
         spice_g_signal_connect_object(channel, "notify::mute",
@@ -571,12 +553,11 @@ static gboolean connect_channel(SpiceAudio *audio, SpiceChannel *channel)
         g_return_val_if_fail(p->rchannel == NULL, FALSE);
 
         p->rchannel = g_object_ref(channel);
+        g_object_weak_ref(G_OBJECT(p->rchannel), channel_weak_notified, audio);
         spice_g_signal_connect_object(channel, "record-start",
                                       G_CALLBACK(record_start), gstaudio, 0);
         spice_g_signal_connect_object(channel, "record-stop",
-                                      G_CALLBACK(record_stop), gstaudio, 0);
-        spice_g_signal_connect_object(channel, "channel-event",
-                                      G_CALLBACK(channel_event), gstaudio, 0);
+                                      G_CALLBACK(record_stop), gstaudio, G_CONNECT_SWAPPED);
         spice_g_signal_connect_object(channel, "notify::volume",
                                       G_CALLBACK(record_volume_changed), gstaudio, 0);
         spice_g_signal_connect_object(channel, "notify::mute",
diff --git a/gtk/spice-pulse.c b/gtk/spice-pulse.c
index 744ce4d..fc1662e 100644
--- a/gtk/spice-pulse.c
+++ b/gtk/spice-pulse.c
@@ -74,10 +74,9 @@ static const char *context_state_names[] = {
 #define STATE_NAME(array, state) \
     ((state < G_N_ELEMENTS(array)) ? array[state] : NULL)
 
-static void channel_event(SpiceChannel *channel, SpiceChannelEvent event,
-                          gpointer data);
 static void stream_stop(SpicePulse *pulse, struct stream *s);
 static gboolean connect_channel(SpiceAudio *audio, SpiceChannel *channel);
+static void channel_weak_notified(gpointer data, GObject *where_the_object_was);
 
 static void spice_pulse_finalize(GObject *obj)
 {
@@ -120,11 +119,11 @@ static void spice_pulse_dispose(GObject *obj)
     p->record.cork_op = NULL;
 
     if (p->pchannel)
-        g_object_unref(p->pchannel);
+        g_object_weak_unref(G_OBJECT(p->pchannel), channel_weak_notified, pulse);
     p->pchannel = NULL;
 
     if (p->rchannel)
-        g_object_unref(p->rchannel);
+        g_object_weak_unref(G_OBJECT(p->rchannel), channel_weak_notified, pulse);
     p->rchannel = NULL;
 
     G_OBJECT_CLASS(spice_pulse_parent_class)->dispose(obj);
@@ -567,9 +566,8 @@ static void record_start(SpiceRecordChannel *channel, gint format, gint channels
     p->state = state;
 }
 
-static void record_stop(SpiceRecordChannel *channel, gpointer data)
+static void record_stop(SpicePulse *pulse)
 {
-    SpicePulse *pulse = data;
     SpicePulsePrivate *p = pulse->priv;
 
     SPICE_DEBUG("%s", __FUNCTION__);
@@ -581,33 +579,6 @@ static void record_stop(SpiceRecordChannel *channel, gpointer data)
     stream_stop(pulse, &p->record);
 }
 
-static void channel_event(SpiceChannel *channel, SpiceChannelEvent event,
-                          gpointer data)
-{
-    SpicePulse *pulse = data;
-    SpicePulsePrivate *p = pulse->priv;
-
-    switch (event) {
-    case SPICE_CHANNEL_OPENED:
-        break;
-    case SPICE_CHANNEL_CLOSED:
-        if (channel == p->pchannel) {
-            SPICE_DEBUG("playback closed");
-            p->pchannel = NULL;
-            g_object_unref(channel);
-        } else if (channel == p->rchannel) {
-            SPICE_DEBUG("record closed");
-            record_stop(SPICE_RECORD_CHANNEL(channel), pulse);
-            p->rchannel = NULL;
-            g_object_unref(channel);
-        } else /* if (p->pchannel || p->rchannel) */
-            g_warn_if_reached();
-        break;
-    default:
-        break;
-    }
-}
-
 static void playback_volume_changed(GObject *object, GParamSpec *pspec, gpointer data)
 {
     SpicePulse *pulse = data;
@@ -748,6 +719,22 @@ static void record_volume_changed(GObject *object, GParamSpec *pspec, gpointer d
         pa_operation_unref(op);
 }
 
+static void
+channel_weak_notified(gpointer data,
+                      GObject *where_the_object_was)
+{
+    SpicePulse *pulse = SPICE_PULSE(data);
+    SpicePulsePrivate *p = pulse->priv;
+
+    if (where_the_object_was == (GObject *)p->pchannel) {
+        p->pchannel = NULL;
+    } else if (where_the_object_was == (GObject *)p->rchannel) {
+        SPICE_DEBUG("record closed");
+        record_stop(pulse);
+        p->rchannel = NULL;
+    }
+}
+
 static gboolean connect_channel(SpiceAudio *audio, SpiceChannel *channel)
 {
     SpicePulse *pulse = SPICE_PULSE(audio);
@@ -756,15 +743,14 @@ static gboolean connect_channel(SpiceAudio *audio, SpiceChannel *channel)
     if (SPICE_IS_PLAYBACK_CHANNEL(channel)) {
         g_return_val_if_fail(p->pchannel == NULL, FALSE);
 
-        p->pchannel = g_object_ref(channel);
+        p->pchannel = channel;
+        g_object_weak_ref(G_OBJECT(p->pchannel), channel_weak_notified, audio);
         spice_g_signal_connect_object(channel, "playback-start",
                                       G_CALLBACK(playback_start), pulse, 0);
         spice_g_signal_connect_object(channel, "playback-data",
                                       G_CALLBACK(playback_data), pulse, 0);
         spice_g_signal_connect_object(channel, "playback-stop",
                                       G_CALLBACK(playback_stop), pulse, 0);
-        spice_g_signal_connect_object(channel, "channel-event",
-                                      G_CALLBACK(channel_event), pulse, 0);
         spice_g_signal_connect_object(channel, "notify::volume",
                                       G_CALLBACK(playback_volume_changed), pulse, 0);
         spice_g_signal_connect_object(channel, "notify::mute",
@@ -778,13 +764,12 @@ static gboolean connect_channel(SpiceAudio *audio, SpiceChannel *channel)
     if (SPICE_IS_RECORD_CHANNEL(channel)) {
         g_return_val_if_fail(p->rchannel == NULL, FALSE);
 
-        p->rchannel = g_object_ref(channel);
+        p->rchannel = channel;
+        g_object_weak_ref(G_OBJECT(p->rchannel), channel_weak_notified, audio);
         spice_g_signal_connect_object(channel, "record-start",
                                       G_CALLBACK(record_start), pulse, 0);
         spice_g_signal_connect_object(channel, "record-stop",
-                                      G_CALLBACK(record_stop), pulse, 0);
-        spice_g_signal_connect_object(channel, "channel-event",
-                                      G_CALLBACK(channel_event), pulse, 0);
+                                      G_CALLBACK(record_stop), pulse, G_CONNECT_SWAPPED);
         spice_g_signal_connect_object(channel, "notify::volume",
                                       G_CALLBACK(record_volume_changed), pulse, 0);
         spice_g_signal_connect_object(channel, "notify::mute",
-- 
1.9.3



More information about the Spice-devel mailing list