dexter has uploaded this change for review.
mgcp_client_fsm: fix inconsistent API (param_present, param).
The struct struct mgcp_conn_peer in mgcp_client_fsm.h (and the struct
struct mgcp_msg in mgcp_client.h) use a problematic way to hold
information about codec parameters: Using the struct member param, the
user can set options for AMR (octet-aligned/bandwith-efficient). This is
done using the struct mgcp_codec_param, which is also not very well
thought out. Since we can only set the parameters for AMR exactly once, it
is not possible to set different parameters when AMR occurs multiple times
in codecs.
Since what we would have needed is an array of struct mgcp_codec_param,
where we can set the parameters for each codec we use indvidually. So
let's depreacate the use of param/param_present and add a new struct
member codec_params/codec_params_present, where codec_params is an
array. Let's also re-define mgcp_codec_params as mgcp_codec_params2, and
use a union so that we can add params for other codecs than AMR in the
future.
Unfortunately mgcp_codec_params is also used in osmo-mgw, we should
migrate each occurrence of mgcp_codec_param to mgcp_codec_param2 there
too, but this should be done in a follow up patch.
Related: OS#5723
Change-Id: I50d737f3f3d45e4004c64101700a471fe75b3436
---
M TODO-RELEASE
M include/osmocom/mgcp/mgcp_common.h
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 src/libosmo-mgcp-client/mgcp_client_fsm.c
6 files changed, 85 insertions(+), 8 deletions(-)
git pull ssh://gerrit.osmocom.org:29418/osmo-mgw refs/changes/51/34351/1
diff --git a/TODO-RELEASE b/TODO-RELEASE
index 69b38a9..582ec1b 100644
--- a/TODO-RELEASE
+++ b/TODO-RELEASE
@@ -25,4 +25,7 @@
#
#library what description / commit summary line
libosmo-mgcp-client NEW API mgcp_client_conf_alloc()
-libosmo-mgcp-client DEPRECATED mgcp_client_conf_init()
\ No newline at end of file
+libosmo-mgcp-client DEPRECATED mgcp_client_conf_init()
+libosmo-mgcp-client NEW API struct mgcp_conn_peer, added member, codecs_param_present and param
+libosmo-mgcp-client NEW API struct struct mgcp_msg, added member, codecs_param_present and param
+mgcp NEW API add struct struct mgcp_codec_param2, depreacte struct mgcp_codec_param
diff --git a/include/osmocom/mgcp/mgcp_common.h b/include/osmocom/mgcp/mgcp_common.h
index 7de45f9..ed90fad 100644
--- a/include/osmocom/mgcp/mgcp_common.h
+++ b/include/osmocom/mgcp/mgcp_common.h
@@ -59,12 +59,21 @@
MGCP_X_OSMO_IGN_CALLID = 1,
};
-/* Codec parameters (communicated via SDP/fmtp) */
+/* Deprecated: Codec parameters (communicated via SDP/fmtp) */
struct mgcp_codec_param {
bool amr_octet_aligned_present;
bool amr_octet_aligned;
};
+/* Codec parameters (communicated via SDP/fmtp) */
+struct mgcp_codec_param2 {
+ union {
+ struct {
+ bool octet_aligned;
+ } amr;
+ };
+};
+
/* Ensure that the msg->l2h is NUL terminated. */
static inline int mgcp_msg_terminate_nul(struct msgb *msg)
{
diff --git a/include/osmocom/mgcp_client/mgcp_client.h b/include/osmocom/mgcp_client/mgcp_client.h
index 4e54bbe..0757ce0 100644
--- a/include/osmocom/mgcp_client/mgcp_client.h
+++ b/include/osmocom/mgcp_client/mgcp_client.h
@@ -136,8 +136,10 @@
uint32_t x_osmo_ign;
bool x_osmo_osmux_use;
int x_osmo_osmux_cid; /* -1 is wildcard */
- bool param_present;
- struct mgcp_codec_param param;
+ bool param_present; /* Depreacted, please use codecs_param_present */
+ struct mgcp_codec_param param; /* Depreacted, please use codecs_param */
+ bool codecs_param_present; /* See also mgcp_client_fsm.h, struct mgcp_conn_peer */
+ struct mgcp_codec_param2 codecs_param[MGCP_MAX_CODECS]; /* See also mgcp_client_fsm.h, struct mgcp_conn_peer */
};
struct mgcp_client_conf *mgcp_client_conf_alloc(void *ctx);
diff --git a/include/osmocom/mgcp_client/mgcp_client_fsm.h b/include/osmocom/mgcp_client/mgcp_client_fsm.h
index 245e0a7..3e9a497 100644
--- a/include/osmocom/mgcp_client/mgcp_client_fsm.h
+++ b/include/osmocom/mgcp_client/mgcp_client_fsm.h
@@ -58,10 +58,16 @@
* address is set. If != MGCP_CONN_NONE, force this conn mode. */
enum mgcp_connection_mode conn_mode;
- /*! Global codec params. In case the codec requires additional format parameters (fmtp), those cann be set
- * here, see also mgcp_common.h. The format parameters will be applied on all codecs where applicable. */
+ /*! (deprecated, use codecs_param) Global codec params. In case the codec requires additional format parameters
+ * (fmtp), those cann be set here, see also mgcp_common.h. The format parameters will be applied on all codecs
+ * where applicable. */
bool param_present;
struct mgcp_codec_param param;
+
+ /* In case the codec requires additional format parameters (fmtp), those can be set here for each codec
+ * individually, see also mgcp_common.h. The index in codecs_param[] must match index as in codecs[]. */
+ bool codecs_param_present;
+ struct mgcp_codec_param2 codecs_param[MGCP_MAX_CODECS];
};
struct osmo_fsm_inst *mgcp_conn_create(struct mgcp_client *mgcp, struct osmo_fsm_inst *parent_fi, uint32_t parent_term_evt,
diff --git a/src/libosmo-mgcp-client/mgcp_client.c b/src/libosmo-mgcp-client/mgcp_client.c
index ee65706..21963ab 100644
--- a/src/libosmo-mgcp-client/mgcp_client.c
+++ b/src/libosmo-mgcp-client/mgcp_client.c
@@ -1370,7 +1370,10 @@
MSGB_PRINTF_OR_RET("\r\n");
/* Add optional codec parameters (fmtp) */
+ /* Never use mgcp_msg->param (deprecated) together mgcp_msg->param_present->codecs_param! */
+ OSMO_ASSERT(mgcp_msg->param_present != mgcp_msg->codecs_param_present);
if (mgcp_msg->param_present) {
+ LOGPMGW(mgcp, LOGL_DEBUG, "using depreacted struct member mgcp_msg->param, please use mgcp_msg->codecs_param!\n");
memset(ptmap_used, 0, sizeof(ptmap_used));
for (i = 0; i < mgcp_msg->codecs_len; i++) {
/* The following is only applicable for AMR */
@@ -1383,6 +1386,19 @@
MSGB_PRINTF_OR_RET("a=fmtp:%u octet-align=0\r\n", pt);
}
}
+ if (mgcp_msg->codecs_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_pt2(ptmap_used, mgcp_msg->ptmap, mgcp_msg->ptmap_len, mgcp_msg->codecs[i]);
+ if (mgcp_msg->codecs_param[i].amr.octet_aligned)
+ MSGB_PRINTF_OR_RET("a=fmtp:%u octet-align=1\r\n", pt);
+ else
+ MSGB_PRINTF_OR_RET("a=fmtp:%u octet-align=0\r\n", pt);
+ }
+ }
memset(ptmap_used, 0, sizeof(ptmap_used));
for (i = 0; i < mgcp_msg->codecs_len; i++) {
diff --git a/src/libosmo-mgcp-client/mgcp_client_fsm.c b/src/libosmo-mgcp-client/mgcp_client_fsm.c
index 660f0ad..1d50b3f 100644
--- a/src/libosmo-mgcp-client/mgcp_client_fsm.c
+++ b/src/libosmo-mgcp-client/mgcp_client_fsm.c
@@ -107,6 +107,9 @@
static void make_crcx_msg(struct mgcp_msg *mgcp_msg, struct mgcp_conn_peer *info)
{
+ /* Never use info->param (deprecated) together with info->codecs_param! */
+ OSMO_ASSERT(info->param_present != info->codecs_param_present);
+
*mgcp_msg = (struct mgcp_msg) {
.verb = MGCP_VERB_CRCX,
.presence = (MGCP_MSG_PRESENCE_ENDPOINT | MGCP_MSG_PRESENCE_CALL_ID
@@ -116,12 +119,14 @@
.ptime = info->ptime,
.codecs_len = info->codecs_len,
.ptmap_len = info->ptmap_len,
- .param_present = info->param_present
+ .param_present = info->param_present,
+ .codecs_param_present = info->codecs_param_present
};
osmo_strlcpy(mgcp_msg->endpoint, info->endpoint, MGCP_ENDPOINT_MAXLEN);
memcpy(mgcp_msg->codecs, info->codecs, sizeof(mgcp_msg->codecs));
memcpy(mgcp_msg->ptmap, info->ptmap, sizeof(mgcp_msg->ptmap));
memcpy(&mgcp_msg->param, &info->param, sizeof(mgcp_msg->param));
+ memcpy(&mgcp_msg->codecs_param, &info->codecs_param, sizeof(mgcp_msg->codecs_param));
if (info->x_osmo_ign) {
mgcp_msg->x_osmo_ign = info->x_osmo_ign;
@@ -153,6 +158,9 @@
{
struct mgcp_msg mgcp_msg;
+ /* Never use mgcp_ctx->conn_peer_local->param (deprecated) together with mgcp_ctx->conn_peer_local->codecs_param! */
+ OSMO_ASSERT(mgcp_ctx->conn_peer_local.param_present != mgcp_ctx->conn_peer_local.codecs_param_present);
+
mgcp_msg = (struct mgcp_msg) {
.verb = MGCP_VERB_MDCX,
.presence = (MGCP_MSG_PRESENCE_ENDPOINT | MGCP_MSG_PRESENCE_CALL_ID | MGCP_MSG_PRESENCE_CONN_ID |
@@ -165,12 +173,14 @@
.ptime = mgcp_ctx->conn_peer_local.ptime,
.codecs_len = mgcp_ctx->conn_peer_local.codecs_len,
.ptmap_len = mgcp_ctx->conn_peer_local.ptmap_len,
- .param_present = mgcp_ctx->conn_peer_local.param_present
+ .param_present = mgcp_ctx->conn_peer_local.param_present,
+ .codecs_param_present = mgcp_ctx->conn_peer_local.codecs_param_present
};
osmo_strlcpy(mgcp_msg.endpoint, mgcp_ctx->conn_peer_remote.endpoint, MGCP_ENDPOINT_MAXLEN);
memcpy(mgcp_msg.codecs, mgcp_ctx->conn_peer_local.codecs, sizeof(mgcp_msg.codecs));
memcpy(mgcp_msg.ptmap, mgcp_ctx->conn_peer_local.ptmap, sizeof(mgcp_msg.ptmap));
memcpy(&mgcp_msg.param, &mgcp_ctx->conn_peer_local.param, sizeof(mgcp_ctx->conn_peer_local.param));
+ memcpy(&mgcp_msg.codecs_param, &mgcp_ctx->conn_peer_local.codecs_param, sizeof(mgcp_ctx->conn_peer_local.codecs_param));
set_conn_mode(&mgcp_msg, &mgcp_ctx->conn_peer_local);
To view, visit change 34351. To unsubscribe, or for help writing mail filters, visit settings.