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

13 files changed, 123 insertions, 102 deletions

diff --git a/include/osmocom/mgcp/mgcp_codec.h b/include/osmocom/mgcp/mgcp_codec.h
index cfc8ecf06..4702f6dda 100644
--- a/include/osmocom/mgcp/mgcp_codec.h
+++ b/include/osmocom/mgcp/mgcp_codec.h
@@ -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);
diff --git a/include/osmocom/mgcp/mgcp_common.h b/include/osmocom/mgcp/mgcp_common.h
index 05ae95f3f..200376e3c 100644
--- a/include/osmocom/mgcp/mgcp_common.h
+++ b/include/osmocom/mgcp/mgcp_common.h
@@ -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;
diff --git a/include/osmocom/mgcp/mgcp_network.h b/include/osmocom/mgcp/mgcp_network.h
index a7bd3330d..99c783488 100644
--- a/include/osmocom/mgcp/mgcp_network.h
+++ b/include/osmocom/mgcp/mgcp_network.h
@@ -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 */
diff --git a/include/osmocom/mgcp_client/mgcp_client.h b/include/osmocom/mgcp_client/mgcp_client.h
index 1d336905a..e97bdb19b 100644
--- a/include/osmocom/mgcp_client/mgcp_client.h
+++ b/include/osmocom/mgcp_client/mgcp_client.h
@@ -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);
diff --git a/include/osmocom/mgcp_client/mgcp_client_fsm.h b/include/osmocom/mgcp_client/mgcp_client_fsm.h
index dbd51288e..3e31e9c65 100644
--- a/include/osmocom/mgcp_client/mgcp_client_fsm.h
+++ b/include/osmocom/mgcp_client/mgcp_client_fsm.h
@@ -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,
diff --git a/src/libosmo-mgcp-client/Makefile.am b/src/libosmo-mgcp-client/Makefile.am
index b2bed9adc..b88ed2b92 100644
--- a/src/libosmo-mgcp-client/Makefile.am
+++ b/src/libosmo-mgcp-client/Makefile.am
@@ -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)
diff --git a/src/libosmo-mgcp-client/mgcp_client.c b/src/libosmo-mgcp-client/mgcp_client.c
index 489ce6942..b42ce0fa4 100644
--- a/src/libosmo-mgcp-client/mgcp_client.c
+++ b/src/libosmo-mgcp-client/mgcp_client.c
@@ -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);
 }
diff --git a/src/libosmo-mgcp/Makefile.am b/src/libosmo-mgcp/Makefile.am
index 769a9cd9d..042ccebbd 100644
--- a/src/libosmo-mgcp/Makefile.am
+++ b/src/libosmo-mgcp/Makefile.am
@@ -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) \
diff --git a/src/libosmo-mgcp/mgcp_codec.c b/src/libosmo-mgcp/mgcp_codec.c
index 2f1df7bd5..da7dd3d0e 100644
--- a/src/libosmo-mgcp/mgcp_codec.c
+++ b/src/libosmo-mgcp/mgcp_codec.c
@@ -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.
diff --git a/src/libosmo-mgcp/mgcp_network.c b/src/libosmo-mgcp/mgcp_network.c
index 111c5cf23..5c890aacd 100644
--- a/src/libosmo-mgcp/mgcp_network.c
+++ b/src/libosmo-mgcp/mgcp_network.c
@@ -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;
 		}
 	}
diff --git a/src/libosmo-mgcp/mgcp_protocol.c b/src/libosmo-mgcp/mgcp_protocol.c
index ac5c68241..22eb12d47 100644
--- a/src/libosmo-mgcp/mgcp_protocol.c
+++ b/src/libosmo-mgcp/mgcp_protocol.c
@@ -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);
 
diff --git a/src/libosmo-mgcp/mgcp_sdp.c b/src/libosmo-mgcp/mgcp_sdp.c
index 77825f6ce..33cca2a2c 100644
--- a/src/libosmo-mgcp/mgcp_sdp.c
+++ b/src/libosmo-mgcp/mgcp_sdp.c
@@ -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;
diff --git a/tests/mgcp/mgcp_test.c b/tests/mgcp/mgcp_test.c
index c76bd9d83..6fa032e9b 100644
--- a/tests/mgcp/mgcp_test.c
+++ b/tests/mgcp/mgcp_test.c
@@ -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);
 		}