Change in osmo-mgw[master]: mgcp_network: translate payload type numbers in RTP packets

This is merely a historical archive of years 2008-2021, before the migration to mailman3.

A maintained and still updated list archive can be found at https://lists.osmocom.org/hyperkitty/list/gerrit-log@lists.osmocom.org/.

Harald Welte gerrit-no-reply at lists.osmocom.org
Tue Jul 31 17:18:15 UTC 2018


Harald Welte has submitted this change and it was merged. ( https://gerrit.osmocom.org/10172 )

Change subject: mgcp_network: translate payload type numbers in RTP packets
......................................................................

mgcp_network: translate payload type numbers in RTP packets

Since no transcoding is in place osmo-mgw forwards the incoming rtp
packets as they are (there may be minor modifications of the header) from
an ingress connection to an egress connection.

This works without problems as long as both connections use the same
payload type. For IANA defined fixed payload type numbers this is
usually the case, but for dynemic payload type numbers both ends may set
up the same codecs but with different payload type numbers.

When different payload type numbers are set up, and the packet is passed
through without modification, it will have the wrong payload type when
it is sent. The receiving end may then toss the packet since it expects
packets with the payload type it has configured.

The machanism, which is introduced with this patch looks up actual codec
inside the struct data of the ingress connection and then looks for the
matching codec in the struct data of the egress connection. When it
finds the codec there it looks up the payload type of this codec. The
header of the RTP packet is then patched with the correct payoad type.

- Add function mgcp_codec_pt_translate() to look up the payload type
- Add unit-test for function mgcp_codec_pt_translate()
- Add payload type translation to mgcp_network.c

Change-Id: I3a874e59fa07bcc2a67c376cafa197360036f539
Related: OS#2728
Related: OS#3384
---
M include/osmocom/mgcp/mgcp_codec.h
M src/libosmo-mgcp/mgcp_codec.c
M src/libosmo-mgcp/mgcp_network.c
M tests/mgcp/mgcp_test.c
4 files changed, 203 insertions(+), 0 deletions(-)

Approvals:
  Harald Welte: Looks good to me, approved
  Jenkins Builder: Verified



diff --git a/include/osmocom/mgcp/mgcp_codec.h b/include/osmocom/mgcp/mgcp_codec.h
index f8d5e70..334913b 100644
--- a/include/osmocom/mgcp/mgcp_codec.h
+++ b/include/osmocom/mgcp/mgcp_codec.h
@@ -4,3 +4,4 @@
 void mgcp_codec_reset_all(struct mgcp_conn_rtp *conn);
 int mgcp_codec_add(struct mgcp_conn_rtp *conn, int payload_type, const char *audio_name);
 int mgcp_codec_decide(struct mgcp_conn_rtp *conn);
+int mgcp_codec_pt_translate(struct mgcp_conn_rtp *conn_src, struct mgcp_conn_rtp *conn_dst, int payload_type);
diff --git a/src/libosmo-mgcp/mgcp_codec.c b/src/libosmo-mgcp/mgcp_codec.c
index 2ce90dd..55be554 100644
--- a/src/libosmo-mgcp/mgcp_codec.c
+++ b/src/libosmo-mgcp/mgcp_codec.c
@@ -341,3 +341,70 @@
 
 	return -EINVAL;
 }
+
+/* Compare two codecs, all parameters must match up, except for the payload type
+ * number. */
+static bool codecs_cmp(struct mgcp_rtp_codec *codec_a, struct mgcp_rtp_codec *codec_b)
+{
+	if (codec_a->rate != codec_b->rate)
+		return false;
+	if (codec_a->channels != codec_b->channels)
+		return false;
+	if (codec_a->frame_duration_num != codec_b->frame_duration_num)
+		return false;
+	if (codec_a->frame_duration_den != codec_b->frame_duration_den)
+		return false;
+	if (strcmp(codec_a->audio_name, codec_b->audio_name))
+		return false;
+	if (strcmp(codec_a->subtype_name, codec_b->subtype_name))
+		return false;
+
+	return true;
+}
+
+/*! Translate a given payload type number that belongs to the packet of a
+ *  source connection to the equivalent payload type number that matches the
+ *  configuration of a destination connection.
+ *  \param[in] conn_src related source rtp-connection.
+ *  \param[in] conn_dst related destination rtp-connection.
+ *  \param[in] payload_type number from the source packet or source connection.
+ *  \returns translated payload type number on success, -EINVAL on failure. */
+int mgcp_codec_pt_translate(struct mgcp_conn_rtp *conn_src, struct mgcp_conn_rtp *conn_dst, int payload_type)
+{
+	struct mgcp_rtp_end *rtp_src;
+	struct mgcp_rtp_end *rtp_dst;
+	struct mgcp_rtp_codec *codec_src = NULL;
+	struct mgcp_rtp_codec *codec_dst = NULL;
+	unsigned int i;
+	unsigned int codecs_assigned;
+
+	rtp_src = &conn_src->end;
+	rtp_dst = &conn_dst->end;
+
+	/* Find the codec information that is used on the source side */
+	codecs_assigned = rtp_src->codecs_assigned;
+	OSMO_ASSERT(codecs_assigned <= MGCP_MAX_CODECS);
+	for (i = 0; i < codecs_assigned; i++) {
+		if (payload_type == rtp_src->codecs[i].payload_type) {
+			codec_src = &rtp_src->codecs[i];
+			break;
+		}
+	}
+	if (!codec_src)
+		return -EINVAL;
+
+	/* Use the codec infrmation from the source and try to find the
+	 * equivalent of it on the destination side */
+	codecs_assigned = rtp_dst->codecs_assigned;
+	OSMO_ASSERT(codecs_assigned <= MGCP_MAX_CODECS);
+	for (i = 0; i < codecs_assigned; i++) {
+		if (codecs_cmp(codec_src, &rtp_dst->codecs[i])) {
+			codec_dst = &rtp_dst->codecs[i];
+			break;
+		}
+	}
+	if (!codec_dst)
+		return -EINVAL;
+
+	return codec_dst->payload_type;
+}
diff --git a/src/libosmo-mgcp/mgcp_network.c b/src/libosmo-mgcp/mgcp_network.c
index 3ac93be..1b1867a 100644
--- a/src/libosmo-mgcp/mgcp_network.c
+++ b/src/libosmo-mgcp/mgcp_network.c
@@ -40,8 +40,10 @@
 #include <osmocom/mgcp/osmux.h>
 #include <osmocom/mgcp/mgcp_conn.h>
 #include <osmocom/mgcp/mgcp_endp.h>
+#include <osmocom/mgcp/mgcp_codec.h>
 #include <osmocom/mgcp/debug.h>
 
+
 #define RTP_SEQ_MOD		(1 << 16)
 #define RTP_MAX_DROPOUT		3000
 #define RTP_MAX_MISORDER	100
@@ -474,6 +476,28 @@
 	state->stats.max_seq = seq;
 }
 
+/* There may be different payload type numbers negotiated for two connections.
+ * Patch the payload type of an RTP packet so that it uses the payload type
+ * that is valid for the destination connection (conn_dst) */
+static int mgcp_patch_pt(struct mgcp_conn_rtp *conn_src,
+			 struct mgcp_conn_rtp *conn_dst, char *data, int len)
+{
+	struct rtp_hdr *rtp_hdr;
+	uint8_t pt_in;
+	int pt_out;
+
+	OSMO_ASSERT(len >= sizeof(struct rtp_hdr));
+	rtp_hdr = (struct rtp_hdr *)data;
+
+	pt_in = rtp_hdr->payload_type;
+	pt_out = mgcp_codec_pt_translate(conn_src, conn_dst, pt_in);
+	if (pt_out < 0)
+		return -EINVAL;
+
+	rtp_hdr->payload_type = (uint8_t) pt_out;
+	return 0;
+}
+
 /* The RFC 3550 Appendix A assumes there are multiple sources but
  * some of the supported endpoints (e.g. the nanoBTS) can only handle
  * one source and this code will patch RTP header to appear as if there
@@ -665,6 +689,7 @@
 	struct mgcp_rtp_end *rtp_end;
 	struct mgcp_rtp_state *rtp_state;
 	char *dest_name;
+	int rc;
 
 	OSMO_ASSERT(conn_src);
 	OSMO_ASSERT(conn_dst);
@@ -684,6 +709,21 @@
 	     ENDPOINT_NUMBER(endp), tcfg->audio_loop, conn_src->conn->mode,
 	     conn_src->conn->mode == MGCP_CONN_LOOPBACK ? " (loopback)" : "");
 
+	/* FIXME: It is legal that the payload type on the egress connection is
+	 * different from the payload type that has been negotiated on the
+	 * ingress connection. Essentially the codecs are the same so we can
+	 * match them and patch the payload type. However, if we can not find
+	 * the codec pendant (everything ist equal except the PT), we are of
+	 * course unable to patch the payload type. A situation like this
+	 * should not occur if transcoding is consequently avoided. Until
+	 * we have transcoding support in osmo-mgw we can not resolve this. */
+	rc = mgcp_patch_pt(conn_src, conn_dst, buf, len);
+	if (rc < 0) {
+		LOGP(DRTP, LOGL_ERROR,
+		     "endpoint:0x%x can not patch PT because no suitable egress codec was found.\n",
+		     ENDPOINT_NUMBER(endp));
+	}
+
 	/* Note: In case of loopback configuration, both, the source and the
 	 * destination will point to the same connection. */
 	rtp_end = &conn_dst->end;
diff --git a/tests/mgcp/mgcp_test.c b/tests/mgcp/mgcp_test.c
index 56d0cee..df6ea2f 100644
--- a/tests/mgcp/mgcp_test.c
+++ b/tests/mgcp/mgcp_test.c
@@ -1616,6 +1616,100 @@
 	OSMO_ASSERT(check_local_cx_options(ctx, ",,,") == -1);
 }
 
+static void test_mgcp_codec_pt_translate_pars(struct mgcp_rtp_codec *c)
+{
+	c->rate = 8000;
+	c->channels = 1;
+	c->frame_duration_num = 23;
+	c->frame_duration_den = 42;
+}
+
+static void test_mgcp_codec_pt_translate(void)
+{
+	struct mgcp_conn_rtp conn_src;
+	struct mgcp_conn_rtp conn_dst;
+	int pt_dst;
+
+	/* Setup a realistic set of codec configurations on both
+	 * ends. AMR and HR will use different payload types. PCMU
+	 * must use 0 on both ends since this is not a dynamic payload
+	 * type */
+	test_mgcp_codec_pt_translate_pars(&conn_src.end.codecs[0]);
+	test_mgcp_codec_pt_translate_pars(&conn_dst.end.codecs[0]);
+	test_mgcp_codec_pt_translate_pars(&conn_src.end.codecs[1]);
+	test_mgcp_codec_pt_translate_pars(&conn_dst.end.codecs[1]);
+	test_mgcp_codec_pt_translate_pars(&conn_src.end.codecs[2]);
+	test_mgcp_codec_pt_translate_pars(&conn_dst.end.codecs[2]);
+	conn_src.end.codecs[0].payload_type = 112;
+	conn_dst.end.codecs[0].payload_type = 96;
+	conn_src.end.codecs[1].payload_type = 0;
+	conn_dst.end.codecs[1].payload_type = 0;
+	conn_src.end.codecs[2].payload_type = 111;
+	conn_dst.end.codecs[2].payload_type = 97;
+	conn_src.end.codecs[0].audio_name = "AMR/8000/1";
+	conn_dst.end.codecs[0].audio_name = "AMR/8000/1";
+	conn_src.end.codecs[1].audio_name = "PCMU/8000/1";
+	conn_dst.end.codecs[1].audio_name = "PCMU/8000/1";
+	conn_src.end.codecs[2].audio_name = "GSM-HR-08/8000/1";
+	conn_dst.end.codecs[2].audio_name = "GSM-HR-08/8000/1";
+	conn_src.end.codecs[0].subtype_name = "AMR";
+	conn_dst.end.codecs[0].subtype_name = "AMR";
+	conn_src.end.codecs[1].subtype_name = "PCMU";
+	conn_dst.end.codecs[1].subtype_name = "PCMU";
+	conn_src.end.codecs[2].subtype_name = "GSM-HR-08";
+	conn_dst.end.codecs[2].subtype_name = "GSM-HR-08";
+	conn_src.end.codecs_assigned = 3;
+	conn_dst.end.codecs_assigned = 3;
+
+	/* We expect the function to find the PT we must use when we send the
+	 * packet out to the destination. All we know is the context for both
+	 * connections and the payload type from the source packet */
+	pt_dst =
+	    mgcp_codec_pt_translate(&conn_src, &conn_dst,
+				    conn_src.end.codecs[0].payload_type);
+	OSMO_ASSERT(pt_dst == conn_dst.end.codecs[0].payload_type);
+	pt_dst =
+	    mgcp_codec_pt_translate(&conn_src, &conn_dst,
+				    conn_src.end.codecs[1].payload_type);
+	OSMO_ASSERT(pt_dst == conn_dst.end.codecs[1].payload_type);
+	pt_dst =
+	    mgcp_codec_pt_translate(&conn_src, &conn_dst,
+				    conn_src.end.codecs[2].payload_type);
+	OSMO_ASSERT(pt_dst == conn_dst.end.codecs[2].payload_type);
+
+	/* Try some constellations that must fail */
+	pt_dst = mgcp_codec_pt_translate(&conn_src, &conn_dst, 123);
+	OSMO_ASSERT(pt_dst == -EINVAL);
+	conn_src.end.codecs_assigned = 0;
+	conn_dst.end.codecs_assigned = 3;
+	pt_dst =
+	    mgcp_codec_pt_translate(&conn_src, &conn_dst,
+				    conn_src.end.codecs[0].payload_type);
+	OSMO_ASSERT(pt_dst == -EINVAL);
+	pt_dst =
+	    mgcp_codec_pt_translate(&conn_src, &conn_dst,
+				    conn_src.end.codecs[1].payload_type);
+	OSMO_ASSERT(pt_dst == -EINVAL);
+	pt_dst =
+	    mgcp_codec_pt_translate(&conn_src, &conn_dst,
+				    conn_src.end.codecs[2].payload_type);
+	OSMO_ASSERT(pt_dst == -EINVAL);
+	conn_src.end.codecs_assigned = 3;
+	conn_dst.end.codecs_assigned = 0;
+	pt_dst =
+	    mgcp_codec_pt_translate(&conn_src, &conn_dst,
+				    conn_src.end.codecs[0].payload_type);
+	OSMO_ASSERT(pt_dst == -EINVAL);
+	pt_dst =
+	    mgcp_codec_pt_translate(&conn_src, &conn_dst,
+				    conn_src.end.codecs[1].payload_type);
+	OSMO_ASSERT(pt_dst == -EINVAL);
+	pt_dst =
+	    mgcp_codec_pt_translate(&conn_src, &conn_dst,
+				    conn_src.end.codecs[2].payload_type);
+	OSMO_ASSERT(pt_dst == -EINVAL);
+}
+
 int main(int argc, char **argv)
 {
 	void *ctx = talloc_named_const(NULL, 0, "mgcp_test");
@@ -1639,6 +1733,7 @@
 	test_osmux_cid();
 	test_get_lco_identifier();
 	test_check_local_cx_options(ctx);
+	test_mgcp_codec_pt_translate();
 
 	OSMO_ASSERT(talloc_total_size(msgb_ctx) == 0);
 	OSMO_ASSERT(talloc_total_blocks(msgb_ctx) == 1);

-- 
To view, visit https://gerrit.osmocom.org/10172
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings

Gerrit-Project: osmo-mgw
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: I3a874e59fa07bcc2a67c376cafa197360036f539
Gerrit-Change-Number: 10172
Gerrit-PatchSet: 4
Gerrit-Owner: dexter <pmaier at sysmocom.de>
Gerrit-Reviewer: Harald Welte <laforge at gnumonks.org>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: Neels Hofmeyr <nhofmeyr at sysmocom.de>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.osmocom.org/pipermail/gerrit-log/attachments/20180731/dcde61b6/attachment.htm>


More information about the gerrit-log mailing list