mgcp_codec: refactor payload type converstion
in mgcp_codec.c we have a function mgcp_codec_pt_translate that is used
to find the equivalent payload type number on an opposite connection.
This is quite specific and the resulting payload type number may still
belong to a codec that might require some form of conversion.
Lets refactor this so that the function just finds a convertible codec
for a given connection. This also opens up other usecases. The payload
type conversion like we did it before can then be done with a few lines
in mgcp_network.c
Related: OS#5461
Change-Id: I085260a2ca8cfecdb58656b7a046c536189e238d
diff --git a/include/osmocom/mgcp/mgcp_codec.h b/include/osmocom/mgcp/mgcp_codec.h
index 994d770..1f29b08 100644
--- a/include/osmocom/mgcp/mgcp_codec.h
+++ b/include/osmocom/mgcp/mgcp_codec.h
@@ -14,8 +14,9 @@
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, const struct mgcp_codec_param *param);
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);
+struct mgcp_rtp_codec *mgcp_codec_find_convertible(struct mgcp_conn_rtp *conn, struct mgcp_rtp_codec *codec);
const struct mgcp_rtp_codec *mgcp_codec_pt_find_by_subtype_name(struct mgcp_conn_rtp *conn,
const char *subtype_name, unsigned int match_nr);
bool mgcp_codec_amr_align_mode_is_indicated(const struct mgcp_rtp_codec *codec);
bool mgcp_codec_amr_is_octet_aligned(const struct mgcp_rtp_codec *codec);
+struct mgcp_rtp_codec *mgcp_codec_from_pt(struct mgcp_conn_rtp *conn, int payload_type);
diff --git a/src/libosmo-mgcp/mgcp_codec.c b/src/libosmo-mgcp/mgcp_codec.c
index ccb5d77..9086047 100644
--- a/src/libosmo-mgcp/mgcp_codec.c
+++ b/src/libosmo-mgcp/mgcp_codec.c
@@ -425,63 +425,42 @@
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)
+/*! For a given codec, find a convertible codec in the given connection.
+ * \param[in] conn connection to search for a convertible codec
+ * \param[in] codec for which a convertible codec shall be found.
+ * \returns codec on success, -NULL on failure. */
+struct mgcp_rtp_codec *mgcp_codec_find_convertible(struct mgcp_conn_rtp *conn, struct mgcp_rtp_codec *codec)
{
- 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;
+ struct mgcp_rtp_end *rtp_end;
unsigned int i;
unsigned int codecs_assigned;
+ struct mgcp_rtp_codec *codec_convertible = NULL;
- 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;
+ rtp_end = &conn->end;
/* Use the codec information from the source and try to find the equivalent of it on the destination side. In
* the first run we will look for an exact match. */
- codecs_assigned = rtp_dst->codecs_assigned;
+ codecs_assigned = rtp_end->codecs_assigned;
OSMO_ASSERT(codecs_assigned <= MGCP_MAX_CODECS);
for (i = 0; i < codecs_assigned; i++) {
- if (codecs_same(codec_src, &rtp_dst->codecs[i])) {
- codec_dst = &rtp_dst->codecs[i];
+ if (codecs_same(codec, &rtp_end->codecs[i])) {
+ codec_convertible = &rtp_end->codecs[i];
break;
}
}
/* In case we weren't able to find an exact match, we will try to find a match that is the same codec, but the
* payload format may be different. This alternative will require a frame format conversion (i.e. AMR bwe->oe) */
- if (!codec_dst) {
+ if (!codec_convertible) {
for (i = 0; i < codecs_assigned; i++) {
- if (codecs_convertible(codec_src, &rtp_dst->codecs[i])) {
- codec_dst = &rtp_dst->codecs[i];
+ if (codecs_convertible(codec, &rtp_end->codecs[i])) {
+ codec_convertible = &rtp_end->codecs[i];
break;
}
}
}
- if (!codec_dst)
- return -EINVAL;
-
- return codec_dst->payload_type;
+ return codec_convertible;
}
/* Find the payload type number configured for a specific codec by SDP.
@@ -508,3 +487,26 @@
}
return NULL;
}
+
+/*! Lookup a codec that is assigned to a connection by its payload type number.
+ * \param[in] conn related rtp-connection.
+ * \param[in] payload_type number of the codec to look up.
+ * \returns pointer to codec struct on success, NULL on failure. */
+struct mgcp_rtp_codec *mgcp_codec_from_pt(struct mgcp_conn_rtp *conn, int payload_type)
+{
+ struct mgcp_rtp_end *rtp_end = &conn->end;
+ unsigned int codecs_assigned = rtp_end->codecs_assigned;
+ struct mgcp_rtp_codec *codec = NULL;
+ size_t i;
+
+ OSMO_ASSERT(codecs_assigned <= MGCP_MAX_CODECS);
+
+ for (i = 0; i < codecs_assigned; i++) {
+ if (payload_type == rtp_end->codecs[i].payload_type) {
+ codec = &rtp_end->codecs[i];
+ break;
+ }
+ }
+
+ return codec;
+}
diff --git a/src/libosmo-mgcp/mgcp_network.c b/src/libosmo-mgcp/mgcp_network.c
index af9ae99..ce08dd4 100644
--- a/src/libosmo-mgcp/mgcp_network.c
+++ b/src/libosmo-mgcp/mgcp_network.c
@@ -493,27 +493,31 @@
/* 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, struct msgb *msg)
+static int mgcp_patch_pt(struct mgcp_conn_rtp *conn_src, struct mgcp_conn_rtp *conn_dst, struct msgb *msg)
{
struct rtp_hdr *rtp_hdr;
- uint8_t pt_in;
- int pt_out;
+ struct mgcp_rtp_codec *codec_src;
+ struct mgcp_rtp_codec *codec_dst;
if (msgb_length(msg) < sizeof(struct rtp_hdr)) {
LOG_CONN_RTP(conn_src, LOGL_ERROR, "RTP packet too short (%u < %zu)\n",
msgb_length(msg), sizeof(struct rtp_hdr));
return -EINVAL;
}
-
rtp_hdr = (struct rtp_hdr *)msgb_data(msg);
- pt_in = rtp_hdr->payload_type;
- pt_out = mgcp_codec_pt_translate(conn_src, conn_dst, pt_in);
- if (pt_out < 0)
+ /* Find the codec information that is used on the source side */
+ codec_src = mgcp_codec_from_pt(conn_src, rtp_hdr->payload_type);
+ if (!codec_src)
return -EINVAL;
- rtp_hdr->payload_type = (uint8_t) pt_out;
+ /* Lookup a suitable codec in the destination connection. (The codec must be of the same type or at least
+ * convertible) */
+ codec_dst = mgcp_codec_find_convertible(conn_dst, codec_src);
+ if (!codec_dst)
+ return -EINVAL;
+
+ rtp_hdr->payload_type = (uint8_t) codec_dst->payload_type;
return 0;
}
diff --git a/tests/mgcp/mgcp_test.c b/tests/mgcp/mgcp_test.c
index 8f081a9..bc2dae0 100644
--- a/tests/mgcp/mgcp_test.c
+++ b/tests/mgcp/mgcp_test.c
@@ -1769,26 +1769,26 @@
.amr_octet_aligned_present = false,
};
-struct testcase_mgcp_codec_pt_translate_codec {
+struct testcase_mgcp_codec_find_convertible_codec {
int payload_type;
const char *audio_name;
const struct mgcp_codec_param *param;
int expect_rc;
};
-struct testcase_mgcp_codec_pt_translate_expect {
+struct testcase_mgcp_codec_find_convertible_expect {
bool end;
int payload_type_map[2];
};
-struct testcase_mgcp_codec_pt_translate {
+struct testcase_mgcp_codec_find_convertible {
const char *descr;
/* two conns on an endpoint, each with N configured codecs */
- struct testcase_mgcp_codec_pt_translate_codec codecs[2][10];
- struct testcase_mgcp_codec_pt_translate_expect expect[32];
+ struct testcase_mgcp_codec_find_convertible_codec codecs[2][10];
+ struct testcase_mgcp_codec_find_convertible_expect expect[32];
};
-static const struct testcase_mgcp_codec_pt_translate test_mgcp_codec_pt_translate_cases[] = {
+static const struct testcase_mgcp_codec_find_convertible test_mgcp_codec_find_convertible_cases[] = {
{
.descr = "same order, but differing payload type numbers",
.codecs = {
@@ -2044,14 +2044,39 @@
},
};
-static void test_mgcp_codec_pt_translate(void)
+static int pt_translate(struct mgcp_conn_rtp *conn, unsigned int index_src, unsigned int index_dst, int payload_type)
+{
+ struct mgcp_rtp_codec *codec_src = NULL;
+ struct mgcp_rtp_codec *codec_dst = NULL;
+ struct mgcp_conn_rtp *conn_src = &conn[index_src];
+ struct mgcp_conn_rtp *conn_dst = &conn[index_dst];
+
+ /* Find the codec information that is used on the source side */
+ codec_src = mgcp_codec_from_pt(conn_src, payload_type);
+ if (!codec_src) {
+ printf(" - mgcp_codec_from_pt(conn%u, %d) -> NO RESULT\n", index_src, payload_type);
+ return -EINVAL;
+ }
+ printf(" - mgcp_codec_from_pt(conn%u, %d) -> %s\n", index_src, payload_type, codec_src->subtype_name);
+
+ codec_dst = mgcp_codec_find_convertible(conn_dst, codec_src);
+ if (!codec_dst) {
+ printf(" - mgcp_codec_find_convertible(conn%u, %s) -> NO RESULT\n", index_dst, codec_src->subtype_name);
+ return -EINVAL;
+ }
+ printf(" - mgcp_codec_find_convertible(conn%u, %s) -> %s -> %u\n",
+ index_dst, codec_src->subtype_name, codec_dst->subtype_name, codec_dst->payload_type);
+ return codec_dst->payload_type;
+}
+
+static void test_mgcp_codec_find_convertible(void)
{
int i;
bool ok = true;
- printf("\nTesting mgcp_codec_pt_translate()\n");
+ printf("\nTesting mgcp_codec_find_convertible()\n");
- for (i = 0; i < ARRAY_SIZE(test_mgcp_codec_pt_translate_cases); i++) {
- const struct testcase_mgcp_codec_pt_translate *t = &test_mgcp_codec_pt_translate_cases[i];
+ for (i = 0; i < ARRAY_SIZE(test_mgcp_codec_find_convertible_cases); i++) {
+ const struct testcase_mgcp_codec_find_convertible *t = &test_mgcp_codec_find_convertible_cases[i];
struct mgcp_conn_rtp conn[2] = {};
int rc;
int conn_i;
@@ -2062,7 +2087,7 @@
for (conn_i = 0; conn_i < 2; conn_i++) {
printf(" - add codecs on conn%d:\n", conn_i);
for (c = 0; c < ARRAY_SIZE(t->codecs[conn_i]); c++) {
- const struct testcase_mgcp_codec_pt_translate_codec *codec = &t->codecs[conn_i][c];
+ const struct testcase_mgcp_codec_find_convertible_codec *codec = &t->codecs[conn_i][c];
if (!codec->audio_name)
break;
@@ -2086,15 +2111,13 @@
}
for (c = 0; c < ARRAY_SIZE(t->expect); c++) {
- const struct testcase_mgcp_codec_pt_translate_expect *expect = &t->expect[c];
+ const struct testcase_mgcp_codec_find_convertible_expect *expect = &t->expect[c];
int result;
if (expect->end)
break;
- result = mgcp_codec_pt_translate(&conn[0], &conn[1], expect->payload_type_map[0]);
- printf(" - mgcp_codec_pt_translate(conn0, conn1, %d) -> %d\n",
- expect->payload_type_map[0], result);
+ result = pt_translate(conn, 0, 1, expect->payload_type_map[0]);
if (result != expect->payload_type_map[1]) {
printf(" ERROR: expected -> %d\n", expect->payload_type_map[1]);
ok = false;
@@ -2104,9 +2127,7 @@
if (expect->payload_type_map[1] < 0)
continue;
- result = mgcp_codec_pt_translate(&conn[1], &conn[0], expect->payload_type_map[1]);
- printf(" - mgcp_codec_pt_translate(conn1, conn0, %d) -> %d\n",
- expect->payload_type_map[1], result);
+ result = pt_translate(conn, 1, 0, expect->payload_type_map[1]);
if (result != expect->payload_type_map[0]) {
printf(" ERROR: expected -> %d\n", expect->payload_type_map[0]);
ok = false;
@@ -2274,7 +2295,7 @@
test_osmux_cid();
test_get_lco_identifier();
test_check_local_cx_options(ctx);
- test_mgcp_codec_pt_translate();
+ test_mgcp_codec_find_convertible();
test_conn_id_matching();
test_e1_trunk_nr_from_epname();
test_mgcp_is_rtp_dummy_payload();
diff --git a/tests/mgcp/mgcp_test.ok b/tests/mgcp/mgcp_test.ok
index 8b3b3d1..4ea9a34 100644
--- a/tests/mgcp/mgcp_test.ok
+++ b/tests/mgcp/mgcp_test.ok
Binary files differ