mgw, client: use new fmtp.h in ptmap: allow all fmtp

Remove the limit of having only one AMR octet-aligned fmtp parameter per
MGCP message. Instead allow any arbitrary fmtp options, one per every
codec.

Deprecate all use of struct mgcp_codec_param. Instead, store and pass
plain fmtp strings.

Provide legacy shims that still act correctly for any callers that may
pass the old struct mgcp_codec_param. (I'm not sure if we need to keep
this, but we can always drop it in another patch.)

Adjust one mgcp_test.c: instead of returning only the octet-aligned
parameter, now osmo-mgw keeps and returns all the fmtp parameters that
the user provided. So add the missing "mode-change-capability".

Related: OS#6171
Related: osmo-msc Ief9225c9bcf7525a9a0a07c282ffb8cc0d092186
Change-Id: If58590bda8627519ff07e0b6f43aa47a274f052b
This commit is contained in:
Neels Hofmeyr
2023-10-26 03:30:36 +02:00
parent e878f54acd
commit e48ae27813
13 changed files with 123 additions and 102 deletions

View File

@@ -13,6 +13,7 @@ struct mgcp_conn_rtp;
void mgcp_codec_summary(struct mgcp_conn_rtp *conn);
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_add2(struct mgcp_conn_rtp *conn, int payload_type, const char *audio_name, const char *fmtp);
int mgcp_codec_decide(struct mgcp_conn_rtp *conn_src, struct mgcp_conn_rtp *conn_dst);
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);

View File

@@ -59,7 +59,7 @@ enum mgcp_x_osmo_ign {
MGCP_X_OSMO_IGN_CALLID = 1,
};
/* Codec parameters (communicated via SDP/fmtp) */
/* Deprecated. Use the new fmtp string instead. */
struct mgcp_codec_param {
bool amr_octet_aligned_present;
bool amr_octet_aligned;

View File

@@ -83,8 +83,11 @@ struct mgcp_rtp_codec {
char audio_name[64];
char subtype_name[64];
/* Deprecated. Use the new fmtp string instead. */
bool param_present;
struct mgcp_codec_param param;
char fmtp[256];
};
/* 'mgcp_rtp_end': basically a wrapper around the RTP+RTCP ports */

View File

@@ -77,6 +77,9 @@ struct ptmap {
/*! payload type number (96-127) */
unsigned int pt;
/*! the MGCP 'a=fmtp:N <...>' string, e.g. "mode-set=1,2,3;octet-align=0". */
char fmtp[256];
};
int ptmap_cmp(const struct ptmap *a, const struct ptmap *b);

View File

@@ -57,10 +57,11 @@ struct mgcp_conn_peer {
* address is set. If != MGCP_CONN_NONE, force this conn mode. */
enum mgcp_connection_mode conn_mode;
/*! If the codec requires additional format parameters (fmtp), those cann be set here, see also
* mgcp_common.h */
bool param_present;
struct mgcp_codec_param param;
/*! Deprectated, use ptmap[].fmtp instead.
* Global codec params. In case the codec requires additional format parameters (fmtp), those can be set
* here, see also mgcp_common.h. The format parameters will be applied on all codecs where applicable. */
bool param_present OSMO_DEPRECATED_OUTSIDE_LIBOSMOMGCPCLIENT("use ptmap[].fmtp instead");
struct mgcp_codec_param param OSMO_DEPRECATED_OUTSIDE_LIBOSMOMGCPCLIENT("use ptmap[].fmtp instead");
};
struct osmo_fsm_inst *mgcp_conn_create(struct mgcp_client *mgcp, struct osmo_fsm_inst *parent_fi, uint32_t parent_term_evt,

View File

@@ -40,6 +40,7 @@ libosmo_mgcp_client_la_LDFLAGS = \
$(NULL)
libosmo_mgcp_client_la_LIBADD = \
$(top_builddir)/src/libosmo-sdp/libosmo-sdp.la \
$(LIBOSMOCORE_LIBS) \
$(LIBOSMOVTY_LIBS) \
$(NULL)

View File

@@ -1377,13 +1377,20 @@ static int add_sdp(struct msgb *msg, struct mgcp_msg *mgcp_msg, struct mgcp_clie
}
/* Add optional codec parameters (fmtp) */
if (mgcp_msg->param_present) {
for (i = 0; i < mgcp_msg->ptmap_len; i++) {
/* The following is only applicable for AMR */
if (mgcp_msg->ptmap[i].codec != CODEC_AMR_8000_1
&& mgcp_msg->ptmap[i].codec != CODEC_AMRWB_16000_1)
continue;
for (i = 0; i < mgcp_msg->ptmap_len; i++) {
/* Add fmtp string, if any is set. */
if (mgcp_msg->ptmap[i].fmtp[0]) {
MSGB_PRINTF_OR_RET("a=fmtp:%u %s\r\n", mgcp_msg->ptmap[i].pt, mgcp_msg->ptmap[i].fmtp);
continue;
}
/* LEGACY COMPAT. Fill in fmtp with the legacy mgcp_msg->param, if any, when no individual fmtp is set
* on the codec. We only ever supported AMR fmtp in mgcp_msg->param. */
if (mgcp_msg->param_present
&& (mgcp_msg->ptmap[i].codec == CODEC_AMR_8000_1
|| mgcp_msg->ptmap[i].codec == CODEC_AMRWB_16000_1)) {
pt = mgcp_msg->ptmap[i].pt;
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)
@@ -1621,5 +1628,8 @@ int ptmap_cmp(const struct ptmap *a, const struct ptmap *b)
rc = OSMO_CMP(a->codec, b->codec);
if (rc)
return rc;
return OSMO_CMP(a->pt, b->pt);
rc = OSMO_CMP(a->pt, b->pt);
if (rc)
return rc;
return strcmp(a->fmtp, b->fmtp);
}

View File

@@ -45,6 +45,7 @@ libosmo_mgcp_la_SOURCES = \
$(NULL)
libosmo_mgcp_la_LIBADD = \
$(top_builddir)/src/libosmo-sdp/libosmo-sdp.la \
$(LIBOSMOCORE_LIBS) \
$(LIBOSMOGSM_LIBS) \
$(LIBOSMOVTY_LIBS) \

View File

@@ -24,6 +24,10 @@
#include <osmocom/mgcp/mgcp_endp.h>
#include <osmocom/mgcp/mgcp_trunk.h>
#include <osmocom/mgcp/mgcp_codec.h>
#include <osmocom/sdp/sdp_strings.h>
#include <osmocom/sdp/fmtp.h>
#include <errno.h>
/* Helper function to dump codec information of a specified codec to a printable
@@ -116,9 +120,9 @@ void mgcp_codec_reset_all(struct mgcp_conn_rtp *conn)
* \param[out] conn related rtp-connection.
* \param[in] payload_type codec type id (e.g. 3 for GSM, -1 when undefined).
* \param[in] audio_name audio codec name, in uppercase (e.g. "GSM/8000/1").
* \param[in] param optional codec parameters (set to NULL when unused).
* \param[in] fmtp optional codec parameters (set to NULL when unused).
* \returns 0 on success, -EINVAL on failure. */
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_add2(struct mgcp_conn_rtp *conn, int payload_type, const char *audio_name, const char *fmtp)
{
int rate;
int channels;
@@ -261,12 +265,16 @@ int mgcp_codec_add(struct mgcp_conn_rtp *conn, int payload_type, const char *aud
}
}
/* Copy over optional codec parameters */
if (param) {
codec->param = *param;
codec->param_present = true;
} else
codec->param_present = false;
if (fmtp) {
OSMO_STRLCPY_ARRAY(codec->fmtp, fmtp);
if (strlen(codec->fmtp) != strlen(fmtp)) {
LOGP(DLMGCP, LOGL_ERROR, "fmtp too long: %zu > %zu\n", strlen(fmtp), strlen(codec->fmtp));
/* let's just hope what is there is still useful, worst case the call's audio doesn't work */
}
}
/* legacy */
codec->param_present = false;
conn->end.codecs_assigned++;
return 0;
@@ -276,23 +284,33 @@ error:
return -EINVAL;
}
/* Return true if octet-aligned is set in the given codec. Default to octet-aligned=0, i.e. bandwidth-efficient mode.
* See RFC4867 "RTP Payload Format for AMR and AMR-WB" sections "8.1. AMR Media Type Registration" and "8.2. AMR-WB
* Media Type Registration":
*
* octet-align: Permissible values are 0 and 1. If 1, octet-aligned
* operation SHALL be used. If 0 or if not present,
* bandwidth-efficient operation is employed.
*
* https://tools.ietf.org/html/rfc4867
*/
/*! Legacy compat, use mgcp_codec_add2() instead to be able to pass any fmtp besides AMR octet-align=1.
* Add codec configuration depending on payload type and/or codec name. This
* function uses the input parameters to extrapolate the full codec information.
* \param[out] codec configuration (caller provided memory).
* \param[out] conn related rtp-connection.
* \param[in] payload_type codec type id (e.g. 3 for GSM, -1 when undefined).
* \param[in] audio_name audio codec name, in uppercase (e.g. "GSM/8000/1").
* \param[in] param optional codec parameters (set to NULL when unused).
* \returns 0 on success, -EINVAL on failure. */
int mgcp_codec_add(struct mgcp_conn_rtp *conn, int payload_type, const char *audio_name, const struct mgcp_codec_param *param)
{
const char *fmtp = NULL;
if (param && param->amr_octet_aligned_present)
fmtp = (param->amr_octet_aligned ? OSMO_SDP_STR_AMR_OCTET_ALIGN_1 : OSMO_SDP_STR_AMR_OCTET_ALIGN_0);
return mgcp_codec_add2(conn, payload_type, audio_name, fmtp);
}
bool mgcp_codec_amr_is_octet_aligned(const struct mgcp_rtp_codec *codec)
{
if (!codec->param_present)
return false;
if (!codec->param.amr_octet_aligned_present)
return false;
return codec->param.amr_octet_aligned;
/* Legacy */
if (!codec->fmtp[0]
&& codec->param_present
&& codec->param.amr_octet_aligned_present)
return codec->param.amr_octet_aligned;
return osmo_sdp_fmtp_amr_is_octet_aligned(codec->fmtp);
}
/* Compare two codecs, all parameters must match up */
@@ -471,13 +489,13 @@ int mgcp_codec_decide(struct mgcp_conn_rtp *conn_src, struct mgcp_conn_rtp *conn
/* Check if the codec has a specific AMR mode (octet-aligned or bandwith-efficient) set. */
bool mgcp_codec_amr_align_mode_is_indicated(const struct mgcp_rtp_codec *codec)
{
if (codec->param_present == false)
return false;
if (!codec->param.amr_octet_aligned_present)
return false;
if (strcmp(codec->subtype_name, "AMR") != 0)
return false;
return true;
if (!codec->fmtp[0]) {
/* Legacy */
return codec->param_present && codec->param.amr_octet_aligned_present;
}
/* Just check for presence, not the actual value. */
return osmo_sdp_fmtp_get_val(NULL, 0, codec->fmtp, OSMO_SDP_STR_AMR_OCTET_ALIGN);
}
/* Find the payload type number configured for a specific codec by SDP.

View File

@@ -1189,12 +1189,12 @@ int mgcp_send(struct mgcp_endpoint *endp, int is_rtp, struct osmo_sockaddr *addr
if (mgcp_conn_rtp_is_iuup(conn_dst) || mgcp_conn_rtp_is_iuup(conn_src)) {
/* the iuup code will correctly transform to the correct AMR mode */
} else if (mgcp_codec_amr_align_mode_is_indicated(conn_dst->end.codec)) {
rc = amr_oa_bwe_convert(endp, msg,
conn_dst->end.codec->param.amr_octet_aligned);
bool oa = mgcp_codec_amr_is_octet_aligned(conn_dst->end.codec);
rc = amr_oa_bwe_convert(endp, msg, oa);
if (rc < 0) {
LOGPENDP(endp, DRTP, LOGL_ERROR,
"Error in AMR octet-aligned <-> bandwidth-efficient mode conversion (target=%s)\n",
conn_dst->end.codec->param.amr_octet_aligned ? "octet-aligned" : "bandwidth-efficient");
oa ? "octet-aligned" : "bandwidth-efficient");
break;
}
} else if (rtp_end->rfc5993_hr_convert &&
@@ -1505,16 +1505,18 @@ static int rx_rtp(struct msgb *msg)
if (mc->proto == MGCP_PROTO_RTP
&& conn_src->end.codec
&& mgcp_codec_amr_align_mode_is_indicated(conn_src->end.codec)) {
bool src_oa;
/* Make sure that the incoming AMR frame format matches the frame format that the call agent has
* communicated via SDP when the connection was created/modfied. */
int oa = amr_oa_check((char*)msgb_data(msg), msgb_length(msg));
if (oa < 0)
return -1;
if (((bool)oa) != conn_src->end.codec->param.amr_octet_aligned) {
src_oa = mgcp_codec_amr_is_octet_aligned(conn_src->end.codec);
if (((bool)oa) != src_oa) {
LOG_CONN_RTP(conn_src, LOGL_NOTICE,
"rx_rtp(%u bytes): Expected RTP AMR octet-aligned=%u but got octet-aligned=%u."
" check the config of your call-agent!\n",
msgb_length(msg), conn_src->end.codec->param.amr_octet_aligned, oa);
msgb_length(msg), src_oa, oa);
return -1;
}
}

View File

@@ -1131,6 +1131,7 @@ mgcp_header_done:
LOGPCONN(_conn, DLMGCP, LOGL_NOTICE,
"CRCX: connection successfully created\n");
rate_ctr_inc(rate_ctr_group_get_ctr(rate_ctrs, MGCP_CRCX_SUCCESS));
mgcp_endp_update(endp);

View File

@@ -35,6 +35,9 @@
#include <osmocom/mgcp/mgcp_sdp.h>
#include <osmocom/mgcp/mgcp_protocol.h>
#include <osmocom/sdp/fmtp.h>
#include <osmocom/sdp/sdp_strings.h>
#include <errno.h>
#include <stdlib.h>
#include <limits.h>
@@ -55,7 +58,7 @@ struct sdp_rtp_map {
};
struct sdp_fmtp_param {
int payload_type;
struct mgcp_codec_param param;
const char *fmtp;
};
@@ -195,11 +198,7 @@ static int fmtp_from_sdp(void *ctx, struct sdp_fmtp_param *fmtp_param, char *sdp
{
char *str;
char *str_ptr;
char *param_str;
unsigned int pt;
unsigned int count = 0;
char delimiter;
unsigned int amr_octet_aligned;
memset(fmtp_param, 0, sizeof(*fmtp_param));
@@ -218,40 +217,13 @@ static int fmtp_from_sdp(void *ctx, struct sdp_fmtp_param *fmtp_param, char *sdp
goto error;
fmtp_param->payload_type = pt;
/* Advance pointer to the beginning of the parameter section and
* tokenize string */
/* Advance pointer to the beginning of the parameter section */
str_ptr = strstr(str_ptr, " ");
if (!str_ptr)
goto error;
str_ptr++;
param_str = strtok(str_ptr, " ");
if (!param_str)
goto exit;
while (1) {
/* Make sure that we don't get trapped in an endless loop */
if (count > 256)
goto error;
/* Chop off delimiters ';' at the end */
delimiter = str_ptr[strlen(str_ptr) - 1];
if (delimiter == ';' || delimiter == ',')
str_ptr[strlen(str_ptr) - 1] = '\0';
/* AMR octet aligned parameter (see also RFC 3267, section 8.3) */
if (sscanf(param_str, "octet-align=%d", &amr_octet_aligned) == 1) {
fmtp_param->param.amr_octet_aligned_present = true;
fmtp_param->param.amr_octet_aligned = false;
if (amr_octet_aligned == 1)
fmtp_param->param.amr_octet_aligned = true;
}
param_str = strtok(NULL, " ");
if (!param_str)
break;
count++;
}
fmtp_param->fmtp = talloc_strdup(ctx, str_ptr);
exit:
talloc_free(str);
@@ -299,13 +271,13 @@ static int audio_ip_from_sdp(struct osmo_sockaddr *dst_addr, char *sdp)
/* Pick optional fmtp parameters by payload type, if there are no fmtp
* parameters, a nullpointer is returned */
static struct mgcp_codec_param *param_by_pt(int pt, struct sdp_fmtp_param *fmtp_params, unsigned int fmtp_params_len)
static const char *param_by_pt(int pt, struct sdp_fmtp_param *fmtp_params, unsigned int fmtp_params_len)
{
unsigned int i;
for (i = 0; i < fmtp_params_len; i++) {
if (fmtp_params[i].payload_type == pt)
return &fmtp_params[i].param;
return fmtp_params[i].fmtp;
}
return NULL;
@@ -326,7 +298,6 @@ int mgcp_parse_sdp_data(const struct mgcp_endpoint *endp,
unsigned int codecs_used = 0;
struct sdp_fmtp_param fmtp_params[MGCP_MAX_CODECS];
unsigned int fmtp_used = 0;
struct mgcp_codec_param *codec_param;
char ipbuf[INET6_ADDRSTRLEN];
char *line;
unsigned int i;
@@ -421,8 +392,8 @@ int mgcp_parse_sdp_data(const struct mgcp_endpoint *endp,
/* Store parsed codec information */
for (i = 0; i < codecs_used; i++) {
codec_param = param_by_pt(codecs[i].payload_type, fmtp_params, fmtp_used);
rc = mgcp_codec_add(conn, codecs[i].payload_type, codecs[i].map_line, codec_param);
const char *fmtp = param_by_pt(codecs[i].payload_type, fmtp_params, fmtp_used);
rc = mgcp_codec_add2(conn, codecs[i].payload_type, codecs[i].map_line, fmtp);
if (rc < 0)
LOGPENDP(endp, DLMGCP, LOGL_NOTICE, "failed to add codec\n");
}
@@ -436,10 +407,12 @@ int mgcp_parse_sdp_data(const struct mgcp_endpoint *endp,
if (codecs_used == 0)
LOGPC(DLMGCP, LOGL_NOTICE, "none");
for (i = 0; i < codecs_used; i++) {
LOGPC(DLMGCP, LOGL_NOTICE, "%d=%s",
LOGPC(DLMGCP, LOGL_NOTICE, " %d=%s%s%s%s",
rtp->codecs[i].payload_type,
strlen(rtp->codecs[i].subtype_name) ? rtp->codecs[i].subtype_name : "unknown");
LOGPC(DLMGCP, LOGL_NOTICE, " ");
strlen(rtp->codecs[i].subtype_name) ? rtp->codecs[i].subtype_name : "unknown",
rtp->codecs[i].fmtp[0] ? ",fmtp='" : "",
rtp->codecs[i].fmtp,
rtp->codecs[i].fmtp[0] ? "'" : "");
}
LOGPC(DLMGCP, LOGL_NOTICE, "\n");
@@ -494,18 +467,15 @@ static int add_fmtp(struct msgb *sdp, struct sdp_fmtp_param *fmtp_params, unsign
int rc;
for (i = 0; i < fmtp_params_len; i++) {
bool first = true;
rc = msgb_printf(sdp, "a=fmtp:%u", fmtp_params[i].payload_type);
if (rc < 0)
return -EINVAL;
/* Add amr octet align parameter */
if (fmtp_params[i].param.amr_octet_aligned_present) {
if (fmtp_params[i].param.amr_octet_aligned)
rc = msgb_printf(sdp, " octet-align=1");
else
rc = msgb_printf(sdp, " octet-align=0");
if (rc < 0)
return -EINVAL;
if (fmtp_params[i].fmtp) {
msgb_printf(sdp, "%s%s", first ? " " : ";", fmtp_params[i].fmtp);
first = false;
}
rc = msgb_printf(sdp, "\r\n");
@@ -529,7 +499,6 @@ int mgcp_write_response_sdp(const struct mgcp_endpoint *endp,
const struct mgcp_rtp_codec *codec;
const char *audio_name;
int payload_type;
struct sdp_fmtp_param fmtp_param;
int rc;
int payload_types[1];
int local_port;
@@ -578,12 +547,22 @@ int mgcp_write_response_sdp(const struct mgcp_endpoint *endp,
goto buffer_too_small;
}
if (codec->param_present) {
fmtp_param.payload_type = payload_type;
fmtp_param.param = codec->param;
fmtp_params[0] = fmtp_param;
if (codec->fmtp[0]) {
fmtp_params[0] = (struct sdp_fmtp_param){
.payload_type = payload_type,
.fmtp = codec->fmtp,
};
fmtp_params_len = 1;
} else if (codec->param_present) {
/* Legacy */
fmtp_params[0] = (struct sdp_fmtp_param){
.payload_type = payload_type,
};
fmtp_params_len = 1;
fmtp_params[0].fmtp = (codec->param.amr_octet_aligned ?
OSMO_SDP_STR_AMR_OCTET_ALIGN_1 : OSMO_SDP_STR_AMR_OCTET_ALIGN_0);
}
rc = add_fmtp(sdp, fmtp_params, fmtp_params_len);
if (rc < 0)
goto buffer_too_small;

View File

@@ -550,7 +550,7 @@ static void test_strline(void)
"t=0 0\r\n" \
"m=audio 16012 RTP/AVP 111\r\n" \
"a=rtpmap:111 AMR/8000/1\r\n" \
"a=fmtp:111 octet-align=1\r\n" \
"a=fmtp:111 mode-change-capability=2; octet-align=1\r\n" \
"a=ptime:20\r\n"
#define CRCX_NO_LCO_NO_SDP_RET \
@@ -838,6 +838,7 @@ static void test_messages(void)
}
} else if (check_response(msg->data, t->exp_resp) != 0) {
printf("%s failed.\n", t->name);
fflush(stdout);
OSMO_ASSERT(false);
}