[Telepathy-commits] [telepathy-gabble/master] Only send codecs whose clockrate or channels have changed.

[Will Thompson]

---
 src/jingle-media-rtp.c                        |   92 +++++++++++++++++++++++-
 tests/twisted/jingle/test-description-info.py |   18 ++---
 2 files changed, 96 insertions(+), 14 deletions(-)

diff --git a/src/jingle-media-rtp.c b/src/jingle-media-rtp.c
index bc60d36..1b08d8d 100644
--- a/src/jingle-media-rtp.c
+++ b/src/jingle-media-rtp.c
@@ -568,6 +568,82 @@ produce_description (GabbleJingleContent *obj, LmMessageNode *content_node)
   priv->local_codec_updates = NULL;
 }
 
+/**
+ * string_string_maps_equal:
+ *
+ * Returns: TRUE iff @a and @b contain exactly the same keys and values when
+ *          compared as strings.
+ */
+static gboolean
+string_string_maps_equal (GHashTable *a,
+                          GHashTable *b)
+{
+  /* FIXME */
+  return TRUE;
+}
+
+/**
+ * codec_info_equal:
+ * Compares the clockrate, channels and params of the supplied codecs,
+ * returning TRUE iff they are all equal.
+ *
+ * Does *not* compare the codecs' id or name.
+ */
+static gboolean
+codec_info_equal (const JingleCodec *c,
+                  const JingleCodec *d)
+{
+  return (c->clockrate == d->clockrate &&
+    c->channels == d->channels &&
+    string_string_maps_equal (c->params, d->params));
+}
+
+static GList *
+changed_codecs (GList *old,
+                GList *new)
+{
+  GList *changed = NULL;
+  GList *k, *l;
+
+  for (k = new; k != NULL; k = k->next)
+    {
+      JingleCodec *new_c = k->data;
+
+      for (l = old; l != NULL; l = l->next)
+        {
+          JingleCodec *old_c = l->data;
+
+          if (new_c->id != old_c->id)
+            continue;
+
+          if (tp_strdiff (new_c->name, old_c->name))
+            {
+              DEBUG ("streaming implementation has changed codec %u's name "
+                  "from %s to %s!", new_c->id, old_c->name, new_c->name);
+
+              /* FIXME: make CodecsUpdated fail. */
+            }
+
+          if (!codec_info_equal (old_c, new_c))
+            {
+              changed = g_list_prepend (changed, new_c);
+              break;
+            }
+        }
+
+      if (l == NULL)
+        {
+          DEBUG ("streaming implementation tried to update codec %u (%s) which "
+              "wasn't there before", new_c->id, new_c->name);
+          /* FIXME: make CodecsUpdated fail. */
+        }
+    }
+
+  /* FIXME: this doesn't detect the streaming implementation trying to remove codecs. */
+
+  return changed;
+}
+
 /* Takes in a list of slice-allocated JingleCodec structs */
 void
 jingle_media_rtp_set_local_codecs (GabbleJingleMediaRtp *self, GList *codecs)
@@ -576,10 +652,18 @@ jingle_media_rtp_set_local_codecs (GabbleJingleMediaRtp *self, GList *codecs)
 
   DEBUG ("setting new local codecs");
 
-  /* TODO: if priv->local_codecs is non-empty, set priv->local_codec_updates to
-   * the changed codecs.
-   */
-  jingle_media_rtp_free_codecs (priv->local_codecs);
+  if (priv->local_codecs != NULL)
+    {
+      /* Calling _gabble_jingle_content_set_media_ready () should use and unset
+       * these right after we set them.
+       */
+      g_assert (priv->local_codec_updates == NULL);
+      priv->local_codec_updates = changed_codecs (priv->local_codecs, codecs);
+
+      jingle_media_rtp_free_codecs (priv->local_codecs);
+      priv->local_codecs = codecs;
+    }
+
   priv->local_codecs = codecs;
 
   _gabble_jingle_content_set_media_ready (GABBLE_JINGLE_CONTENT (self));
diff --git a/tests/twisted/jingle/test-description-info.py b/tests/twisted/jingle/test-description-info.py
index 9b2a6df..0f0bea2 100644
--- a/tests/twisted/jingle/test-description-info.py
+++ b/tests/twisted/jingle/test-description-info.py
@@ -116,16 +116,14 @@ def test(q, bus, conn, stream):
             x.stanza))
     payload_types = xpath.queryForNodes(
         "/iq/jingle/content[@name='stream1']/description/payload-type", e.stanza)
-    # FIXME: Gabble should only include the changed codecs in description-info
-    #assert len(payload_types) == 2, payload_types
-    # The order, strictly speaking, doesn't matter.
-    for i in [0,1]:
-        assert payload_types[i]['name'] == new_codecs[i][0], \
-            (payload_types[i], new_codecs[i])
-        assert payload_types[i]['id'] == str(new_codecs[i][1]), \
-            (payload_types[i], new_codecs[i])
-        assert payload_types[i]['clockrate'] == str(new_codecs[i][2]), \
-            (payload_types[i], new_codecs[i])
+    # Gabble SHOULD only include the changed codecs in description-info
+    assert len(payload_types) == 2, payload_types
+
+    # FIXME: this should check the parameters too.
+    payload_types_tupled = [ (pt['name'], int(pt['id']), int(pt['clockrate']))
+                             for pt in payload_types ]
+    assert sorted(payload_types_tupled) == sorted(new_codecs[0:2]), \
+        (payload_types_tupled, new_codecs[0:2])
 
     # Instead, the remote end decides to change the clockrate of the third codec.
     new_codecs = [ ('GSM', 3, 8000), ('PCMA', 8, 8000), ('PCMU', 0, 1600) ]
-- 
1.5.6.5