[Telepathy-commits] [telepathy-gabble/master] Update (not replace) remote codecs on transport-info

Will Thompson will.thompson at collabora.co.uk
Thu Feb 26 17:21:33 PST 2009


The XEP says that clients SHOULD only send information about codecs
whose parameters have changed in transport-info. So, we have to update
Gabble's cache of remote codecs rather than just replacing it.

This patch also checks whether the name associated with an id has
changed, and if so NAKs the iq. If the remote end has sent information
for a new payload-type, it's just ignored.

The test had to be modified, because it was buggy:
 - it changed the codec names;
 - it sent all the codecs in the transport-info, rather than just the
   changes.
---
 src/jingle-media-rtp.c                        |   68 ++++++++++++++++++++++--
 tests/twisted/jingle/test-description-info.py |   38 ++++++++++----
 2 files changed, 89 insertions(+), 17 deletions(-)

diff --git a/src/jingle-media-rtp.c b/src/jingle-media-rtp.c
index 5e2e8e6..5a95d3b 100644
--- a/src/jingle-media-rtp.c
+++ b/src/jingle-media-rtp.c
@@ -333,6 +333,67 @@ parse_payload_type (LmMessageNode *node)
 }
 
 static void
+update_remote_codecs (GabbleJingleMediaRtp *self,
+                      GList *new_codecs,
+                      GError **error)
+{
+  GabbleJingleMediaRtpPrivate *priv = self->priv;
+  GList *k, *l;
+
+  if (priv->remote_codecs == NULL)
+    {
+      priv->remote_codecs = new_codecs;
+      goto out;
+    }
+
+  /* We already know some remote codecs, so this is just the other end updating
+   * some parameters.
+   */
+  for (k = new_codecs; k != NULL; k = k->next)
+    {
+      JingleCodec *new_codec = k->data;
+
+      for (l = priv->remote_codecs; l != NULL; l = l->next)
+        {
+          JingleCodec *old_codec = l->data;
+          GHashTable *tmp;
+
+          if (old_codec->id != new_codec->id)
+            continue;
+
+          if (tp_strdiff (old_codec->name, new_codec->name))
+            {
+              DEBUG ("Codec with id %u has changed from %s to %s! Rejecting",
+                  old_codec->id, old_codec->name, new_codec->name);
+              SET_BAD_REQ ("Codec with id %u is %s, not %s", old_codec->id,
+                  old_codec->name, new_codec->name);
+              jingle_media_rtp_free_codecs (new_codecs);
+              return;
+            }
+
+          old_codec->clockrate = new_codec->clockrate;
+          old_codec->channels = new_codec->channels;
+
+          tmp = old_codec->params;
+          old_codec->params = new_codec->params;
+          new_codec->params = tmp;
+
+          break;
+        }
+
+      if (l == NULL)
+        {
+          DEBUG ("Codec with id %u ('%s') unknown; ignoring update",
+              new_codec->id, new_codec->name);
+        }
+    }
+
+out:
+  DEBUG ("emitting remote-codecs signal");
+  g_signal_emit (self, signals[REMOTE_CODECS], 0, priv->remote_codecs);
+}
+
+static void
 parse_description (GabbleJingleContent *content,
     LmMessageNode *desc_node, GError **error)
 {
@@ -375,12 +436,7 @@ parse_description (GabbleJingleContent *content,
 
   priv->media_type = mtype;
 
-  DEBUG ("emitting remote-codecs signal");
-  g_signal_emit (self, signals[REMOTE_CODECS], 0, codecs);
-
-  /* set them as the known remote codecs */
-  jingle_media_rtp_free_codecs (priv->remote_codecs);
-  priv->remote_codecs = codecs;
+  update_remote_codecs (self, codecs, error);
 }
 
 static void
diff --git a/tests/twisted/jingle/test-description-info.py b/tests/twisted/jingle/test-description-info.py
index e75e1d0..9b2a6df 100644
--- a/tests/twisted/jingle/test-description-info.py
+++ b/tests/twisted/jingle/test-description-info.py
@@ -106,28 +106,44 @@ def test(q, bus, conn, stream):
     e = q.expect('stream-iq')
     assert jp.match_jingle_action(e.query, 'session-accept')
 
-    # We decide we want to update our codec parameters
-    jt2.audio_codecs = [ ('A', 3, 8000), ('B', 8, 8000), ('C', 0, 8000) ]
-    stream_handler.CodecsUpdated(jt2.get_audio_codecs_dbus())
+    # We decide we want to update the clockrates of the first two codecs, not
+    # changing the third.
+    new_codecs = [ ('GSM', 3, 4000), ('PCMA', 8, 4000), ('PCMU', 0, 8000) ]
+    stream_handler.CodecsUpdated(jt2.dbusify_codecs(new_codecs))
 
     e = q.expect('stream-iq', iq_type='set', predicate=lambda x:
         xpath.queryForNodes("/iq/jingle[@action='description-info']",
             x.stanza))
-    assert xpath.queryForNodes("/iq/jingle/content[@name='stream1']/description/payload-type[@name='A']", e.stanza) is not None
-
-    # Now the remote end decides to change something
+    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])
+
+    # Instead, the remote end decides to change the clockrate of the third codec.
+    new_codecs = [ ('GSM', 3, 8000), ('PCMA', 8, 8000), ('PCMU', 0, 1600) ]
+    # As per the XEP, it only sends the ones which have changed.
+    c = new_codecs[2]
     node = jp.SetIq(jt2.peer, jt2.jid, [
         jp.Jingle(jt2.sid, jt2.peer, 'description-info', [
             jp.Content('stream1', 'initiator', 'both', [
                 jp.Description('audio', [
-                    jp.PayloadType(name, str(rate), str(id)) for
-                        (name, id, rate) in jt2.audio_codecs ]) ]) ]) ])
+                    jp.PayloadType(c[0], str(c[2]), str(c[1])) ]) ]) ]) ])
     stream.send(jp.xml(node))
 
+    # Gabble should patch its idea of the remote codecs with the update it just
+    # got, and emit SetRemoteCodecs for them all.
     e = q.expect('dbus-signal', signal='SetRemoteCodecs')
-    assert jt2.audio_codecs == [ (name, id, rate)
-        for id, name, type, rate, channels, parameters in unwrap(e.args[0]) ], \
-        (jt2.audio_codecs, unwrap(e.args[0]))
+    new_codecs_dbus = unwrap(jt2.dbusify_codecs(new_codecs))
+    announced = unwrap(e.args[0])
+    assert new_codecs_dbus == announced, (new_codecs_dbus, announced)
 
     # We close the session by removing the stream
     media_iface.RemoveStreams([id1])
-- 
1.5.6.5




More information about the telepathy-commits mailing list