dexter has uploaded this change for review.
mgcp_client_fsm: allow the same codec multiple times in ptmap
Ih struct mgcp_conn_peer, the API user is supposed to fill in the codecs
he which to used and optionally a payload type map (ptmap) that is used
to assign a user defined payload type number for each codec (in case
ptmap is not used, than the IANA/3GPP assigned payload type numbers
apply).
Unfortunately ptmap works like a simple lookup table. When a payload
type is looked up for a specific codec, then the lookup table is
searched for the codec and the payload type of the first entry that is
found is returned. This works fine as long as the same codec is not
apparing twice, which different payload type numbers.
What seems to be a corner case on the first look turns out to be a
common case with AMR, where a bandwith-efficient mode and an
octet-aligned mode may be specified. Then we need to specify AMR twice,
but with different payload type numbers in ptmap.
So, let's equip the function that does the mapping (map_pt_to_codec)
with a memory, so that it can checkmark entries it has already mapped.
This will allow us to put the same codec type multiple times in ptmap
and as long as the order in ptmap matches the order used in codecs the
mapping will be consistent.
Related: OS#5723
Change-Id: Ie13ce59d3165936a16e16588b4d58f0ce7e0ae67
---
M include/osmocom/mgcp_client/mgcp_client.h
M include/osmocom/mgcp_client/mgcp_client_fsm.h
M src/libosmo-mgcp-client/mgcp_client.c
M tests/mgcp_client/mgcp_client_test.c
M tests/mgcp_client/mgcp_client_test.ok
5 files changed, 113 insertions(+), 8 deletions(-)
git pull ssh://gerrit.osmocom.org:29418/osmo-mgw refs/changes/49/34349/1
diff --git a/include/osmocom/mgcp_client/mgcp_client.h b/include/osmocom/mgcp_client/mgcp_client.h
index 9a0611a..4e54bbe 100644
--- a/include/osmocom/mgcp_client/mgcp_client.h
+++ b/include/osmocom/mgcp_client/mgcp_client.h
@@ -183,7 +183,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 codec) __attribute__((__deprecated__));
+unsigned int map_codec_to_pt2(bool *ptmap_used, 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);
diff --git a/include/osmocom/mgcp_client/mgcp_client_fsm.h b/include/osmocom/mgcp_client/mgcp_client_fsm.h
index ade4f49..0a5b8d5 100644
--- a/include/osmocom/mgcp_client/mgcp_client_fsm.h
+++ b/include/osmocom/mgcp_client/mgcp_client_fsm.h
@@ -35,8 +35,9 @@
/*! Number of codecs in RTP codec list (optional) */
unsigned int codecs_len;
- /*! RTP payload type map (optional, only needed when payload types are
- * used that differ from what IANA/3GPP defines) */
+ /*! RTP payload type map. This array may be used to assign a user defined payload type number to the codecs (see
+ * codecs array above). In case the same codec type (enum mgcp_codecs) is appearing multiple times in codecs.
+ * Then the order in ptmap must match the order in codecs. */
struct ptmap ptmap[MGCP_MAX_CODECS];
/*! RTP payload type map length (optional, only needed when payload
diff --git a/src/libosmo-mgcp-client/mgcp_client.c b/src/libosmo-mgcp-client/mgcp_client.c
index 5df4560..ee65706 100644
--- a/src/libosmo-mgcp-client/mgcp_client.c
+++ b/src/libosmo-mgcp-client/mgcp_client.c
@@ -138,6 +138,20 @@
unsigned int map_codec_to_pt(const struct ptmap *ptmap, unsigned int ptmap_len,
enum mgcp_codecs codec)
{
+ return map_codec_to_pt2(NULL, ptmap, ptmap_len, codec);
+}
+
+/*! Map a codec to a payload type (but not the same entry twice).
+ * \ptmap[inout] ptmap_used array of length ptmap_len to checkmark ptmap
+ * entries which are already used. Must be initialized with
+ * zeros before use. (optional, may be NULL).
+ * \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_pt2(bool *ptmap_used, 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
@@ -153,8 +167,16 @@
for (i = 0; i < ptmap_len; i++) {
/* Skip illegal map entries */
- if (check_ptmap(ptmap) == 0 && ptmap->codec == codec)
+ if (check_ptmap(ptmap) == 0 && ptmap->codec == codec) {
+ if (ptmap_used) {
+ if (ptmap_used[i])
+ goto skip;
+ else
+ ptmap_used[i] = true;
+ }
return ptmap->pt;
+ }
+ skip:
ptmap++;
}
@@ -179,7 +201,7 @@
* the mapping is also ignored and a 1:1 mapping is performed
* instead. */
- /* See also note in map_codec_to_pt() */
+ /* See also note in map_codec_to_pt2() */
if (pt < 96 || pt > 127)
return pt;
@@ -1280,6 +1302,10 @@
const char *codec;
unsigned int pt;
+ /* We may use statically allocated memory here since we intend to work only on mgcp_msg->ptmap arrays, which
+ * have a fixed length of MGCP_MAX_CODECS */
+ bool ptmap_used[MGCP_MAX_CODECS];
+
#define MSGB_PRINTF_OR_RET(FMT, ARGS...) do { \
if (msgb_printf(msg, FMT, ##ARGS) != 0) { \
LOGPMGW(mgcp, LOGL_ERROR, "Message buffer too small, can not generate MGCP message (SDP)\n"); \
@@ -1335,8 +1361,9 @@
MSGB_PRINTF_OR_RET("t=0 0\r\n");
MSGB_PRINTF_OR_RET("m=audio %u RTP/AVP", mgcp_msg->audio_port);
+ memset(ptmap_used, 0, sizeof(ptmap_used));
for (i = 0; i < mgcp_msg->codecs_len; i++) {
- pt = map_codec_to_pt(mgcp_msg->ptmap, mgcp_msg->ptmap_len, mgcp_msg->codecs[i]);
+ pt = map_codec_to_pt2(ptmap_used, mgcp_msg->ptmap, mgcp_msg->ptmap_len, mgcp_msg->codecs[i]);
MSGB_PRINTF_OR_RET(" %u", pt);
}
@@ -1344,11 +1371,12 @@
/* Add optional codec parameters (fmtp) */
if (mgcp_msg->param_present) {
+ memset(ptmap_used, 0, sizeof(ptmap_used));
for (i = 0; i < mgcp_msg->codecs_len; i++) {
/* The following is only applicable for AMR */
if (mgcp_msg->codecs[i] != CODEC_AMR_8000_1 && mgcp_msg->codecs[i] != CODEC_AMRWB_16000_1)
continue;
- pt = map_codec_to_pt(mgcp_msg->ptmap, mgcp_msg->ptmap_len, mgcp_msg->codecs[i]);
+ pt = map_codec_to_pt2(ptmap_used, mgcp_msg->ptmap, mgcp_msg->ptmap_len, mgcp_msg->codecs[i]);
if (mgcp_msg->param.amr_octet_aligned_present && mgcp_msg->param.amr_octet_aligned)
MSGB_PRINTF_OR_RET("a=fmtp:%u octet-align=1\r\n", pt);
else if (mgcp_msg->param.amr_octet_aligned_present && !mgcp_msg->param.amr_octet_aligned)
@@ -1356,8 +1384,9 @@
}
}
+ memset(ptmap_used, 0, sizeof(ptmap_used));
for (i = 0; i < mgcp_msg->codecs_len; i++) {
- pt = map_codec_to_pt(mgcp_msg->ptmap, mgcp_msg->ptmap_len, mgcp_msg->codecs[i]);
+ pt = map_codec_to_pt2(ptmap_used, mgcp_msg->ptmap, mgcp_msg->ptmap_len, mgcp_msg->codecs[i]);
/* Note: Only dynamic payload type from the range 96-127
* require to be explained further via rtpmap. All others
diff --git a/tests/mgcp_client/mgcp_client_test.c b/tests/mgcp_client/mgcp_client_test.c
index 37c5f6c..13bd879 100644
--- a/tests/mgcp_client/mgcp_client_test.c
+++ b/tests/mgcp_client/mgcp_client_test.c
@@ -624,6 +624,34 @@
printf("\n");
}
+static void test_map_codec_to_pt2_and_map_pt_to_codec(void)
+{
+ struct ptmap ptmap[10];
+ bool ptmap_used[10] = { 0 };
+ 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;
+ /* This would not work with map_codec_to_pt() as we already have an entry with CODEC_AMR_8000_1 in the list. */
+ ptmap[4].codec = CODEC_AMR_8000_1;
+ ptmap[4].pt = 100;
+ ptmap_len = 5;
+
+ /* Mappings that are covered by the table */
+ for (i = 0; i < ptmap_len; i++)
+ printf(" %u => %u\n", ptmap[i].codec, map_codec_to_pt2(ptmap_used, 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;
@@ -703,6 +731,7 @@
test_mgcp_client_cancel();
test_sdp_section_start();
test_map_codec_to_pt_and_map_pt_to_codec();
+ test_map_codec_to_pt2_and_map_pt_to_codec();
test_map_pt_to_codec();
test_mgcp_client_e1_epname();
diff --git a/tests/mgcp_client/mgcp_client_test.ok b/tests/mgcp_client/mgcp_client_test.ok
index 039fbd9..dcbb6fc 100644
--- a/tests/mgcp_client/mgcp_client_test.ok
+++ b/tests/mgcp_client/mgcp_client_test.ok
@@ -210,6 +210,17 @@
2 <= 2
100 <= 100
+ 110 => 96
+ 111 => 97
+ 112 => 98
+ 113 => 99
+ 112 => 100
+ 96 <= 110
+ 97 <= 111
+ 98 <= 112
+ 99 <= 113
+ 100 <= 112
+
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 34349. To unsubscribe, or for help writing mail filters, visit settings.