neels has submitted this change. ( https://gerrit.osmocom.org/c/osmo-mgw/+/35526?usp=email ) Change subject: Revert "drop (now) unused code" ...................................................................... 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/17... --- 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(-) Approvals: laforge: Looks good to me, approved Jenkins Builder: Verified 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 https://gerrit.osmocom.org/c/osmo-mgw/+/35526?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/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