[Telepathy-commits] [telepathy-gabble/master] properly dispose of MediaStreams when content is removed or rejected

Senko Rasic senko at phyrexia.lan
Tue Dec 2 04:34:02 PST 2008


---
 src/jingle-content.c |   77 ++++++++++++++++-------
 src/jingle-content.h |    2 +-
 src/jingle-session.c |  167 ++++++++++++++++++++++++++++++++++++++++---------
 src/media-channel.c  |   25 +++++---
 src/media-stream.c   |   17 +++++-
 5 files changed, 222 insertions(+), 66 deletions(-)

diff --git a/src/jingle-content.c b/src/jingle-content.c
index 5cdc3d6..c571c2f 100644
--- a/src/jingle-content.c
+++ b/src/jingle-content.c
@@ -1,5 +1,5 @@
 /*
- * gabble-jingle-session.c - Source for GabbleJingleContent
+ * gabble-jingle-content.c - Source for GabbleJingleContent
  * Copyright (C) 2008 Collabora Ltd.
  *
  * This library is free software; you can redistribute it and/or
@@ -41,6 +41,7 @@ enum
 {
   READY,
   NEW_CANDIDATES,
+  REMOVED,
   LAST_SIGNAL
 };
 
@@ -394,6 +395,13 @@ gabble_jingle_content_class_init (GabbleJingleContentClass *cls)
     NULL, NULL,
     g_cclosure_marshal_VOID__POINTER, G_TYPE_NONE, 1, G_TYPE_POINTER);
 
+  signals[REMOVED] = g_signal_new ("removed",
+    G_OBJECT_CLASS_TYPE (cls),
+    G_SIGNAL_RUN_LAST | G_SIGNAL_DETAILED,
+    0,
+    NULL, NULL,
+    g_cclosure_marshal_VOID__VOID,
+    G_TYPE_NONE, 0);
 }
 
 #define SET_BAD_REQ(txt...) g_set_error (error, GABBLE_XMPP_ERROR, XMPP_ERROR_BAD_REQUEST, txt)
@@ -434,7 +442,7 @@ send_gtalk4_transport_accept (gpointer user_data)
   tnode = lm_message_node_add_child (sess_node, "transport", NULL);
   lm_message_node_set_attribute (tnode, "xmlns", priv->transport_ns);
 
-  gabble_jingle_session_send (c->session, msg, NULL);
+  gabble_jingle_session_send (c->session, msg, NULL, NULL);
 
   return FALSE;
 }
@@ -765,7 +773,7 @@ send_content_add_or_accept (GabbleJingleContent *self)
   msg = gabble_jingle_session_new_message (self->session,
       action, &sess_node);
   gabble_jingle_content_produce_node (self, sess_node, TRUE);
-  gabble_jingle_session_send (self->session, msg, NULL);
+  gabble_jingle_session_send (self->session, msg, NULL, NULL);
 
   priv->state = new_state;
   g_object_notify (G_OBJECT (self), "state");
@@ -846,25 +854,6 @@ gabble_jingle_content_set_transport_state (GabbleJingleContent *self,
     }
 }
 
-void
-gabble_jingle_content_accept (GabbleJingleContent *c)
-{
-  GabbleJingleContentPrivate *priv = GABBLE_JINGLE_CONTENT_GET_PRIVATE (c);
-  LmMessage *msg;
-  LmMessageNode *sess_node;
-
-  g_assert (!priv->created_by_us);
-  g_assert (gabble_jingle_content_is_ready (c));
-
-  msg = gabble_jingle_session_new_message (c->session,
-      JINGLE_ACTION_CONTENT_ACCEPT, &sess_node);
-
-  gabble_jingle_content_produce_node (c, sess_node, TRUE);
-  gabble_jingle_session_send (c->session, msg, NULL);
-
-  g_object_set (c, "state", JINGLE_CONTENT_STATE_ACKNOWLEDGED, NULL);
-}
-
 GList *
 gabble_jingle_content_get_remote_candidates (GabbleJingleContent *c)
 {
@@ -895,9 +884,51 @@ gabble_jingle_content_change_direction (GabbleJingleContent *c,
   msg = gabble_jingle_session_new_message (c->session,
       JINGLE_ACTION_CONTENT_MODIFY, &sess_node);
   gabble_jingle_content_produce_node (c, sess_node, FALSE); 
-  gabble_jingle_session_send (c->session, msg, NULL);
+  gabble_jingle_session_send (c->session, msg, NULL, NULL);
 
   /* FIXME: actually check whether remote end accepts our content-modify */
   return TRUE;
 }
 
+static void
+_on_remove_reply (GabbleJingleSession *sess, gboolean success,
+    LmMessage *reply, gpointer user_data)
+{
+  GabbleJingleContent *c = GABBLE_JINGLE_CONTENT (user_data);
+  GabbleJingleContentPrivate *priv = GABBLE_JINGLE_CONTENT_GET_PRIVATE (c);
+
+  g_assert (priv->state == JINGLE_CONTENT_STATE_REMOVING);
+
+  g_signal_emit (c, signals[REMOVED], 0);
+}
+
+void
+gabble_jingle_content_remove (GabbleJingleContent *c)
+{
+  GabbleJingleContentPrivate *priv = GABBLE_JINGLE_CONTENT_GET_PRIVATE (c);
+  LmMessage *msg;
+  LmMessageNode *sess_node;
+
+  if (priv->state == JINGLE_CONTENT_STATE_REMOVING)
+    {
+      DEBUG ("ignoring request to remove content which is already being removed");
+      return;
+    }
+
+  priv->state = JINGLE_CONTENT_STATE_REMOVING;
+  g_object_notify ((GObject *) c, "state");
+
+  /* If we were already signalled, we have to signal removal to the peer. */
+  if (priv->state != JINGLE_CONTENT_STATE_EMPTY)
+    {
+      msg = gabble_jingle_session_new_message (c->session,
+          JINGLE_ACTION_CONTENT_REMOVE, &sess_node);
+      gabble_jingle_content_produce_node (c, sess_node, FALSE);
+      gabble_jingle_session_send (c->session, msg, _on_remove_reply, c);
+    }
+  else
+    {
+      g_signal_emit (c, signals[REMOVED], 0);
+    }
+}
+
diff --git a/src/jingle-content.h b/src/jingle-content.h
index f4b7cbc..116b8dc 100644
--- a/src/jingle-content.h
+++ b/src/jingle-content.h
@@ -108,7 +108,7 @@ void _gabble_jingle_content_set_media_ready (GabbleJingleContent *self);
 gboolean gabble_jingle_content_is_ready (GabbleJingleContent *self);
 void gabble_jingle_content_set_transport_state (GabbleJingleContent *content,
     JingleTransportState state);
-void gabble_jingle_content_accept (GabbleJingleContent *c);
+void gabble_jingle_content_remove (GabbleJingleContent *c);
 GList *gabble_jingle_content_get_remote_candidates (GabbleJingleContent *c);
 gboolean gabble_jingle_content_change_direction (GabbleJingleContent *c,
     JingleContentSenders senders);
diff --git a/src/jingle-session.c b/src/jingle-session.c
index 35a5c3e..b92e958 100644
--- a/src/jingle-session.c
+++ b/src/jingle-session.c
@@ -495,6 +495,56 @@ _foreach_content (GabbleJingleSession *sess, LmMessageNode *node,
 }
 
 static void content_ready_cb (GabbleJingleContent *c, gpointer user_data);
+static void content_removed_cb (GabbleJingleContent *c, gpointer user_data);
+
+struct idle_content_reject_ctx {
+    GabbleJingleSession *session;
+    gchar *creator;
+    gchar *name;
+};
+
+static gboolean
+idle_content_reject (gpointer data)
+{
+  LmMessage *msg;
+  LmMessageNode *sess_node, *node;
+  struct idle_content_reject_ctx *ctx = data;
+
+  msg = gabble_jingle_session_new_message (ctx->session,
+      JINGLE_ACTION_CONTENT_REJECT, &sess_node);
+
+  g_debug ("name = %s, intiiator = %s", ctx->name, ctx->creator);
+
+  node = lm_message_node_add_child (sess_node, "content", NULL);
+  lm_message_node_set_attributes (node,
+      "name", ctx->name, "creator", ctx->creator, NULL);
+
+  gabble_jingle_session_send (ctx->session, msg, NULL, NULL);
+
+  g_object_unref (ctx->session);
+  g_free (ctx->name);
+  g_free (ctx->creator);
+  g_free (ctx);
+
+  return FALSE;
+}
+
+static void
+fire_idle_content_reject (GabbleJingleSession *sess, const gchar *name,
+    const gchar *creator)
+{
+  struct idle_content_reject_ctx *ctx = g_new0 (struct idle_content_reject_ctx, 1);
+
+  if (creator == NULL)
+      creator = "";
+
+  ctx->session = g_object_ref (sess);
+  ctx->name = g_strdup (name);
+  ctx->creator = g_strdup (creator);
+
+  g_idle_add (idle_content_reject, ctx);
+}
+
 
 static void
 _each_content_add (GabbleJingleSession *sess, GabbleJingleContent *c,
@@ -516,8 +566,19 @@ _each_content_add (GabbleJingleSession *sess, GabbleJingleContent *c,
 
   if (content_type == 0)
     {
-      SET_BAD_REQ ("unsupported content type");
       DEBUG ("unsupported content type with ns %s", content_ns);
+
+      /* if this is session-initiate, we should return error, otherwise,
+       * we should respond with content-reject */
+      if (priv->state < JS_STATE_PENDING_INITIATED)
+        {
+          SET_BAD_REQ ("unsupported content type");
+        }
+      else
+        {
+          fire_idle_content_reject (sess, name,
+              lm_message_node_get_attribute (content_node, "creator"));
+        }
       return;
     }
 
@@ -556,6 +617,8 @@ _each_content_add (GabbleJingleSession *sess, GabbleJingleContent *c,
 
   g_signal_connect (c, "ready",
       (GCallback) content_ready_cb, sess);
+  g_signal_connect (c, "removed",
+      (GCallback) content_removed_cb, sess);
 
   gabble_jingle_content_parse_add (c, content_node,
     ((priv->dialect == JINGLE_DIALECT_GTALK3) ||
@@ -592,15 +655,17 @@ _each_content_remove (GabbleJingleSession *sess, GabbleJingleContent *c,
       return;
     }
 
+  /* FIXME: do we treat this as a session terminate instead of error? */
   if (g_hash_table_size (priv->contents) == 1)
     {
       SET_BAD_REQ ("unable to remove the last content in a session");
       return;
     }
 
-  /* This should have the effect of shutting the content down.
-   * FIXME: do we need to have REMOVING state at all? */
-  g_hash_table_remove (priv->contents, name);
+  g_object_set (c, "state", JINGLE_CONTENT_STATE_REMOVING, NULL);
+  /* FIXME: we emit "removed" on GabbleJingleContent object, is this
+   * too hackish? */
+  g_signal_emit_by_name (c, "removed");
 }
 
 static void
@@ -963,6 +1028,7 @@ gabble_jingle_session_parse (GabbleJingleSession *sess, LmMessage *message, GErr
       SET_BAD_REQ ("sender with no resource");
       return NULL;
     }
+  resource++;
 
   /* this one is not required, so it can be NULL */
   responder = lm_message_node_get_attribute (session_node, "responder");
@@ -1174,36 +1240,47 @@ _fill_content (GabbleJingleSession *sess,
     }
 }
 
+struct jingle_reply_ctx {
+  JingleReplyHandler handler;
+  gpointer user_data;
+};
+
 static LmHandlerResult
 _process_reply (GabbleConnection *conn,
     LmMessage *sent, LmMessage *reply, GObject *obj, gpointer user_data)
 {
   GabbleJingleSession *sess = GABBLE_JINGLE_SESSION (obj);
-  JingleReplyHandler handler = user_data;
+  struct jingle_reply_ctx *ctx = user_data;
 
   if (lm_message_get_sub_type (reply) == LM_MESSAGE_SUB_TYPE_RESULT)
     {
-      handler (sess, TRUE, reply);
+      ctx->handler (sess, TRUE, reply, ctx->user_data);
     }
   else
     {
-      handler (sess, FALSE, reply);
+      ctx->handler (sess, FALSE, reply, ctx->user_data);
     }
 
+  g_free (ctx);
   return LM_HANDLER_RESULT_REMOVE_MESSAGE;
 }
 
 void
 gabble_jingle_session_send (GabbleJingleSession *sess, LmMessage *msg,
-    JingleReplyHandler cb)
+    JingleReplyHandler cb, gpointer user_data)
 {
   GabbleJingleSessionPrivate *priv = GABBLE_JINGLE_SESSION_GET_PRIVATE (sess);
 
   if (cb != NULL)
     {
+      struct jingle_reply_ctx *ctx = g_new0 (struct jingle_reply_ctx, 1);
+
+      ctx->handler = cb;
+      ctx->user_data = user_data;
+
       DEBUG ("sending with reply %p", cb);
       _gabble_connection_send_with_reply (priv->conn, msg,
-          _process_reply, G_OBJECT (sess), cb, NULL);
+          _process_reply, G_OBJECT (sess), ctx, NULL);
     }
   else
     {
@@ -1215,7 +1292,7 @@ gabble_jingle_session_send (GabbleJingleSession *sess, LmMessage *msg,
 
 static void
 _on_initiate_reply (GabbleJingleSession *sess, gboolean success,
-    LmMessage *reply)
+    LmMessage *reply, gpointer user_data)
 {
   if (success)
       set_state (sess, JS_STATE_PENDING_INITIATED);
@@ -1225,7 +1302,7 @@ _on_initiate_reply (GabbleJingleSession *sess, gboolean success,
 
 static void
 _on_accept_reply (GabbleJingleSession *sess, gboolean success,
-    LmMessage *reply)
+    LmMessage *reply, gpointer user_data)
 {
   /* FIXME: clear session timeout timer here */
 
@@ -1297,7 +1374,7 @@ try_session_initiate_or_accept (GabbleJingleSession *sess)
 
   msg = gabble_jingle_session_new_message (sess, action, &sess_node);
   _map_initial_contents (sess, _fill_content, sess_node);
-  gabble_jingle_session_send (sess, msg, handler);
+  gabble_jingle_session_send (sess, msg, handler, NULL);
   set_state (sess, new_state);
 }
 
@@ -1349,7 +1426,7 @@ gabble_jingle_session_terminate (GabbleJingleSession *sess)
     {
       gabble_jingle_session_send (sess,
           gabble_jingle_session_new_message (sess,
-              JINGLE_ACTION_SESSION_TERMINATE, NULL), NULL);
+              JINGLE_ACTION_SESSION_TERMINATE, NULL), NULL, NULL);
     }
 
   /* NOTE: on "terminated", jingle factory and media channel will unref
@@ -1361,33 +1438,59 @@ gabble_jingle_session_terminate (GabbleJingleSession *sess)
   set_state (sess, JS_STATE_ENDED);
 }
 
-void
-gabble_jingle_session_remove_content (GabbleJingleSession *sess,
-    GabbleJingleContent *c)
+static gboolean
+_terminate_delayed (gpointer user_data)
+{
+  GabbleJingleSession *sess = user_data;
+  gabble_jingle_session_terminate (sess);
+  return FALSE;
+}
+
+static void
+content_removed_cb (GabbleJingleContent *c, gpointer user_data)
 {
+  GabbleJingleSession *sess = GABBLE_JINGLE_SESSION (user_data);
   GabbleJingleSessionPrivate *priv = GABBLE_JINGLE_SESSION_GET_PRIVATE (sess);
-  const gchar *content_name;
-  LmMessage *msg;
-  LmMessageNode *sess_node;
+  const gchar *name;
+
+  DEBUG ("JingleSession:contant_removed_cb() called");
 
-  g_object_get (c, "name", &content_name, NULL);
+  g_object_get (c, "name", &name, NULL);
+  g_hash_table_remove (priv->contents, name);
 
-  DEBUG ("removing content '%s'", content_name);
+  if (g_hash_table_size (priv->contents) == 0)
+    {
+      /* Terminate the session from idle loop
+       * so that content removal can be processed
+       * in media stream before that. */
+      g_idle_add (_terminate_delayed, sess);
+    }
+}
 
-  msg = gabble_jingle_session_new_message (sess, JINGLE_ACTION_CONTENT_REMOVE,
-      &sess_node);
-  gabble_jingle_content_produce_node (c, sess_node, FALSE);
-  gabble_jingle_session_send (sess, msg, NULL);
+void
+gabble_jingle_session_remove_content (GabbleJingleSession *sess,
+    GabbleJingleContent *c)
+{
+  GabbleJingleSessionPrivate *priv = GABBLE_JINGLE_SESSION_GET_PRIVATE (sess);
 
-  /* FIXME: emit a 'content-removed' signal, mark it as in removal? */
-  /* When we and MediaChannel unref the object, it should get disposed */
-  g_hash_table_remove (priv->contents, content_name);
+  DEBUG ("called");
 
-  /* If that was the last one, we can terminate the session */
-  if (g_hash_table_size (priv->contents) == 0)
+  /* If this is the last content, instead of removing it we can just
+   * terminate the entire session. */
+  if (g_hash_table_size (priv->contents) == 1)
     {
-      gabble_jingle_session_terminate (sess);
+      /* We'll fake the content removal signal, so both we and media
+       * channel clean up after it properly. */
+      // gabble_jingle_session_terminate (sess);
+      DEBUG ("manually removing content %p and emitting the signal", c);
+      g_object_set (c, "state", JINGLE_CONTENT_STATE_REMOVING, NULL);
+      g_signal_emit_by_name (c, "removed");
+      return;
     }
+
+  /* When content-remove is acknowledged, "removed" signal will be fired,
+   * so we can clean up. */
+  gabble_jingle_content_remove (c);
 }
 
 GabbleJingleContent *
@@ -1426,6 +1529,8 @@ gabble_jingle_session_add_content (GabbleJingleSession *sess, JingleMediaType mt
 
   g_signal_connect (c, "ready",
       (GCallback) content_ready_cb, sess);
+  g_signal_connect (c, "removed",
+      (GCallback) content_removed_cb, sess);
 
   g_hash_table_insert (priv->contents, g_strdup (name), c);
   g_signal_emit (sess, signals[NEW_CONTENT], 0, c);
diff --git a/src/media-channel.c b/src/media-channel.c
index e37b8f6..86a474c 100644
--- a/src/media-channel.c
+++ b/src/media-channel.c
@@ -1730,8 +1730,11 @@ session_terminated_cb (GabbleJingleSession *session,
   if (priv->streams != NULL)
     {
       GPtrArray *tmp = priv->streams;
+      GabbleMediaStream *s = g_ptr_array_index (tmp, 0);
 
       DEBUG ("unreffing streams");
+      DEBUG ("%p with refcount %d",
+          s, G_OBJECT(s)->ref_count);
 
       /* move priv->streams aside so that the stream_close_cb
        * doesn't double unref */
@@ -2013,6 +2016,10 @@ stream_close_cb (GabbleMediaStream *stream,
   GabbleMediaChannelPrivate *priv = GABBLE_MEDIA_CHANNEL_GET_PRIVATE (chan);
   guint id;
 
+  DEBUG ("%p called", chan);
+
+  g_assert (GABBLE_IS_MEDIA_CHANNEL (chan));
+
   g_object_get (stream,
       "id", &id,
       NULL);
@@ -2038,20 +2045,20 @@ stream_error_cb (GabbleMediaStream *stream,
                  const gchar *message,
                  GabbleMediaChannel *chan)
 {
-  // GabbleMediaChannelPrivate *priv = GABBLE_MEDIA_CHANNEL_GET_PRIVATE (chan);
+  GabbleMediaChannelPrivate *priv = GABBLE_MEDIA_CHANNEL_GET_PRIVATE (chan);
+  GabbleJingleContent *c;
   guint id;
 
+  DEBUG ("%p called", chan);
+
   /* emit signal */
-  g_object_get (stream, "id", &id, NULL);
+  g_object_get (stream, "id", &id, "content", &c, NULL);
   tp_svc_channel_type_streamed_media_emit_stream_error (chan, id, errno,
       message);
 
-  /* remove stream from session */
-  // TODO _gabble_media_session_remove_streams (priv->session, &stream, 1);
-
-  /* FIXME: we unref the stream, so it should be disposed and remove
-   * jingle content. */
-  g_object_unref (stream);
+  /* remove stream from session (removal will be signalled
+   * so we can dispose of the stream) */
+  gabble_jingle_session_remove_content (priv->session, c);
 }
 
 static void
@@ -2159,7 +2166,7 @@ create_stream_from_content (GabbleMediaChannel *chan, GabbleJingleContent *c)
       "id", id,
       NULL);
 
-  DEBUG ("created new MediaStream %p for content '%s'", stream, name);
+  DEBUG ("%p: created new MediaStream %p for content '%s'", chan, stream, name);
 
   /* we will own the only reference to this stream */
   g_ptr_array_add (priv->streams, stream);
diff --git a/src/media-stream.c b/src/media-stream.c
index 7c5e7b1..3e12cb8 100644
--- a/src/media-stream.c
+++ b/src/media-stream.c
@@ -60,7 +60,6 @@ G_DEFINE_TYPE_WITH_CODE(GabbleMediaStream,
 enum
 {
     DESTROY,
-
     NEW_ACTIVE_CANDIDATE_PAIR,
     NEW_NATIVE_CANDIDATE,
     SUPPORTED_CODECS,
@@ -153,6 +152,8 @@ static void content_state_changed_cb (GabbleJingleContent *c,
      GParamSpec *pspec, GabbleMediaStream *stream);
 static void content_senders_changed_cb (GabbleJingleContent *c,
      GParamSpec *pspec, GabbleMediaStream *stream);
+static void content_removed_cb (GabbleJingleContent *content,
+      GabbleMediaStream *stream);
 
 static void
 gabble_media_stream_init (GabbleMediaStream *self)
@@ -342,7 +343,7 @@ gabble_media_stream_set_property (GObject      *object,
 
       priv->content = g_value_get_object (value);
 
-      DEBUG ("connecting to content %p signals", priv->content);
+      DEBUG ("%p: connecting to content %p signals", stream, priv->content);
       g_signal_connect (priv->content, "new-candidates",
           (GCallback) new_remote_candidates_cb, stream);
 
@@ -357,6 +358,9 @@ gabble_media_stream_set_property (GObject      *object,
       g_signal_connect (priv->content, "notify::senders",
           (GCallback) content_senders_changed_cb, stream);
 
+      g_signal_connect (priv->content, "removed",
+          (GCallback) content_removed_cb, stream);
+
       /* we can immediately get the codecs if we're responder */
       new_remote_codecs_cb (priv->content,
           gabble_jingle_media_rtp_get_remote_codecs (GABBLE_JINGLE_MEDIA_RTP (priv->content)),
@@ -561,6 +565,7 @@ gabble_media_stream_class_init (GabbleMediaStreamClass *gabble_media_stream_clas
   g_object_class_install_property (object_class, PROP_CONTENT, param_spec);
 
   /* signals not exported by D-Bus interface */
+
   signals[DESTROY] =
     g_signal_new ("destroy",
                   G_OBJECT_CLASS_TYPE (gabble_media_stream_class),
@@ -1440,6 +1445,14 @@ content_senders_changed_cb (GabbleJingleContent *c,
   g_object_get (c, "senders", &senders, NULL);
 }
 
+static void
+content_removed_cb (GabbleJingleContent *content, GabbleMediaStream *stream)
+{
+  DEBUG ("MediaStream:content_removed_cb() called");
+  _gabble_media_stream_close (stream);
+}
+
+
 gboolean
 gabble_media_stream_change_direction (GabbleMediaStream *stream,
     guint requested_dir, GError **error)
-- 
1.5.6.5




More information about the Telepathy-commits mailing list