neels submitted this change.

View Change


Approvals: laforge: Looks good to me, approved Jenkins Builder: Verified
Revert "drop (now) unused code"

This reverts commit 2b30cbdfa83c20045ef92f53e88441dda185591b.

Reason for revert: Older versions of osmo-msc were actually calling map_codec_to_pt().

Change-Id: Ifff31012b327d40ed0b1559d5cf4f320784a4061
Related: https://jenkins.osmocom.org/jenkins/job/Osmocom-build-tags-against-master/1792/console
---
M TODO-RELEASE
M include/osmocom/mgcp_client/mgcp_client.h
M src/libosmo-mgcp-client/mgcp_client.c
M tests/mgcp_client/mgcp_client_test.c
M tests/mgcp_client/mgcp_client_test.err
M tests/mgcp_client/mgcp_client_test.ok
6 files changed, 191 insertions(+), 5 deletions(-)

diff --git a/TODO-RELEASE b/TODO-RELEASE
index f1a702f..964ffe1 100644
--- a/TODO-RELEASE
+++ b/TODO-RELEASE
@@ -38,8 +38,3 @@
libosmo-mgcp-client deprecate public API New code should no longer use codecs[], instead use ptmap[].codec. There
is backwards compat code that moves codecs[] entries, if any, over to
ptmap[], so callers may migrate at own leisure.
-libosmo-mgcp-client remove public API Since codecs[] has been deprecated in favor of ptmap[], there is no use
- for the following functions; all known callers have been within
- osmo-mgw.git:
- map_codec_to_pt()
- map_pt_to_codec()
diff --git a/include/osmocom/mgcp_client/mgcp_client.h b/include/osmocom/mgcp_client/mgcp_client.h
index e1748a6..1d33690 100644
--- a/include/osmocom/mgcp_client/mgcp_client.h
+++ b/include/osmocom/mgcp_client/mgcp_client.h
@@ -119,5 +119,9 @@
}

enum mgcp_codecs map_str_to_codec(const char *str);
+unsigned int map_codec_to_pt(const struct ptmap *ptmap, unsigned int ptmap_len,
+ enum mgcp_codecs codec);
+enum mgcp_codecs map_pt_to_codec(struct ptmap *ptmap, unsigned int ptmap_len,
+ unsigned int pt);

const char *mgcp_client_name(const struct mgcp_client *mgcp);
diff --git a/src/libosmo-mgcp-client/mgcp_client.c b/src/libosmo-mgcp-client/mgcp_client.c
index bfe69e6..489ce69 100644
--- a/src/libosmo-mgcp-client/mgcp_client.c
+++ b/src/libosmo-mgcp-client/mgcp_client.c
@@ -105,6 +105,94 @@
return -1;
}

+/* Check the ptmap for illegal mappings */
+static int check_ptmap(const struct ptmap *ptmap)
+{
+ /* Check if there are mappings that leave the IANA assigned dynamic
+ * payload type range. Under normal conditions such mappings should
+ * not occur */
+
+ /* Its ok to have a 1:1 mapping in the statically defined
+ * range, this won't hurt */
+ if (ptmap->codec == ptmap->pt)
+ return 0;
+
+ if (ptmap->codec < 96 || ptmap->codec > 127)
+ goto error;
+ if (ptmap->pt < 96 || ptmap->pt > 127)
+ goto error;
+
+ return 0;
+error:
+ LOGP(DLMGCP, LOGL_ERROR,
+ "ptmap contains illegal mapping: codec=%u maps to pt=%u\n",
+ ptmap->codec, ptmap->pt);
+ return -1;
+}
+
+/*! Map a codec to a payload type.
+ * \ptmap[in] payload pointer to payload type map with specified payload types.
+ * \ptmap[in] ptmap_len length of the payload type map.
+ * \ptmap[in] codec the codec for which the payload type should be looked up.
+ * \returns assigned payload type */
+unsigned int map_codec_to_pt(const struct ptmap *ptmap, unsigned int ptmap_len,
+ enum mgcp_codecs codec)
+{
+ unsigned int i;
+
+ /*! Note: If the payload type map is empty or the codec is not found
+ * in the map, then a 1:1 mapping is performed. If the codec falls
+ * into the statically defined range or if the mapping table isself
+ * tries to map to the statically defined range, then the mapping
+ * is also ignored and a 1:1 mapping is performed instead. */
+
+ /* we may return the codec directly since enum mgcp_codecs directly
+ * corresponds to the statically assigned payload types */
+ if (codec < 96 || codec > 127)
+ return codec;
+
+ for (i = 0; i < ptmap_len; i++) {
+ /* Skip illegal map entries */
+ if (check_ptmap(ptmap) == 0 && ptmap->codec == codec)
+ return ptmap->pt;
+ ptmap++;
+ }
+
+ /* If nothing is found, do not perform any mapping */
+ return codec;
+}
+
+/*! Map a payload type to a codec.
+ * \ptmap[in] payload pointer to payload type map with specified payload types.
+ * \ptmap[in] ptmap_len length of the payload type map.
+ * \ptmap[in] payload type for which the codec should be looked up.
+ * \returns codec that corresponds to the specified payload type */
+enum mgcp_codecs map_pt_to_codec(struct ptmap *ptmap, unsigned int ptmap_len,
+ unsigned int pt)
+{
+ unsigned int i;
+
+ /*! Note: If the payload type map is empty or the payload type is not
+ * found in the map, then a 1:1 mapping is performed. If the payload
+ * type falls into the statically defined range or if the mapping
+ * table isself tries to map to the statically defined range, then
+ * the mapping is also ignored and a 1:1 mapping is performed
+ * instead. */
+
+ /* See also note in map_codec_to_pt() */
+ if (pt < 96 || pt > 127)
+ return pt;
+
+ for (i = 0; i < ptmap_len; i++) {
+ if (check_ptmap(ptmap) == 0 && ptmap->pt == pt)
+ return ptmap->codec;
+ ptmap++;
+ }
+
+ /* If nothing is found, do not perform any mapping */
+ return pt;
+}
+
static void _mgcp_client_conf_init(struct mgcp_client_conf *conf)
{
/* NULL and -1 default to MGCP_CLIENT_*_DEFAULT values */
diff --git a/tests/mgcp_client/mgcp_client_test.c b/tests/mgcp_client/mgcp_client_test.c
index 95a742f..9b356c3 100644
--- a/tests/mgcp_client/mgcp_client_test.c
+++ b/tests/mgcp_client/mgcp_client_test.c
@@ -571,6 +571,57 @@
OSMO_ASSERT(map_str_to_codec("AMR-WB####################################################################################################################") == -1);
}

+static void test_map_codec_to_pt_and_map_pt_to_codec(void)
+{
+ struct ptmap ptmap[10];
+ unsigned int ptmap_len;
+ unsigned int i;
+
+ ptmap[0].codec = CODEC_GSMEFR_8000_1;
+ ptmap[0].pt = 96;
+ ptmap[1].codec = CODEC_GSMHR_8000_1;
+ ptmap[1].pt = 97;
+ ptmap[2].codec = CODEC_AMR_8000_1;
+ ptmap[2].pt = 98;
+ ptmap[3].codec = CODEC_AMRWB_16000_1;
+ ptmap[3].pt = 99;
+ ptmap_len = 4;
+
+ /* Mappings that are covered by the table */
+ for (i = 0; i < ptmap_len; i++)
+ printf(" %u => %u\n", ptmap[i].codec, map_codec_to_pt(ptmap, ptmap_len, ptmap[i].codec));
+ for (i = 0; i < ptmap_len; i++)
+ printf(" %u <= %u\n", ptmap[i].pt, map_pt_to_codec(ptmap, ptmap_len, ptmap[i].pt));
+ printf("\n");
+
+ /* Map some codecs/payload types from the static range, result must
+ * always be a 1:1 mapping */
+ printf(" %u => %u\n", CODEC_PCMU_8000_1, map_codec_to_pt(ptmap, ptmap_len, CODEC_PCMU_8000_1));
+ printf(" %u => %u\n", CODEC_GSM_8000_1, map_codec_to_pt(ptmap, ptmap_len, CODEC_GSM_8000_1));
+ printf(" %u => %u\n", CODEC_PCMA_8000_1, map_codec_to_pt(ptmap, ptmap_len, CODEC_PCMA_8000_1));
+ printf(" %u => %u\n", CODEC_G729_8000_1, map_codec_to_pt(ptmap, ptmap_len, CODEC_G729_8000_1));
+ printf(" %u <= %u\n", CODEC_PCMU_8000_1, map_pt_to_codec(ptmap, ptmap_len, CODEC_PCMU_8000_1));
+ printf(" %u <= %u\n", CODEC_GSM_8000_1, map_pt_to_codec(ptmap, ptmap_len, CODEC_GSM_8000_1));
+ printf(" %u <= %u\n", CODEC_PCMA_8000_1, map_pt_to_codec(ptmap, ptmap_len, CODEC_PCMA_8000_1));
+ printf(" %u <= %u\n", CODEC_G729_8000_1, map_pt_to_codec(ptmap, ptmap_len, CODEC_G729_8000_1));
+ printf("\n");
+
+ /* Try to do mappings from statically defined range to danymic range and vice versa. This
+ * is illegal and should result into a 1:1 mapping */
+ ptmap[3].codec = CODEC_AMRWB_16000_1;
+ ptmap[3].pt = 2;
+ ptmap[4].codec = CODEC_PCMU_8000_1;
+ ptmap[4].pt = 100;
+ ptmap_len = 5;
+
+ /* Apply all mappings again, the illegal ones we defined should result into 1:1 mappings */
+ for (i = 0; i < ptmap_len; i++)
+ printf(" %u => %u\n", ptmap[i].codec, map_codec_to_pt(ptmap, ptmap_len, ptmap[i].codec));
+ for (i = 0; i < ptmap_len; i++)
+ printf(" %u <= %u\n", ptmap[i].pt, map_pt_to_codec(ptmap, ptmap_len, ptmap[i].pt));
+ printf("\n");
+}
+
void test_mgcp_client_e1_epname(void)
{
char *epname;
@@ -857,6 +908,7 @@
test_mgcp_msg();
test_mgcp_client_cancel();
test_sdp_section_start();
+ test_map_codec_to_pt_and_map_pt_to_codec();
test_map_str_to_codec();
test_mgcp_client_e1_epname();

diff --git a/tests/mgcp_client/mgcp_client_test.err b/tests/mgcp_client/mgcp_client_test.err
index 9e98792..94fe351 100644
--- a/tests/mgcp_client/mgcp_client_test.err
+++ b/tests/mgcp_client/mgcp_client_test.err
@@ -128,6 +128,10 @@
body: "some mgcp header data\r\nand header params\r\n\r\nc=IN IP4 \r\n"
DLMGCP Failed to parse MGCP response header (audio ip)
got rc=-22
+DLMGCP ptmap contains illegal mapping: codec=113 maps to pt=2
+DLMGCP ptmap contains illegal mapping: codec=0 maps to pt=100
+DLMGCP ptmap contains illegal mapping: codec=113 maps to pt=2
+DLMGCP ptmap contains illegal mapping: codec=0 maps to pt=100
DLMGCP MGW(mgw) MGCP client: using endpoint domain '@mgw'
DLMGCP MGW(mgw) Cannot compose MGCP e1-endpoint name (ds/e1-15/s-1/su128-0@mgw), rate(128)/offset(0) combination is invalid!
DLMGCP MGW(mgw) Cannot compose MGCP e1-endpoint name (ds/e1-15/s-1/su8-16@mgw), rate(8)/offset(16) combination is invalid!
diff --git a/tests/mgcp_client/mgcp_client_test.ok b/tests/mgcp_client/mgcp_client_test.ok
index b16d1bc..039fbd9 100644
--- a/tests/mgcp_client/mgcp_client_test.ok
+++ b/tests/mgcp_client/mgcp_client_test.ok
@@ -181,6 +181,35 @@
test_sdp_section_start() test [17]:

test_sdp_section_start() test [18]:
+ 110 => 96
+ 111 => 97
+ 112 => 98
+ 113 => 99
+ 96 <= 110
+ 97 <= 111
+ 98 <= 112
+ 99 <= 113
+
+ 0 => 0
+ 3 => 3
+ 8 => 8
+ 18 => 18
+ 0 <= 0
+ 3 <= 3
+ 8 <= 8
+ 18 <= 18
+
+ 110 => 96
+ 111 => 97
+ 112 => 98
+ 113 => 113
+ 0 => 0
+ 96 <= 110
+ 97 <= 111
+ 98 <= 112
+ 2 <= 2
+ 100 <= 100
+
ds/e1-1/s-15/su64-0@mgw
ds/e1-2/s-14/su32-0@mgw
ds/e1-3/s-13/su32-4@mgw

To view, visit change 35526. To unsubscribe, or for help writing mail filters, visit settings.

Gerrit-Project: osmo-mgw
Gerrit-Branch: master
Gerrit-Change-Id: Ifff31012b327d40ed0b1559d5cf4f320784a4061
Gerrit-Change-Number: 35526
Gerrit-PatchSet: 1
Gerrit-Owner: neels <nhofmeyr@sysmocom.de>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: laforge <laforge@osmocom.org>
Gerrit-Reviewer: neels <nhofmeyr@sysmocom.de>
Gerrit-Reviewer: pespin <pespin@sysmocom.de>
Gerrit-MessageType: merged