fix SDP codecs lists returned in OK messages

So far, osmo-mgw picked a single codec per side and returned only that
entry in the SDP part of MGCP 'OK' responses.

Instead, return all of the codec entries that were successfully
assigned.

The difference is shown in mgcp_test.c: before, only one of the codecs
made it through to the 'OK' reponse. Now, all of them are reflected
properly.

To clients that only ever assign a single codec, there is no change in
behavior.

Simplify the code: collapse these functions
  add_rtpmap()
  add_fmtp()
  add_audio()
  codec related part from mgcp_write_response_sdp()
and have only
  add_codecs()
add_codecs() simply iterates all assigned codecs to output the 'm=audio'
line as well as any 'a=rtpmap' and 'a=fmtp' lines.

Also helping to simplify is that we recently dropped the 'fmtp-extra'
feature in Icee0cd1f5a751fa760d5a9deca29089e78e7eb93

Related: OS#6293
Change-Id: Ib197ae30a4e732e7dc36700ba8b5841806ea12f7

TODO-RELEASE

@@ -35,3 +35,8 @@ libosmo-mgcp-client	remove public API	These public API items have not been calle
 						mgcp_client_cancel()
 						mgcp_msg_gen()
 						mgcp_msg_trans_id()
+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()

contrib/osmo-mgw.spec.in

@@ -60,16 +60,6 @@ Osmocom's Media Gateway Control Protocol client librarary.
 This subpackage contains libraries and header files for developing
 applications that want to make use of libosmo-mgcp-client.
 
-%package -n libosmo-mgcp-devel
-Summary:        Development files for Osmocom's Media Gateway server library
-Group:          Development/Libraries/C and C++
-
-%description -n libosmo-mgcp-devel
-Osmocom's Media Gateway Control Protocol server library.
-
-This subpackage contains libraries and header files for developing
-applications that want to make use of libosmo-mgcp.
-
 %prep
 %setup -q
 
@@ -129,9 +119,4 @@ make %{?_smp_mflags} check || (find . -name testsuite.log -exec cat {} +)
 %dir %{_includedir}/osmocom/mgcp_client
 %{_includedir}/osmocom/mgcp_client/*.h
 
-%files -n libosmo-mgcp-devel
-%dir %{_includedir}/osmocom
-%dir %{_includedir}/osmocom/mgcp
-%{_includedir}/osmocom/mgcp/*.h
-
 %changelog

include/Makefile.am

@@ -8,6 +8,10 @@ nobase_include_HEADERS = \
 	osmocom/mgcp_client/mgcp_client_endpoint_fsm.h \
 	osmocom/mgcp_client/mgcp_client_fsm.h \
 	osmocom/mgcp_client/mgcp_client_pool.h \
+	osmocom/mgcp_client/fmtp.h \
+	$(NULL)
+
+noinst_HEADERS = \
 	osmocom/mgcp/mgcp.h \
 	osmocom/mgcp/mgcp_common.h \
 	osmocom/mgcp/osmux.h \

include/osmocom/mgcp/mgcp.h

@@ -138,8 +138,6 @@ struct mgcp_config {
 	mgcp_processing rtp_processing_cb;
 	mgcp_processing_setup setup_rtp_processing_cb;
 
-	mgcp_get_format get_net_downlink_format_cb;
-
 	struct osmo_wqueue gw_fd;
 
 	struct mgcp_port_range net_ports;

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);

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;

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 */
@@ -108,7 +111,6 @@ struct mgcp_rtp_end {
 	int  frames_per_packet;
 	uint32_t packet_duration_ms;
 	int maximum_packet_time; /* -1: not set */
-	char *fmtp_extra;
 	/* are we transmitting packets (true) or dropping (false) outbound packets */
 	bool output_enabled;
 	/* FIXME: This parameter can be set + printed, but is nowhere used! */

include/osmocom/mgcp/mgcp_trunk.h

@@ -27,7 +27,6 @@ struct mgcp_trunk {
 	unsigned int trunk_nr;
 	enum mgcp_trunk_type trunk_type;
 
-	char *audio_fmtp_extra;
 	int audio_send_ptime;
 	int audio_send_name;
 

include/osmocom/mgcp_client/fmtp.h

@@ -0,0 +1,29 @@
+#pragma once
+
+#include <stddef.h>
+#include <stdbool.h>
+
+#define OSMO_SDP_NAME_A "a"
+#define OSMO_SDP_NAME_FMTP "fmtp"
+#define OSMO_SDP_NAME_AMR_OCTET_ALIGN "octet-align"
+
+#define OSMO_SDP_VAL_AMR_OCTET_ALIGN_0 OSMO_SDP_NAME_AMR_OCTET_ALIGN "=0"
+#define OSMO_SDP_VAL_AMR_OCTET_ALIGN_1 OSMO_SDP_NAME_AMR_OCTET_ALIGN "=1"
+
+/* "fmtp:" */
+#define OSMO_SDP_PREFIX_FMTP OSMO_SDP_NAME_FMTP ":"
+/* "a=fmtp:" */
+#define OSMO_SDP_PREFIX_A_FMTP OSMO_SDP_NAME_A "=" OSMO_SDP_PREFIX_FMTP
+
+bool osmo_sdp_fmtp_get_val(char *val, size_t val_size, const char *fmtp, const char *option_name);
+int osmo_sdp_fmtp_get_int(const char *fmtp, const char *option_name, int default_value);
+
+/* Some AMR related fmtp parameters as in https://www.rfc-editor.org/rfc/rfc4867#section-8.1 that osmo-mgw needs.*/
+bool osmo_sdp_fmtp_amr_is_octet_aligned(const char *fmtp);
+
+/*! To compose AMR related fmtp indicating octet-align.
+ * Usage:
+ *   printf("%s", OSMO_SDP_AMR_SET_OCTET_ALIGN(oa_flag));
+ */
+#define OSMO_SDP_AMR_SET_OCTET_ALIGN(VAL) \
+	((VAL) ? OSMO_SDP_VAL_AMR_OCTET_ALIGN_1 : OSMO_SDP_VAL_AMR_OCTET_ALIGN_0 )

include/osmocom/mgcp_client/mgcp_client.h

@@ -77,8 +77,13 @@ 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);
+
 enum mgcp_verb {
 	MGCP_VERB_CRCX,
 	MGCP_VERB_MDCX,
@@ -117,9 +122,5 @@ static inline const char *mgcp_client_cmode_name(enum mgcp_connection_mode mode)
 }
 
 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);

include/osmocom/mgcp_client/mgcp_client_fsm.h

@@ -12,7 +12,11 @@
  *  When modifiying a connection, the endpoint and call_id members may be left
  *  unpopulated. The call_id field is ignored in this case. If an endpoint
  *  identifier is supplied it is checked against the internal state to make
- *  sure it is correct. */
+ *  sure it is correct.
+ *
+ *  CAUTION: This struct may be subject to changes and new struct members may
+ *  be added in the future. To prevent memory conflicts it is strongly advised
+ *  to allocate this struct dynamically using mgcp_conn_peer_alloc() */
 struct mgcp_conn_peer {
 	/*! RTP connection IP-Address (optional, string e.g. "127.0.0.1") */
 	char addr[INET6_ADDRSTRLEN];
@@ -29,11 +33,11 @@ struct mgcp_conn_peer {
 	/*! RTP packetization interval (optional) */
 	unsigned int ptime;
 
-	/*! RTP codec list (optional) */
-	enum mgcp_codecs codecs[MGCP_MAX_CODECS];
-
-	/*! Number of codecs in RTP codec list (optional) */
-	unsigned int codecs_len;
+	/*! Deprecated. Use only ptmap[].codec in new code. */
+	enum mgcp_codecs codecs[MGCP_MAX_CODECS]
+		OSMO_DEPRECATED_OUTSIDE_LIBOSMOMGCPCLIENT("use ptmap[i].codec instead");
+	unsigned int codecs_len
+		OSMO_DEPRECATED_OUTSIDE_LIBOSMOMGCPCLIENT("use ptmap[] and ptmap_len instead");
 
 	/*! RTP payload type map (optional, only needed when payload types are
 	 * used that differ from what IANA/3GPP defines) */
@@ -57,18 +61,25 @@ 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 */
+	/*! 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;
 	struct mgcp_codec_param param;
 };
 
-struct osmo_fsm_inst *mgcp_conn_create(struct mgcp_client *mgcp, struct osmo_fsm_inst *parent_fi, uint32_t parent_term_evt,
-				       uint32_t parent_evt, struct mgcp_conn_peer *conn_peer);
-int mgcp_conn_modify(struct osmo_fsm_inst *fi, uint32_t parent_evt, struct mgcp_conn_peer *conn_peer);
-void mgcp_conn_delete(struct osmo_fsm_inst *fi);
+struct mgcp_conn_peer *mgcp_conn_peer_alloc(void *ctx);
 
-const char *mgcp_conn_get_ci(struct osmo_fsm_inst *fi);
+struct osmo_fsm_inst *mgcp_conn_create(struct mgcp_client *mgcp, struct osmo_fsm_inst *parent_fi, uint32_t parent_term_evt,
+				       uint32_t parent_evt, struct mgcp_conn_peer *conn_peer)
+	OSMO_DEPRECATED_OUTSIDE_LIBOSMOMGCPCLIENT("use osmo_mgcpc_ep_alloc() and osmo_mgcpc_ep_ci_add() instead");
+int mgcp_conn_modify(struct osmo_fsm_inst *fi, uint32_t parent_evt, struct mgcp_conn_peer *conn_peer)
+	OSMO_DEPRECATED_OUTSIDE_LIBOSMOMGCPCLIENT("use osmo_mgcpc_ep_ci_request() instead");
+void mgcp_conn_delete(struct osmo_fsm_inst *fi)
+	OSMO_DEPRECATED_OUTSIDE_LIBOSMOMGCPCLIENT("use osmo_mgcpc_ep_ci_dlcx() instead");
+
+const char *mgcp_conn_get_ci(struct osmo_fsm_inst *fi)
+	OSMO_DEPRECATED_OUTSIDE_LIBOSMOMGCPCLIENT("use osmo_mgcpc_ep_ci.mgcp_ci_str instead");
 struct mgcp_client *mgcp_conn_get_client(struct osmo_fsm_inst *fi);
 
 const char *osmo_mgcpc_conn_peer_name(const struct mgcp_conn_peer *info);

include/osmocom/mgcp_client/mgcp_client_internal.h

@@ -38,8 +38,6 @@ struct mgcp_response {
 	uint16_t audio_port;
 	char audio_ip[INET6_ADDRSTRLEN];
 	unsigned int ptime;
-	enum mgcp_codecs codecs[MGCP_MAX_CODECS];
-	unsigned int codecs_len;
 	struct ptmap ptmap[MGCP_MAX_CODECS];
 	unsigned int ptmap_len;
 };
@@ -84,8 +82,6 @@ struct mgcp_msg {
 	char *audio_ip;
 	enum mgcp_connection_mode conn_mode;
 	unsigned int ptime;
-	enum mgcp_codecs codecs[MGCP_MAX_CODECS];
-	unsigned int codecs_len;
 	struct ptmap ptmap[MGCP_MAX_CODECS];
 	unsigned int ptmap_len;
 	uint32_t x_osmo_ign;

src/libosmo-mgcp-client/Makefile.am

@@ -31,6 +31,7 @@ libosmo_mgcp_client_la_SOURCES = \
 	mgcp_client_fsm.c \
 	mgcp_client_endpoint_fsm.c \
 	mgcp_client_pool.c \
+	fmtp.c \
 	$(NULL)
 
 libosmo_mgcp_client_la_LDFLAGS = \

src/libosmo-mgcp-client/fmtp.c

@@ -0,0 +1,120 @@
+/*
+ * (C) 2023-2015 by sysmocom - s.f.m.c. GmbH <info@sysmocom.de>
+ * All Rights Reserved
+ *
+ * Author: Neels Hofmeyr
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU Affero General Public License as published by
+ * the Free Software Foundation; either version 3 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU Affero General Public License for more details.
+ *
+ * You should have received a copy of the GNU Affero General Public License
+ * along with this program.  If not, see <http://www.gnu.org/licenses/>.
+ *
+ */
+#include <string.h>
+#include <ctype.h>
+#include <osmocom/core/utils.h>
+#include <osmocom/core/logging.h>
+#include <osmocom/mgcp_client/fmtp.h>
+
+static const char *fmtp_next_option(const char *fmtp)
+{
+	for (; fmtp && *fmtp && *fmtp != ';'; fmtp++);
+	for (; fmtp && isspace(*fmtp); fmtp++);
+	return fmtp;
+}
+
+/*! Parse a given SDP fmtp value string, returning the value of a specific option, if present.
+ *
+ * Example:
+ *
+ *   const char *fmtp_vals = "octet-align=1;mode-set=0,2,4,7";
+ *
+ *   char res[23];
+ *   if (osmo_sdp_fmtp_get_val(res, sizeof(res), fmtp_vals, "mode-set"))
+ *           use_modeset(res);
+ *   else
+ *           use_modeset(MY_DEFAULT_MODESET);
+ *
+ * \param[out] val  Buffer to write the option's value to.
+ * \param[in] val_size  Space available in val.
+ * \param[in] fmtp  fmtp value string to parse -- must not contain the "a=fmtp:N " prefix, only the value part.
+ * \param[in] option_name  Which fmtp option to get the value for.
+ * \return true when the option was found, false when it was not present.
+ */
+bool osmo_sdp_fmtp_get_val(char *val, size_t val_size, const char *fmtp, const char *option_name)
+{
+	const char *pos = fmtp;
+	const char *end;
+	int option_name_len = strlen(option_name);
+	for (; pos && *pos; pos = fmtp_next_option(pos)) {
+		if (!osmo_str_startswith(pos, option_name))
+			continue;
+		pos += option_name_len;
+		if (*pos != '=')
+			continue;
+		pos++;
+		break;
+	}
+
+	if (!pos || !*pos)
+		return false;
+
+	end = fmtp_next_option(pos);
+	OSMO_ASSERT(end);
+	if (val && val_size)
+		osmo_strlcpy(val, pos, OSMO_MIN(val_size, end - pos + 1));
+	return true;
+}
+
+/*! Parse a given SDP fmtp value string, returning the value of a specific integer option, if present.
+ *
+ * Example:
+ *
+ *   const char *fmtp_vals = "octet-align=1;mode-set=0,2,4,7";
+ *   bool oa = osmo_sdp_fmtp_get_int(fmtp_vals, OSMO_SDP_AMR_OCTET_ALIGN_NAME, 1);
+ *
+ * \param[in] fmtp  fmtp value string to parse -- must not contain the "a=fmtp:N " prefix, only the value part.
+ * \param[in] option_name  Which fmtp option to get the value for.
+ * \param[in] default_value  If option_name is not present or cannot be parsed as integer, return this instead.
+ * \return the integer value when the option was found and actually an integer, default_value otherwise.
+ */
+int osmo_sdp_fmtp_get_int(const char *fmtp, const char *option_name, int default_value)
+{
+	char val[128];
+	if (!osmo_sdp_fmtp_get_val(val, sizeof(val), fmtp, option_name))
+		return default_value;
+	if (!val[0])
+		return default_value;
+	int i;
+	if (osmo_str_to_int(&i, val, 10, 0, 1)) {
+		/* error parsing number */
+		LOGP(DLMGCP, LOGL_ERROR, "Invalid number in fmtp parameter '%s': '%s'\n", option_name, val);
+		return default_value;
+	}
+	return i;
+}
+
+/*! Return true if octet-align is present and set to 1 in the given AMR related fmtp value.
+ * Default to octet-align=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-align
+ *                 operation SHALL be used.  If 0 or if not present,
+ *                 bandwidth-efficient operation is employed.
+ *
+ * https://tools.ietf.org/html/rfc4867
+ */
+bool osmo_sdp_fmtp_amr_is_octet_aligned(const char *fmtp)
+{
+	return osmo_sdp_fmtp_get_int(fmtp, OSMO_SDP_NAME_AMR_OCTET_ALIGN, 0) == 1;
+}

src/libosmo-mgcp-client/mgcp_client.c

@@ -105,94 +105,6 @@ enum mgcp_codecs map_str_to_codec(const char *str)
 	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 */
@@ -302,7 +214,7 @@ static int mgcp_parse_audio_port_pt(struct mgcp_response *r, char *line)
 	char *pt_str;
 	char *pt_end;
 	unsigned long int pt;
-	unsigned int count = 0;
+	unsigned int ptmap_len;
 	unsigned int i;
 
 	/* Extract port information */
@@ -316,40 +228,58 @@ static int mgcp_parse_audio_port_pt(struct mgcp_response *r, char *line)
 	if (!line)
 		goto exit;
 
+	/* Clear any previous entries before writing over r->ptmap */
+	r->ptmap_len = 0;
+	/* Keep a local ptmap_len to show only the full list after parsing succeeded in whole. */
+	ptmap_len = 0;
+
 	pt_str = strtok(line, " ");
 	while (1) {
-		/* Do not allow excessive payload types */
-		if (count >= ARRAY_SIZE(r->codecs))
-			goto response_parse_failure_pt;
-
 		pt_str = strtok(NULL, " ");
 		if (!pt_str)
 			break;
 		errno = 0;
 		pt = strtoul(pt_str, &pt_end, 0);
 		if ((errno == ERANGE && pt == ULONG_MAX) || (errno && !pt) ||
-		    pt_str == pt_end)
+		    pt_str == pt_end) {
+			LOGP(DLMGCP, LOGL_ERROR, "SDP: cannot parse payload type number from '%s'\n", pt_str);
 			goto response_parse_failure_pt;
+		}
 
-		if (pt >> 7) /* PT is 7 bit field, higher values not allowed */
+		/* PT is 7 bit field, higher values not allowed */
+		if (pt >> 7) {
+			LOGP(DLMGCP, LOGL_ERROR, "SDP: payload type number out of range: %lu > 127\n", pt);
 			goto response_parse_failure_pt;
+		}
 
 		/* Do not allow duplicate payload types */
-		for (i = 0; i < count; i++)
-			if (r->codecs[i] == pt)
+		for (i = 0; i < ptmap_len; i++) {
+			if (r->ptmap[i].pt == pt) {
+				LOGP(DLMGCP, LOGL_ERROR, "SDP: payload type number %lu listed twice\n", pt);
 				goto response_parse_failure_pt;
+			}
+		}
 
-		/* Note: The payload type we store may not necessarly match
-		 * the codec types we have defined in enum mgcp_codecs. To
-		 * ensure that the end result only contains codec types which
-		 * match enum mgcp_codecs, we will go through afterwards and
-		 * remap the affected entries with the inrofmation we learn
-		 * from rtpmap */
-		r->codecs[count] = pt;
-		count++;
+		/* Do not allow excessive payload types */
+		if (ptmap_len >= ARRAY_SIZE(r->ptmap)) {
+			LOGP(DLMGCP, LOGL_ERROR,
+			     "SDP: can parse only up to %zu payload type numbers\n", ARRAY_SIZE(r->ptmap));
+			goto response_parse_failure_pt;
+		}
+
+		/* Some payload type numbers imply a specific codec. For those, using the PT number as enum mgcp_codecs
+		 * yields the correct result. If no more specific information on the codec follows in "a=rtpmap:N"
+		 * lines, then this default number takes over. This only applies for PT below the dynamic range (<96). */
+		if (pt < 96)
+			r->ptmap[ptmap_len].codec = pt;
+		else
+			r->ptmap[ptmap_len].codec = -1;
+		r->ptmap[ptmap_len].pt = pt;
+		ptmap_len++;
 	}
 
-	r->codecs_len = count;
+	/* Parsing succeeded, publish all entries. */
+	r->ptmap_len = ptmap_len;
 
 exit:
 	return 0;
@@ -365,15 +295,17 @@ response_parse_failure_pt:
 	return -EINVAL;
 }
 
-/* Parse a line like "m=audio 16002 RTP/AVP 98", extract port and payload types */
+/* Parse an 'a=...' parameter */
 static int mgcp_parse_audio_ptime_rtpmap(struct mgcp_response *r, const char *line)
 {
 	unsigned int pt;
-	char codec_resp[64];
+	unsigned int i;
+	char codec_resp[256];
 	int rc;
 
 #define A_PTIME "a=ptime:"
 #define A_RTPMAP "a=rtpmap:"
+#define A_FMTP "a=fmtp:"
 
 	if (osmo_str_startswith(line, A_PTIME)) {
 		if (sscanf(line, A_PTIME "%u", &r->ptime) != 1) {
@@ -382,23 +314,80 @@ static int mgcp_parse_audio_ptime_rtpmap(struct mgcp_response *r, const char *li
 			return -EINVAL;
 		}
 	} else if (osmo_str_startswith(line, A_RTPMAP)) {
-		if (sscanf(line, A_RTPMAP "%d %63s", &pt, codec_resp) != 2) {
+		if (sscanf(line, A_RTPMAP "%d %255s", &pt, codec_resp) != 2) {
 			LOGP(DLMGCP, LOGL_ERROR,
 			     "Failed to parse SDP parameter, invalid rtpmap: %s\n", osmo_quote_str(line, -1));
 			return -EINVAL;
 		}
-		if (r->ptmap_len >= ARRAY_SIZE(r->ptmap)) {
-			LOGP(DLMGCP, LOGL_ERROR, "No more space in ptmap array (len=%u)\n", r->ptmap_len);
-			return -ENOSPC;
-		}
 		rc = map_str_to_codec(codec_resp);
 		if (rc < 0) {
 			LOGP(DLMGCP, LOGL_ERROR,
 			     "Failed to parse SDP parameter, can't parse codec in rtpmap: %s\n", osmo_quote_str(line, -1));
 			return -EINVAL;
 		}
-		r->ptmap[r->ptmap_len].pt = pt;
-		r->ptmap[r->ptmap_len].codec = rc;
+
+		/* Earlier, a line like "m=audio 16002 RTP/AVP 98 112 3" established the desired order of payloads, now
+		 * enrich it with actual codec information provided by "a=rtpmap:..." entries.
+		 * For each, find the entry with the right pt number and add the info there. */
+
+		for (i = 0; i < r->ptmap_len; i++) {
+			if (r->ptmap[i].pt != pt)
+				continue;
+			r->ptmap[i].codec = rc;
+			return 0;
+		}
+
+		/* No entry was found. This is an error in the MGCP protocol, but let's just add another entry
+		 * anyway, to not make it look like it was never there. */
+		LOGP(DLMGCP, LOGL_ERROR,
+		     "error in MGCP message: 'a=rtpmap:%u' has no matching entry in 'm=audio ... %u'\n",
+		     pt, pt);
+		if (r->ptmap_len >= ARRAY_SIZE(r->ptmap)) {
+			LOGP(DLMGCP, LOGL_ERROR,
+			     "cannot parse all codecs: can only store up to %zu rtpmap entries.\n",
+			     ARRAY_SIZE(r->ptmap));
+			return -ENOSPC;
+		}
+		r->ptmap[r->ptmap_len] = (struct ptmap){
+			.pt = pt,
+			.codec = rc,
+		};
+		r->ptmap_len++;
+
+	} else if (osmo_str_startswith(line, A_FMTP)) {
+		if (sscanf(line, A_FMTP "%d %255s", &pt, codec_resp) != 2) {
+			LOGP(DLMGCP, LOGL_ERROR,
+			     "Failed to parse SDP parameter, invalid fmtp: %s\n", osmo_quote_str(line, -1));
+			return -EINVAL;
+		}
+
+		/* Earlier, a line like "m=audio 16002 RTP/AVP 98 112 3" established the desired order of payloads, now
+		 * enrich it with actual codec information provided by "a=rtpmap:..." entries.
+		 * For each, find the entry with the right pt number and add the info there. */
+
+		for (i = 0; i < r->ptmap_len; i++) {
+			if (r->ptmap[i].pt != pt)
+				continue;
+			OSMO_STRLCPY_ARRAY(r->ptmap[r->ptmap_len].fmtp, codec_resp);
+			return 0;
+		}
+
+		/* No entry was found. This is an error in the MGCP protocol, but let's just add another entry
+		 * anyway, to not make it look like it was never there. */
+		LOGP(DLMGCP, LOGL_ERROR,
+		     "error in MGCP message: 'a=fmtp:%u' has no matching entry in 'm=audio ... %u'\n",
+		     pt, pt);
+		if (r->ptmap_len >= ARRAY_SIZE(r->ptmap)) {
+			LOGP(DLMGCP, LOGL_ERROR,
+			     "cannot parse all codecs: can only store up to %zu rtpmap entries.\n",
+			     ARRAY_SIZE(r->ptmap));
+			return -ENOSPC;
+		}
+		r->ptmap[r->ptmap_len] = (struct ptmap){
+			.pt = pt,
+			.codec = -1,
+		};
+		OSMO_STRLCPY_ARRAY(r->ptmap[r->ptmap_len].fmtp, codec_resp);
 		r->ptmap_len++;
 	}
 
@@ -508,7 +497,6 @@ int mgcp_response_parse_params(struct mgcp_response *r)
 	int rc;
 	char *data;
 	char *data_ptr;
-	int i;
 
 	/* Since this functions performs a destructive parsing, we create a
 	 * local copy of the body data */
@@ -553,10 +541,6 @@ int mgcp_response_parse_params(struct mgcp_response *r)
 		}
 	}
 
-	/* See also note in mgcp_parse_audio_port_pt() */
-	for (i = 0; i < r->codecs_len; i++)
-	        r->codecs[i] =  map_pt_to_codec(r->ptmap, r->ptmap_len, r->codecs[i]);
-
 	rc = 0;
 exit:
 	talloc_free(data);
@@ -1234,7 +1218,6 @@ static int add_lco(struct msgb *msg, struct mgcp_msg *mgcp_msg)
 {
 	unsigned int i;
 	const char *codec;
-	unsigned int pt;
 
 #define MSGB_PRINTF_OR_RET(FMT, ARGS...) do { \
 		if (msgb_printf(msg, FMT, ##ARGS) != 0) { \
@@ -1248,11 +1231,10 @@ static int add_lco(struct msgb *msg, struct mgcp_msg *mgcp_msg)
 	if (mgcp_msg->ptime)
 		MSGB_PRINTF_OR_RET(" p:%u,", mgcp_msg->ptime);
 
-	if (mgcp_msg->codecs_len) {
+	if (mgcp_msg->ptmap_len) {
 		MSGB_PRINTF_OR_RET(" a:");
-		for (i = 0; i < mgcp_msg->codecs_len; i++) {
-			pt = mgcp_msg->codecs[i];
-			codec = get_value_string_or_null(osmo_mgcpc_codec_names, pt);
+		for (i = 0; i < mgcp_msg->ptmap_len; i++) {
+			codec = get_value_string_or_null(osmo_mgcpc_codec_names, mgcp_msg->ptmap[i].codec);
 
 			/* Note: Use codec descriptors from enum mgcp_codecs
 			 * in mgcp_client only! */
@@ -1260,7 +1242,7 @@ static int add_lco(struct msgb *msg, struct mgcp_msg *mgcp_msg)
 				return -EINVAL;
 
 			MSGB_PRINTF_OR_RET("%s", extract_codec_name(codec));
-			if (i < mgcp_msg->codecs_len - 1)
+			if (i < mgcp_msg->ptmap_len - 1)
 				MSGB_PRINTF_OR_RET(";");
 		}
 		MSGB_PRINTF_OR_RET(",");
@@ -1338,21 +1320,26 @@ static int add_sdp(struct msgb *msg, struct mgcp_msg *mgcp_msg, struct mgcp_clie
 			return -EINVAL;
 		}
 		MSGB_PRINTF_OR_RET("m=audio %u RTP/AVP", mgcp_msg->audio_port);
-		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]);
-			MSGB_PRINTF_OR_RET(" %u", pt);
-
-		}
+		for (i = 0; i < mgcp_msg->ptmap_len; i++)
+			MSGB_PRINTF_OR_RET(" %u", mgcp_msg->ptmap[i].pt);
 		MSGB_PRINTF_OR_RET("\r\n");
 	}
 
 	/* Add optional codec parameters (fmtp) */
-	if (mgcp_msg->param_present) {
-		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]);
+	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)
@@ -1360,14 +1347,14 @@ static int add_sdp(struct msgb *msg, struct mgcp_msg *mgcp_msg, struct mgcp_clie
 		}
 	}
 
-	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]);
+	for (i = 0; i < mgcp_msg->ptmap_len; i++) {
+		pt = mgcp_msg->ptmap[i].pt;
 
 		/* Note: Only dynamic payload type from the range 96-127
 		 * require to be explained further via rtpmap. All others
 		 * are implcitly definedby the number in m=audio */
 		if (pt >= 96 && pt <= 127) {
-			codec = get_value_string_or_null(osmo_mgcpc_codec_names, mgcp_msg->codecs[i]);
+			codec = get_value_string_or_null(osmo_mgcpc_codec_names, mgcp_msg->ptmap[i].codec);
 
 			/* Note: Use codec descriptors from enum mgcp_codecs
 			 * in mgcp_client only! */
@@ -1575,3 +1562,23 @@ const char *mgcp_client_name(const struct mgcp_client *mgcp)
 	else
 		return mgcp_client_endpoint_domain(mgcp);
 }
+
+/*! Return typical cmp result, comparing a to b.
+ * Return 0 if a == b, -1 if a < b, 1 if a > b; comparing all members of ptmap in turn. */
+int ptmap_cmp(const struct ptmap *a, const struct ptmap *b)
+{
+	int rc;
+	if (a == b)
+		return 0;
+	if (!a)
+		return -1;
+	if (!b)
+		return 1;
+	rc = OSMO_CMP(a->codec, b->codec);
+	if (rc)
+		return rc;
+	rc = OSMO_CMP(a->pt, b->pt);
+	if (rc)
+		return rc;
+	return strcmp(a->fmtp, b->fmtp);
+}

src/libosmo-mgcp-client/mgcp_client_endpoint_fsm.c

@@ -718,11 +718,11 @@ void osmo_mgcpc_ep_ci_request(struct osmo_mgcpc_ep_ci *ci,
 	osmo_strlcpy(cleared_ci.mgcp_ci_str, ci->mgcp_ci_str, sizeof(cleared_ci.mgcp_ci_str));
 	*ci = cleared_ci;
 
-	LOG_CI_VERB(ci, LOGL_DEBUG, "notify=%s\n", osmo_fsm_inst_name(ci->notify.fi));
-
 	if (verb_info)
 		ci->verb_info = *verb_info;
 
+	LOG_CI_VERB(ci, LOGL_DEBUG, "notify=%s\n", osmo_fsm_inst_name(ci->notify.fi));
+
 	if (ep->endpoint[0]) {
 		if (ci->verb_info.endpoint[0] && strcmp(ci->verb_info.endpoint, ep->endpoint))
 			LOG_CI(ci, LOGL_ERROR,

src/libosmo-mgcp-client/mgcp_client_fsm.c

@@ -115,12 +115,10 @@ static void make_crcx_msg(struct mgcp_msg *mgcp_msg, struct mgcp_conn_peer *info
 		.call_id = info->call_id,
 		.conn_mode = MGCP_CONN_RECV_ONLY,
 		.ptime = info->ptime,
-		.codecs_len = info->codecs_len,
 		.ptmap_len = info->ptmap_len,
 		.param_present = info->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));
 
@@ -173,12 +171,10 @@ static struct msgb *make_mdcx_msg(struct mgcp_ctx *mgcp_ctx)
 		.audio_ip = mgcp_ctx->conn_peer_local.addr,
 		.audio_port = mgcp_ctx->conn_peer_local.port,
 		.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
 	};
 	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));
 
@@ -628,6 +624,83 @@ static struct osmo_fsm fsm_mgcp_client = {
 	.log_subsys = DLMGCP,
 };
 
+/*! allocate struct to hold the description of an MGCP connection peer.
+ *  \param[in] ctx talloc context.
+ *  \returns newly-allocated and initialized struct mgcp_conn_peer. */
+struct mgcp_conn_peer *mgcp_conn_peer_alloc(void *ctx)
+{
+	struct mgcp_conn_peer *peer;
+	peer = talloc_zero(ctx, struct mgcp_conn_peer);
+	OSMO_ASSERT(peer);
+	return peer;
+}
+
+/* Provide backwards compat for deprecated conn_peer->codecs[]: when the caller passes in an mgcp_conn_peer instance
+ * that has codecs[] set, apply it to ptmap[] instead. */
+static void mgcp_conn_peer_compat(struct mgcp_conn_peer *conn_peer)
+{
+	struct ptmap ptmap[MGCP_MAX_CODECS];
+	unsigned int ptmap_len;
+
+	if (!conn_peer->codecs_len)
+		return;
+
+	/* Before dropping codecs[], codecs[] would indicate the order in which the codecs should appear in SDP. ptmap[]
+	 * would indicate payload type numbers when not using a default payload type number (may omit entries).
+	 * Now, ptmap[] just indicates both at the same time; codecs[] should be empty, and ptmap[] lists all codecs.
+	 * So if any codecs[] are present, recreate ptmap[] in the order of codecs[]. */
+
+	ptmap_len = 0;
+	for (int i = 0; i < conn_peer->codecs_len; i++) {
+		enum mgcp_codecs codec = conn_peer->codecs[i];
+		struct ptmap *found = NULL;
+
+		/* Look up whether a specific pt was indicated for this codec */
+		for (int p = 0; p < conn_peer->ptmap_len; p++) {
+			if (conn_peer->ptmap[p].codec != codec)
+				continue;
+			found = &conn_peer->ptmap[p];
+			break;
+		}
+		if (found) {
+			ptmap[ptmap_len] = *found;
+		} else {
+			ptmap[ptmap_len] = (struct ptmap){
+				.codec = codec,
+				/* some enum mgcp_codecs correspond to their standard PT nr, so for compat: */
+				.pt = codec,
+			};
+		}
+		ptmap_len++;
+	}
+
+	/* Are there any entries in the old ptmap that were omitted by codecs[]? */
+	for (int p = 0; p < conn_peer->ptmap_len; p++) {
+		bool exists = false;
+		for (int i = 0; i < ptmap_len; i++) {
+			if (ptmap_cmp(&ptmap[i], &conn_peer->ptmap[p]))
+				continue;
+			exists = true;
+			break;
+		}
+
+		if (exists)
+			continue;
+
+		if (ptmap_len >= ARRAY_SIZE(ptmap))
+			break;
+
+		/* Not present yet, add it to the end */
+		ptmap[ptmap_len] = conn_peer->ptmap[p];
+		ptmap_len++;
+	}
+
+	/* Use the new ptmap[], and clear out legacy codecs[]. */
+	memcpy(conn_peer->ptmap, ptmap, sizeof(conn_peer->ptmap));
+	conn_peer->ptmap_len = ptmap_len;
+	conn_peer->codecs_len = 0;
+}
+
 /*! allocate FSM, and create a new connection on the MGW.
  *  \param[in] mgcp MGCP client descriptor.
  *  \param[in] parent_fi Parent FSM instance.
@@ -642,6 +715,7 @@ struct osmo_fsm_inst *mgcp_conn_create(struct mgcp_client *mgcp, struct osmo_fsm
 	struct osmo_fsm_inst *fi;
 	struct in6_addr ip_test;
 
+	mgcp_conn_peer_compat(conn_peer);
 
 	OSMO_ASSERT(parent_fi);
 	OSMO_ASSERT(mgcp);
@@ -681,6 +755,8 @@ int mgcp_conn_modify(struct osmo_fsm_inst *fi, uint32_t parent_evt, struct mgcp_
 	struct mgcp_ctx *mgcp_ctx = fi->priv;
 	struct in6_addr ip_test;
 
+	mgcp_conn_peer_compat(conn_peer);
+
 	OSMO_ASSERT(mgcp_ctx);
 	OSMO_ASSERT(conn_peer);
 

src/libosmo-mgcp/Makefile.am

@@ -42,4 +42,5 @@ libosmo_mgcp_a_SOURCES = \
 	mgcp_ratectr.c \
 	mgcp_e1.c \
 	mgcp_iuup.c \
+	$(top_srcdir)/src/libosmo-mgcp-client/fmtp.c \
 	$(NULL)

src/libosmo-mgcp/mgcp_codec.c

@@ -24,6 +24,7 @@
 #include <osmocom/mgcp/mgcp_endp.h>
 #include <osmocom/mgcp/mgcp_trunk.h>
 #include <osmocom/mgcp/mgcp_codec.h>
+#include <osmocom/mgcp_client/fmtp.h>
 #include <errno.h>
 
 /* Helper function to dump codec information of a specified codec to a printable
@@ -116,9 +117,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 +262,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 +281,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 = OSMO_SDP_AMR_SET_OCTET_ALIGN(param->amr_octet_aligned);
+
+	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 */
@@ -414,6 +429,26 @@ static struct mgcp_rtp_codec *codec_find_convertible(struct mgcp_conn_rtp *conn,
 int mgcp_codec_decide(struct mgcp_conn_rtp *conn_src, struct mgcp_conn_rtp *conn_dst)
 {
 	unsigned int i;
+	if (log_check_level(DLMGCP, LOGL_DEBUG)) {
+		LOGP(DLMGCP, LOGL_DEBUG, "%s(): src.codecs_assigned=%d dst.codecs_assigned=%d\n",
+		     __func__,
+		     conn_src ? conn_src->end.codecs_assigned : 0,
+		     conn_dst ? conn_dst->end.codecs_assigned : 0);
+		if (conn_src) {
+			for (i = 0; i < conn_src->end.codecs_assigned; i++)  {
+				struct mgcp_rtp_codec *c = &conn_src->end.codecs[i];
+				LOGP(DLMGCP, LOGL_DEBUG, "src.codecs[%d]: %d %s %s %s\n",
+				     i, c->payload_type, c->audio_name, c->subtype_name, c->fmtp);
+			}
+		}
+		if (conn_dst) {
+			for (i = 0; i < conn_dst->end.codecs_assigned; i++)  {
+				struct mgcp_rtp_codec *c = &conn_dst->end.codecs[i];
+				LOGP(DLMGCP, LOGL_DEBUG, "dst.codecs[%d]: %d %s %s %s\n",
+				     i, c->payload_type, c->audio_name, c->subtype_name, c->fmtp);
+			}
+		}
+	}
 
 	/* In case no destination connection is available (yet), or in case the destination connection exists but has
 	 * no codecs assigned, we are forced to make a simple tentative decision:
@@ -431,13 +466,13 @@ int mgcp_codec_decide(struct mgcp_conn_rtp *conn_src, struct mgcp_conn_rtp *conn
 	 * of a match set this codec on both connections. This would be an ideal selection since no codec conversion would be
 	 * required. */
 	for (i = 0; i < conn_src->end.codecs_assigned; i++) {
-		struct mgcp_rtp_codec *codec_conn_dst = mgcp_codec_find_same(conn_dst, &conn_src->end.codecs[i]);
+		struct mgcp_rtp_codec *codec_conn_src = &conn_src->end.codecs[i];
+		struct mgcp_rtp_codec *codec_conn_dst = mgcp_codec_find_same(conn_dst, codec_conn_src);
 		if (codec_conn_dst) {
 			/* We found the a codec that is exactly the same (same codec, same payload format etc.) on both
 			 * sides. We now set this codec on both connections. */
 			conn_dst->end.codec = codec_conn_dst;
-			conn_src->end.codec = mgcp_codec_find_same(conn_src, codec_conn_dst);
-			OSMO_ASSERT(conn_src->end.codec);
+			conn_src->end.codec = codec_conn_src;
 			return 0;
 		}
 	}
@@ -445,30 +480,40 @@ int mgcp_codec_decide(struct mgcp_conn_rtp *conn_src, struct mgcp_conn_rtp *conn
 	/* In case we could not find a codec that is exactly the same, let's at least try to find a codec that we are able
 	 * to convert. */
 	for (i = 0; i < conn_src->end.codecs_assigned; i++) {
-		struct mgcp_rtp_codec *codec_conn_dst = codec_find_convertible(conn_dst, &conn_src->end.codecs[i]);
+		struct mgcp_rtp_codec *codec_conn_src = &conn_src->end.codecs[i];
+		struct mgcp_rtp_codec *codec_conn_dst = codec_find_convertible(conn_dst, codec_conn_src);
 		if (codec_conn_dst) {
-			/* We found the a codec that we are able to convert on both sides. We now set this codec on both
-			 * connections. */
+			/* We found the a codec that we can convert to. Set each side to its codec. */
 			conn_dst->end.codec = codec_conn_dst;
-			conn_src->end.codec = codec_find_convertible(conn_src, codec_conn_dst);
-			OSMO_ASSERT(conn_src->end.codec);
+			conn_src->end.codec = codec_conn_src;
 			return 0;
 		}
 	}
 
-	return -EINVAL;
+	LOGP(DLMGCP, LOGL_ERROR, "no matching codec found\n");
+	if (conn_dst->end.codecs_assigned)
+		conn_dst->end.codec = &conn_dst->end.codecs[0];
+	else
+		return -EINVAL;
+
+	if (conn_src->end.codecs_assigned)
+		conn_src->end.codec = &conn_src->end.codecs[0];
+	else
+		return -EINVAL;
+
+	return 0;
 }
 
 /* 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_NAME_AMR_OCTET_ALIGN);
 }
 
 /* Find the payload type number configured for a specific codec by SDP.

src/libosmo-mgcp/mgcp_conn.c

@@ -110,8 +110,6 @@ static int mgcp_rtp_conn_init(struct mgcp_conn_rtp *conn_rtp, struct mgcp_conn *
 	end->rtcp.fd = -1;
 	memset(&end->addr, 0, sizeof(end->addr));
 	end->rtcp_port = 0;
-	talloc_free(end->fmtp_extra);
-	end->fmtp_extra = NULL;
 
 	/* Set default values */
 	end->frames_per_packet = 0;	/* unknown */
@@ -359,6 +357,7 @@ char *mgcp_conn_dump(struct mgcp_conn *conn)
 	static char str[sizeof(conn->name)+sizeof(conn->id)+256];
 	char ipbuf[INET6_ADDRSTRLEN];
 	struct osmo_strbuf sb = { .buf = str, .len = sizeof(str) };
+	int i;
 
 	if (!conn)
 		return "NULL";
@@ -382,6 +381,11 @@ char *mgcp_conn_dump(struct mgcp_conn *conn)
 			break;
 		}
 
+		for (i = 0; i < conn->u.rtp.end.codecs_assigned; i++) {
+			struct mgcp_rtp_codec *c = &conn->u.rtp.end.codecs[i];
+			OSMO_STRBUF_PRINTF(sb, " %s#%d%s", c->subtype_name, c->payload_type, c->fmtp);
+		}
+
 		OSMO_STRBUF_PRINTF(sb, ")");
 		break;
 

src/libosmo-mgcp/mgcp_network.c

@@ -429,18 +429,6 @@ int mgcp_setup_rtp_processing_default(struct mgcp_endpoint *endp,
 	return 0;
 }
 
-void mgcp_get_net_downlink_format_default(struct mgcp_endpoint *endp,
-					  const struct mgcp_rtp_codec **codec,
-					  const char **fmtp_extra,
-					  struct mgcp_conn_rtp *conn)
-{
-	LOGPENDP(endp, DRTP, LOGL_DEBUG, "conn:%s using format defaults\n",
-		 mgcp_conn_dump(conn->conn));
-
-	*codec = conn->end.codec;
-	*fmtp_extra = conn->end.fmtp_extra;
-}
-
 void mgcp_rtp_annex_count(const struct mgcp_endpoint *endp,
 			  struct mgcp_rtp_state *state, const uint16_t seq,
 			  const int32_t transit, const uint32_t ssrc,
@@ -1201,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 &&
@@ -1517,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;
 		}
 	}

src/libosmo-mgcp/mgcp_protocol.c

@@ -397,6 +397,23 @@ struct msgb *mgcp_handle_message(struct mgcp_config *cfg, struct msgb *msg)
 		rq.null_endp = mgcp_endp_is_null(pdata.epname);
 	if (!rq.null_endp)
 		rq.endp = mgcp_endp_by_name(&rc, pdata.epname, pdata.cfg);
+	if (rq.endp) {
+		struct mgcp_conn *c;
+		int count = 0;
+		llist_for_each_entry(c, &rq.endp->conns, entry) {
+			LOGP(DLMGCP, LOGL_DEBUG,
+			     "%s: endp=%s conn %s\n",
+			     rq.name,
+			     rq.endp->name,
+			     mgcp_conn_dump(c));
+			count++;
+		}
+		if (!count)
+			LOGP(DLMGCP, LOGL_DEBUG,
+			     "%s: endp=%s no conns\n",
+			     rq.name,
+			     rq.endp->name);
+	}
 	rq.mgcp_cause = rc;
 	if (!rq.endp) {
 		rate_ctr_inc(rate_ctr_group_get_ctr(rate_ctrs, MGCP_GENERAL_RX_FAIL_NO_ENDPOINT));
@@ -429,6 +446,7 @@ struct msgb *mgcp_handle_message(struct mgcp_config *cfg, struct msgb *msg)
 		/* Check if we have to retransmit a response from a previous transaction */
 		if (pdata.trans && rq.endp->last_trans && strcmp(rq.endp->last_trans, pdata.trans) == 0) {
 			rate_ctr_inc(rate_ctr_group_get_ctr(rate_ctrs, MGCP_GENERAL_RX_MSGS_RETRANSMITTED));
+			LOGP(DLMGCP, LOGL_DEBUG, "%s: retransmission\n", rq.name);
 			return create_retransmission_response(rq.endp);
 		}
 	}
@@ -1076,9 +1094,6 @@ mgcp_header_done:
 		rc = mgcp_conn_iuup_init(conn);
 	}
 
-	conn->end.fmtp_extra = talloc_strdup(trunk->endpoints,
-					     trunk->audio_fmtp_extra);
-
 	if (pdata->cfg->force_ptime) {
 		conn->end.packet_duration_ms = pdata->cfg->force_ptime;
 		conn->end.force_output_ptime = 1;
@@ -1134,6 +1149,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);
 
@@ -1712,8 +1728,6 @@ struct mgcp_config *mgcp_config_alloc(void)
 	cfg->rtp_processing_cb = &mgcp_rtp_processing_default;
 	cfg->setup_rtp_processing_cb = &mgcp_setup_rtp_processing_default;
 
-	cfg->get_net_downlink_format_cb = &mgcp_get_net_downlink_format_default;
-
 	INIT_LLIST_HEAD(&cfg->trunks);
 
 	/* Allocate virtual trunk */

src/libosmo-mgcp/mgcp_sdp.c

@@ -34,6 +34,7 @@
 #include <osmocom/mgcp/mgcp_codec.h>
 #include <osmocom/mgcp/mgcp_sdp.h>
 #include <osmocom/mgcp/mgcp_protocol.h>
+#include <osmocom/mgcp_client/fmtp.h>
 
 #include <errno.h>
 #include <stdlib.h>
@@ -55,7 +56,7 @@ struct sdp_rtp_map {
 };
 struct sdp_fmtp_param {
 	int payload_type;
-	struct mgcp_codec_param param;
+	const char *fmtp;
 };
 
 
@@ -195,11 +196,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 +215,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 +269,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 +296,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 +390,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 +405,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");
 
@@ -447,35 +418,34 @@ int mgcp_parse_sdp_data(const struct mgcp_endpoint *endp,
 }
 
 
-/* Add rtpmap string to the sdp payload, but only when the payload type falls
- * into the dynamic payload type range */
-static int add_rtpmap(struct msgb *sdp, int payload_type, const char *audio_name)
-{
-	int rc;
-
-	if (payload_type >= 96 && payload_type <= 127) {
-		if (!audio_name)
-			return -EINVAL;
-		rc = msgb_printf(sdp, "a=rtpmap:%d %s\r\n", payload_type, audio_name);
-		if (rc < 0)
-			return -EINVAL;
-	}
-
-	return 0;
-}
-
-/* Add audio strings to sdp payload */
-static int add_audio(struct msgb *sdp, int *payload_types, unsigned int payload_types_len, int local_port)
+/* Add all codecs related lines to SDP payload */
+static int add_codecs(struct msgb *sdp, const struct mgcp_conn_rtp *conn)
 {
 	int rc;
 	unsigned int i;
+	int local_port;
+	struct mgcp_trunk *trunk = conn->conn->endp->trunk;
+
+	if (!conn->end.codecs_assigned)
+		return 0;
+
+	/* Compose 'm=audio 1234 RTP/AVP 112 96 3' line, with
+	 * - local RTP port
+	 * - a list of all assigned payload type numbers
+	 */
+
+	if (mgcp_conn_rtp_is_osmux(conn))
+		local_port = trunk->cfg->osmux.local_port;
+	else
+		local_port = conn->end.local_port;
 
 	rc = msgb_printf(sdp, "m=audio %d RTP/AVP", local_port);
 	if (rc < 0)
 		return -EINVAL;
 
-	for (i = 0; i < payload_types_len; i++) {
-		rc = msgb_printf(sdp, " %d", payload_types[i]);
+	for (i = 0; i < conn->end.codecs_assigned; i++) {
+		const struct mgcp_rtp_codec *c = &conn->end.codecs[i];
+		rc = msgb_printf(sdp, " %d", c->payload_type);
 		if (rc < 0)
 			return -EINVAL;
 	}
@@ -484,62 +454,45 @@ static int add_audio(struct msgb *sdp, int *payload_types, unsigned int payload_
 	if (rc < 0)
 		return -EINVAL;
 
-	return 0;
-}
+	/* Compose 'a=rtpmap:N FOO' lines for codecs in above list that require it.
+	 * e.g. GSM-FR is implicitly defined by payload type number 3, so it is enough to list 3 above;
+	 * AMR needs a line like 'a=rtpmap:112 AMR/8000/1' in addition to listing 112 above.
+	 */
+	for (i = 0; i < conn->end.codecs_assigned; i++) {
+		const struct mgcp_rtp_codec *c = &conn->end.codecs[i];
+		if (!c->audio_name[0])
+			continue;
 
-/* Add fmtp strings to sdp payload */
-static int add_fmtp(struct msgb *sdp, struct sdp_fmtp_param *fmtp_params, unsigned int fmtp_params_len,
-		    const char *fmtp_extra)
-{
-	unsigned int i;
-	int rc;
-	int fmtp_extra_pt = -1;
-	char *fmtp_extra_pars = "";
-
-	/* When no fmtp parameters ara available but an fmtp extra string
-	 * is configured, just add the fmtp extra string */
-	if (fmtp_params_len == 0 && fmtp_extra) {
-		return msgb_printf(sdp, "%s\r\n", fmtp_extra);
-	}
-
-	/* When there is fmtp extra configured we dissect it in order to drop
-	 * in the configured extra parameters at the right place when
-	 * generating the fmtp strings. */
-	if (fmtp_extra) {
-		if (sscanf(fmtp_extra, "a=fmtp:%d ", &fmtp_extra_pt) != 1)
-			fmtp_extra_pt = -1;
-
-		fmtp_extra_pars = strstr(fmtp_extra, " ");
-
-		if (!fmtp_extra_pars)
-			fmtp_extra_pars = "";
-		else
-			fmtp_extra_pars++;
-	}
-
-	for (i = 0; i < fmtp_params_len; i++) {
-		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)
+		/* Dynamic payload type numbers need explicit rtpmap defining the codec by "subtype name" like "AMR" or
+		 * "GSM-HR-08". Others are defined implicitly, like GSM-FR by payload type number 3.
+		 *
+		 * Also, if the trunk is configured as "no sdp audio-payload send-name", omit all rtpmap lines.
+		 */
+		if (c->payload_type >= 96 && c->payload_type <= 127
+		    && trunk->audio_send_name) {
+			if (msgb_printf(sdp, "a=rtpmap:%d %s\r\n", c->payload_type, c->audio_name) < 0)
 				return -EINVAL;
 		}
 
-		/* Append extra parameters from fmtp extra */
-		if (fmtp_params[i].payload_type == fmtp_extra_pt) {
-			rc = msgb_printf(sdp, " %s", fmtp_extra_pars);
-			if (rc < 0)
+		/* Compose 'a=fmtp:N foo=bar' line if fmtp is defined for this codec.
+		 * e.g. AMR has fmtp like 'octet-align=1', 'mode-set=0,2,4,7'.
+		 */
+		if (c->fmtp[0]) {
+			if (msgb_printf(sdp, OSMO_SDP_PREFIX_A_FMTP "%d %s\r\n", c->payload_type, c->fmtp) < 0)
 				return -EINVAL;
 		}
+		else if (c->param_present) {
+			/* Legacy */
+			if (msgb_printf(sdp, OSMO_SDP_PREFIX_A_FMTP "%d %s\r\n", c->payload_type,
+					OSMO_SDP_AMR_SET_OCTET_ALIGN(c->param.amr_octet_aligned))
+			    < 0)
+				return -EINVAL;
+		}
+	}
 
-		rc = msgb_printf(sdp, "\r\n");
+	if (conn->end.packet_duration_ms > 0 && conn->conn->endp->trunk->audio_send_ptime) {
+		rc = msgb_printf(sdp, "a=ptime:%u\r\n",
+				 conn->end.packet_duration_ms);
 		if (rc < 0)
 			return -EINVAL;
 	}
@@ -557,16 +510,7 @@ int mgcp_write_response_sdp(const struct mgcp_endpoint *endp,
 			    const struct mgcp_conn_rtp *conn, struct msgb *sdp,
 			    const char *addr)
 {
-	const struct mgcp_rtp_codec *codec;
-	const char *fmtp_extra;
-	const char *audio_name;
-	int payload_type;
-	struct sdp_fmtp_param fmtp_param;
 	int rc;
-	int payload_types[1];
-	int local_port;
-	struct sdp_fmtp_param fmtp_params[1];
-        unsigned int fmtp_params_len = 0;
 	bool addr_is_v6;
 
 	OSMO_ASSERT(endp);
@@ -574,14 +518,6 @@ int mgcp_write_response_sdp(const struct mgcp_endpoint *endp,
 	OSMO_ASSERT(sdp);
 	OSMO_ASSERT(addr);
 
-	/* FIXME: constify endp and conn args in get_net_donwlink_format_cb() */
-	endp->trunk->cfg->get_net_downlink_format_cb((struct mgcp_endpoint *)endp,
-					      &codec, &fmtp_extra,
-					      (struct mgcp_conn_rtp *)conn);
-
-	audio_name = codec->audio_name;
-	payload_type = codec->payload_type;
-
 	addr_is_v6 = osmo_ip_str_type(addr) == AF_INET6;
 
 	rc = msgb_printf(sdp,
@@ -596,43 +532,14 @@ int mgcp_write_response_sdp(const struct mgcp_endpoint *endp,
 	if (rc < 0)
 		goto buffer_too_small;
 
-	if (payload_type >= 0) {
-
-		payload_types[0] = payload_type;
-		if (mgcp_conn_rtp_is_osmux(conn))
-			local_port = endp->trunk->cfg->osmux.local_port;
-		else
-			local_port = conn->end.local_port;
-		rc = add_audio(sdp, payload_types, 1, local_port);
-		if (rc < 0)
-			goto buffer_too_small;
-
-		if (endp->trunk->audio_send_name) {
-			rc = add_rtpmap(sdp, payload_type, audio_name);
-			if (rc < 0)
-				goto buffer_too_small;
-		}
-
-		if (codec->param_present) {
-			fmtp_param.payload_type = payload_type;
-			fmtp_param.param = codec->param;
-			fmtp_params[0] = fmtp_param;
-			fmtp_params_len = 1;
-		}
-		rc = add_fmtp(sdp, fmtp_params, fmtp_params_len, fmtp_extra);
-		if (rc < 0)
-			goto buffer_too_small;
-	}
-	if (conn->end.packet_duration_ms > 0 && endp->trunk->audio_send_ptime) {
-		rc = msgb_printf(sdp, "a=ptime:%u\r\n",
-				 conn->end.packet_duration_ms);
-		if (rc < 0)
-			goto buffer_too_small;
-	}
+	/* Add all codecs related SDP lines */
+	rc = add_codecs(sdp, conn);
+	if (rc < 0)
+		goto buffer_too_small;
 
 	return 0;
 
 buffer_too_small:
-	LOGPCONN(conn->conn, DLMGCP, LOGL_ERROR, "SDP messagebuffer too small\n");
+	LOGPCONN(conn->conn, DLMGCP, LOGL_ERROR, "SDP message too large for buffer\n");
 	return -1;
 }

src/libosmo-mgcp/mgcp_vty.c

@@ -111,9 +111,6 @@ static int config_write_mgcp(struct vty *vty)
 			VTY_NEWLINE);
 	} else
 		vty_out(vty, " no rtp-patch%s", VTY_NEWLINE);
-	if (trunk->audio_fmtp_extra)
-		vty_out(vty, " sdp audio fmtp-extra %s%s",
-			trunk->audio_fmtp_extra, VTY_NEWLINE);
 	vty_out(vty, " %ssdp audio-payload send-ptime%s",
 		trunk->audio_send_ptime ? "" : "no ", VTY_NEWLINE);
 	vty_out(vty, " %ssdp audio-payload send-name%s",
@@ -187,7 +184,7 @@ static void dump_rtp_end(struct vty *vty, struct mgcp_conn_rtp *conn)
 		"   Payload Type: %d Rate: %u Channels: %d %s"
 		"   Frame Duration: %u Frame Denominator: %u%s"
 		"   FPP: %d Packet Duration: %u%s"
-		"   FMTP-Extra: %s Audio-Name: %s Sub-Type: %s%s"
+		"   Audio-Name: %s Sub-Type: %s%s"
 		"   Output-Enabled: %d Force-PTIME: %d%s",
 		tx_packets->current, tx_bytes->current, VTY_NEWLINE,
 		rx_packets->current, rx_bytes->current, VTY_NEWLINE,
@@ -198,7 +195,7 @@ static void dump_rtp_end(struct vty *vty, struct mgcp_conn_rtp *conn)
 		codec->payload_type, codec->rate, codec->channels, VTY_NEWLINE,
 		codec->frame_duration_num, codec->frame_duration_den,
 		VTY_NEWLINE, end->frames_per_packet, end->packet_duration_ms,
-		VTY_NEWLINE, end->fmtp_extra, codec->audio_name,
+		VTY_NEWLINE, codec->audio_name,
 		codec->subtype_name, VTY_NEWLINE, end->output_enabled,
 		end->force_output_ptime, VTY_NEWLINE);
 	if (mgcp_conn_rtp_is_osmux(conn)) {
@@ -681,21 +678,15 @@ DEFUN_USRATTR(cfg_mgcp_no_rtp_force_ptime,
 	return CMD_SUCCESS;
 }
 
-DEFUN_USRATTR(cfg_mgcp_sdp_fmtp_extra,
-	      cfg_mgcp_sdp_fmtp_extra_cmd,
-	      X(MGW_CMD_ATTR_NEWCONN),
-	      "sdp audio fmtp-extra .NAME",
-	      "Add extra fmtp for the SDP file\n" "Audio\n" "Fmtp-extra\n"
-	      "Extra Information\n")
-{
-	struct mgcp_trunk *trunk = mgcp_trunk_by_num(g_cfg, MGCP_TRUNK_VIRTUAL, MGCP_VIRT_TRUNK_ID);
-	OSMO_ASSERT(trunk);
-	char *txt = argv_concat(argv, argc, 0);
-	if (!txt)
-		return CMD_WARNING;
+#define SDP_STR "SDP File related options\n"
+#define AUDIO_STR "Audio payload options\n"
 
-	osmo_talloc_replace_string(g_cfg, &trunk->audio_fmtp_extra, txt);
-	talloc_free(txt);
+DEFUN_DEPRECATED(cfg_mgcp_sdp_fmtp_extra,
+	      cfg_mgcp_sdp_fmtp_extra_cmd,
+	      "sdp audio fmtp-extra .NAME",
+	      SDP_STR AUDIO_STR "Deprecated, without effect since osmo-mgw v1.13\n" "Deprecated, without effect\n")
+{
+	vty_out(vty, "%% deprecated: the config option 'sdp audio fmtp-extra' has been removed.%s", VTY_NEWLINE);
 	return CMD_SUCCESS;
 }
 
@@ -715,8 +706,6 @@ DEFUN_DEPRECATED(cfg_mgcp_no_allow_transcoding,
 	return CMD_SUCCESS;
 }
 
-#define SDP_STR "SDP File related options\n"
-#define AUDIO_STR "Audio payload options\n"
 DEFUN_DEPRECATED(cfg_mgcp_sdp_payload_number,
       cfg_mgcp_sdp_payload_number_cmd,
       "sdp audio-payload number <0-255>",
@@ -1062,28 +1051,17 @@ static int config_write_trunk(struct vty *vty)
 				VTY_NEWLINE);
 		} else
 			vty_out(vty, "  no rtp-patch%s", VTY_NEWLINE);
-		if (trunk->audio_fmtp_extra)
-			vty_out(vty, "   sdp audio fmtp-extra %s%s",
-				trunk->audio_fmtp_extra, VTY_NEWLINE);
 	}
 
 	return CMD_SUCCESS;
 }
 
-DEFUN_USRATTR(cfg_trunk_sdp_fmtp_extra,
+DEFUN_DEPRECATED(cfg_trunk_sdp_fmtp_extra,
 	      cfg_trunk_sdp_fmtp_extra_cmd,
-	      X(MGW_CMD_ATTR_NEWCONN),
 	      "sdp audio fmtp-extra .NAME",
-	      "Add extra fmtp for the SDP file\n" "Audio\n" "Fmtp-extra\n"
-	      "Extra Information\n")
+	      SDP_STR AUDIO_STR "Deprecated, without effect since osmo-mgw v1.13\n" "Deprecated, without effect\n")
 {
-	struct mgcp_trunk *trunk = vty->index;
-	char *txt = argv_concat(argv, argc, 0);
-	if (!txt)
-		return CMD_WARNING;
-
-	osmo_talloc_replace_string(g_cfg, &trunk->audio_fmtp_extra, txt);
-	talloc_free(txt);
+	vty_out(vty, "%% deprecated: the config option 'sdp audio fmtp-extra' has been removed.%s", VTY_NEWLINE);
 	return CMD_SUCCESS;
 }
 

tests/mgcp/Makefile.am

@@ -45,3 +45,6 @@ mgcp_test_LDADD = \
 	$(LIBOSMONETIF_LIBS) \
 	-lm  \
 	$(NULL)
+
+update_exp:
+	$(builddir)/mgcp_test >$(srcdir)/mgcp_test.ok

tests/mgcp/mgcp_test.c

@@ -127,7 +127,6 @@ static void test_strline(void)
 	"t=0 0\r\n" \
 	"m=audio 16006 RTP/AVP 97\r\n" \
 	"a=rtpmap:97 GSM-EFR/8000\r\n" \
-	"a=fmtp:126 0/1/2\r\n" \
 	"a=ptime:40\r\n"
 
 #define MDCX4_ADDR0000 \
@@ -336,7 +335,6 @@ static void test_strline(void)
 	"t=0 0\r\n" \
 	"m=audio 16006 RTP/AVP 97\r\n" \
 	"a=rtpmap:97 GSM-EFR/8000\r\n" \
-	"a=fmtp:126 0/1/2\r\n" \
 	"a=ptime:40\r\n"
 
 #define CRCX_ZYN \
@@ -552,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 \
@@ -586,7 +584,6 @@ struct mgcp_test {
 	const char *req;
 	const char *exp_resp;
 	int ptype;
-	const char *extra_fmtp;
 };
 
 static const struct mgcp_test tests[] = {
@@ -614,10 +611,9 @@ static const struct mgcp_test tests[] = {
 	{"RQNT1", RQNT, RQNT1_RET},
 	{"RQNT2", RQNT2, RQNT2_RET},
 	{"DLCX", DLCX, DLCX_RET, PTYPE_IGNORE},
-	{"CRCX", CRCX, CRCX_FMTP_RET, 97,.extra_fmtp = "a=fmtp:126 0/1/2"},
-	{"MDCX3", MDCX3, MDCX3_FMTP_RET, PTYPE_NONE,.extra_fmtp =
-	 "a=fmtp:126 0/1/2"},
-	{"DLCX", DLCX, DLCX_RET, PTYPE_IGNORE,.extra_fmtp = "a=fmtp:126 0/1/2"},
+	{"CRCX", CRCX, CRCX_FMTP_RET, 97},
+	{"MDCX3", MDCX3, MDCX3_FMTP_RET, PTYPE_NONE},
+	{"DLCX", DLCX, DLCX_RET, PTYPE_IGNORE},
 	{"CRCX", CRCX_NO_LCO_NO_SDP, CRCX_NO_LCO_NO_SDP_RET, 97},
 	{"CRCX", CRCX_X_OSMO_IGN, CRCX_X_OSMO_IGN_RET, 97},
 	{"MDCX_TOO_LONG_CI", MDCX_TOO_LONG_CI, MDCX_TOO_LONG_CI_RET},
@@ -627,6 +623,139 @@ static const struct mgcp_test tests[] = {
 	{"MDCX_NULL", MDCX_NULL, MDCX_NULL_RET},
 	{"DLCX_NULL", DLCX_NULL, DLCX_NULL_RET},
 	{"RQNT_NULL", RQNT_NULL, RQNT_NULL_RET},
+	{
+		"CRCX_EXPLICIT_EP",
+		/* CRCX for a new endpoint 8@mgw, not using the '*@mgw' wildcard */
+		"CRCX 101 rtpbridge/8@mgw MGCP 1.0\r\n"
+			"m: recvonly\r\n"
+			"C: 2\r\n"
+			"L: p:20\r\n"
+			"\r\n"
+			"v=0\r\n"
+			"c=IN IP4 1.2.3.4\r\n"
+			"m=audio 1234 RTP/AVP 112\r\n"
+			"a=rtpmap:112 AMR/8000\r\n"
+			"a=ptime:20\r\n",
+		"200 101 OK\r\n"
+			"I: %s\r\n"
+			"\r\n"
+			"v=0\r\n"
+			"o=- %s 23 IN IP4 0.0.0.0\r\n"
+			"s=-\r\n"
+			"c=IN IP4 0.0.0.0\r\n"
+			"t=0 0\r\n"
+			"m=audio 16014 RTP/AVP 112\r\n"
+			"a=rtpmap:112 AMR/8000\r\n"
+			"a=ptime:20\r\n"
+	},
+	{
+		"CRCX_TWO_PAYLOADS_1",
+		"CRCX 102 rtpbridge/*@mgw MGCP 1.0\r\n"
+			"m: recvonly\r\n"
+			"C: 2\r\n"
+			"L: p:20\r\n"
+			"\r\n"
+			"v=0\r\n"
+			"c=IN IP4 1.2.3.4\r\n"
+			"m=audio 1234 RTP/AVP 112 3\r\n"
+			"a=rtpmap:112 AMR/8000\r\n"
+			"a=ptime:20\r\n",
+		"200 102 OK\r\n"
+			"Z: rtpbridge/2@mgw\r\n"
+			"I: %s\r\n"
+			"\r\n"
+			"v=0\r\n"
+			"o=- %s 23 IN IP4 0.0.0.0\r\n"
+			"s=-\r\n"
+			"c=IN IP4 0.0.0.0\r\n"
+			"t=0 0\r\n"
+			"m=audio 16016 RTP/AVP 112 3\r\n"
+			"a=rtpmap:112 AMR/8000\r\n"
+			"a=ptime:20\r\n"
+	},
+	{
+		"CRCX_TWO_PAYLOADS_2",
+		"CRCX 103 rtpbridge/2@mgw MGCP 1.0\r\n"
+			"m: recvonly\r\n"
+			"C: 2\r\n"
+			"L: p:20\r\n"
+			"\r\n"
+			"v=0\r\n"
+			"c=IN IP4 1.2.3.4\r\n"
+			"m=audio 1234 RTP/AVP 3 112\r\n"
+			"a=rtpmap:112 AMR/8000\r\n"
+			"a=ptime:20\r\n",
+		"200 103 OK\r\n"
+			"I: %s\r\n"
+			"\r\n"
+			"v=0\r\n"
+			"o=- %s 23 IN IP4 0.0.0.0\r\n"
+			"s=-\r\n"
+			"c=IN IP4 0.0.0.0\r\n"
+			"t=0 0\r\n"
+			"m=audio 16018 RTP/AVP 3 112\r\n"
+			"a=rtpmap:112 AMR/8000\r\n"
+			"a=ptime:20\r\n"
+	},
+	{
+		"CRCX_THREE_PAYLOADS_1",
+		"CRCX 104 rtpbridge/*@mgw MGCP 1.0\r\n"
+			"m: recvonly\r\n"
+			"C: 4\r\n"
+			"L: p:20\r\n"
+			"\r\n"
+			"v=0\r\n"
+			"c=IN IP4 1.2.3.4\r\n"
+			"m=audio 1234 RTP/AVP 112 3 111\r\n"
+			"a=rtpmap:112 AMR/8000\r\n"
+			"a=fmtp:112 octet-align=1;mode-set=0,2,4,7\r\n"
+			"a=rtpmap:111 GSM-HR-08/8000\r\n"
+			"a=ptime:20\r\n",
+		"200 104 OK\r\n"
+			"Z: rtpbridge/3@mgw\r\n"
+			"I: %s\r\n"
+			"\r\n"
+			"v=0\r\n"
+			"o=- %s 23 IN IP4 0.0.0.0\r\n"
+			"s=-\r\n"
+			"c=IN IP4 0.0.0.0\r\n"
+			"t=0 0\r\n"
+			"m=audio 16020 RTP/AVP 112 3 111\r\n"
+			"a=rtpmap:112 AMR/8000\r\n"
+			"a=fmtp:112 octet-align=1;mode-set=0,2,4,7\r\n"
+			"a=rtpmap:111 GSM-HR-08/8000\r\n"
+			"a=ptime:20\r\n"
+	},
+	{
+		"CRCX_THREE_PAYLOADS_2",
+		"CRCX 105 rtpbridge/3@mgw MGCP 1.0\r\n"
+			"m: recvonly\r\n"
+			"C: 4\r\n"
+			"L: p:20\r\n"
+			"\r\n"
+			"v=0\r\n"
+			"c=IN IP4 1.2.3.4\r\n"
+			"m=audio 1234 RTP/AVP 3 112 113\r\n"
+			"a=rtpmap:112 AMR/8000\r\n"
+			"a=fmtp:112 octet-align=1;mode-set=0,2,4,7\r\n"
+			"a=rtpmap:113 AMR/8000\r\n"
+			"a=fmtp:113 octet-align=1;mode-set=0,2,4\r\n"
+			"a=ptime:20\r\n",
+		"200 105 OK\r\n"
+			"I: %s\r\n"
+			"\r\n"
+			"v=0\r\n"
+			"o=- %s 23 IN IP4 0.0.0.0\r\n"
+			"s=-\r\n"
+			"c=IN IP4 0.0.0.0\r\n"
+			"t=0 0\r\n"
+			"m=audio 16022 RTP/AVP 3 112 113\r\n"
+			"a=rtpmap:112 AMR/8000\r\n"
+			"a=fmtp:112 octet-align=1;mode-set=0,2,4,7\r\n"
+			"a=rtpmap:113 AMR/8000\r\n"
+			"a=fmtp:113 octet-align=1;mode-set=0,2,4\r\n"
+			"a=ptime:20\r\n"
+	},
 };
 
 static const struct mgcp_test retransmit[] = {
@@ -827,13 +956,10 @@ static void test_messages(void)
 		struct msgb *msg;
 
 		printf("\n================================================\n");
-		printf("Testing %s\n", t->name);
+		printf("Testing %s() %s\n", __func__, t->name);
 
 		dummy_packets = 0;
 
-		osmo_talloc_replace_string(cfg, &trunk->audio_fmtp_extra,
-					   t->extra_fmtp);
-
 		inp = create_msg(t->req, last_conn_id);
 		msg = mgcp_handle_message(cfg, inp);
 		msgb_free(inp);
@@ -845,6 +971,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);
 		}
 
@@ -934,6 +1061,9 @@ static void test_messages(void)
 			/* Reset them again for next test */
 			conn->end.codec->payload_type = PTYPE_NONE;
 		}
+
+		fflush(stdout);
+		fflush(stderr);
 	}
 
 	mgcp_endpoints_release(trunk);
@@ -962,7 +1092,7 @@ static void test_retransmission(void)
 		struct msgb *msg;
 
 		printf("\n================================================\n");
-		printf("Testing %s\n", t->name);
+		printf("Testing %s() %s\n", __func__, t->name);
 
 		inp = create_msg(t->req, last_conn_id);
 		msg = mgcp_handle_message(cfg, inp);
@@ -992,6 +1122,9 @@ static void test_retransmission(void)
 			OSMO_ASSERT(false);
 		}
 		msgb_free(msg);
+
+		fflush(stdout);
+		fflush(stderr);
 	}
 
 	mgcp_endpoints_release(trunk);
@@ -1620,6 +1753,7 @@ static void test_no_cycle(void)
 	talloc_free(cfg);
 }
 
+/* Set audio_send_name=0 and verify that a=rtpmap: entries are omitted. */
 static void test_no_name(void)
 {
 	struct mgcp_trunk *trunk;
@@ -2144,7 +2278,7 @@ static bool codec_decision(struct mgcp_conn_rtp *conn, unsigned int index_conn_s
 static void test_mgcp_codec_decide(void)
 {
 	int i;
-	bool ok = true;
+	bool ok_all = true;
 	printf("\nTesting mgcp_codec_find_convertible()\n");
 
 	for (i = 0; i < ARRAY_SIZE(test_mgcp_codec_find_convertible_cases); i++) {
@@ -2153,6 +2287,7 @@ static void test_mgcp_codec_decide(void)
 		int rc;
 		int conn_i;
 		int c;
+		bool ok = true;
 
 		printf("#%d: %s\n", i, t->descr);
 
@@ -2193,9 +2328,12 @@ static void test_mgcp_codec_decide(void)
 			printf(" ===> SUCCESS: codec decision as expected!\n");
 		else
 			printf(" ===> FAIL: unexpected codec decision!\n");
+
+		if (!ok)
+			ok_all = false;
 	}
 
-	OSMO_ASSERT(ok);
+	OSMO_ASSERT(ok_all);
 }
 
 void test_conn_id_matching(void)
@@ -2334,6 +2472,13 @@ int main(int argc, char **argv)
 	void *ctx = talloc_named_const(NULL, 0, "mgcp_test");
 	void *msgb_ctx = msgb_talloc_ctx_init(ctx, 0);
 	osmo_init_logging2(ctx, &log_info);
+	log_set_print_filename2(osmo_stderr_target, LOG_FILENAME_BASENAME);
+	log_set_print_filename_pos(osmo_stderr_target, LOG_FILENAME_POS_LINE_END);
+	log_set_print_level(osmo_stderr_target, 1);
+	log_set_print_category(osmo_stderr_target, 1);
+	log_set_print_category_hex(osmo_stderr_target, 0);
+	log_set_use_color(osmo_stderr_target, 1);
+	log_set_category_filter(osmo_stderr_target, DLMGCP, true, LOGL_DEBUG);
 
 	test_strline();
 	test_values();

tests/mgcp/mgcp_test.ok

@@ -13,7 +13,7 @@ line: ''
 line: ''
 
 ================================================
-Testing AUEP1
+Testing test_messages() AUEP1
 creating message from statically defined input:
 ---------8<---------
 AUEP 158663169 ds/e1-1/2@mgw MGCP 1.0
@@ -25,7 +25,7 @@ Response matches our expectations.
 (response does not contain a connection id)
 
 ================================================
-Testing AUEP2
+Testing test_messages() AUEP2
 creating message from statically defined input:
 ---------8<---------
 AUEP 18983213 ds/e1-2/1@mgw MGCP 1.0
@@ -37,7 +37,7 @@ Response matches our expectations.
 (response does not contain a connection id)
 
 ================================================
-Testing MDCX1
+Testing test_messages() MDCX1
 creating message from statically defined input:
 ---------8<---------
 MDCX 18983213 ds/e1-3/1@mgw MGCP 1.0
@@ -49,7 +49,7 @@ Response matches our expectations.
 (response does not contain a connection id)
 
 ================================================
-Testing MDCX2
+Testing test_messages() MDCX2
 creating message from statically defined input:
 ---------8<---------
 MDCX 18983214 ds/e1-1/2@mgw MGCP 1.0
@@ -61,7 +61,7 @@ Response matches our expectations.
 (response does not contain a connection id)
 
 ================================================
-Testing CRCX
+Testing test_messages() CRCX
 creating message from statically defined input:
 ---------8<---------
 CRCX 2 1@mgw MGCP 1.0
@@ -83,7 +83,7 @@ Response matches our expectations.
 Dummy packets: 2
 
 ================================================
-Testing MDCX3
+Testing test_messages() MDCX3
 creating message from statically defined input:
 ---------8<---------
 MDCX 18983215 1@mgw MGCP 1.0
@@ -97,7 +97,7 @@ Response matches our expectations.
 Dummy packets: 2
 
 ================================================
-Testing MDCX4_ADDR000
+Testing test_messages() MDCX4_ADDR000
 creating message from statically defined input:
 ---------8<---------
 MDCX 18983216 1@mgw MGCP 1.0
@@ -120,7 +120,7 @@ Response matches our expectations.
 Response matches our expectations.
 (response does not contain a connection id)
 
-================================================
+================================================
 Testing test_messages() MDCX4
 creating message from statically defined input:
 ---------8<---------
@@ -144,7 +144,7 @@ Response matches our expectations.
 Response matches our expectations.
 (response contains a connection id)
 Dummy packets: 2
-
+
 ================================================
 Testing test_messages() MDCX4_PT1
 creating message from statically defined input:
@@ -168,7 +168,7 @@ Response matches our expectations.
 using message with patched conn_id for comparison
 Response matches our expectations.
 (response contains a connection id)
-Dummy packets: 2
+Dummy packets: 2
 
 ================================================
 Testing test_messages() MDCX4_PT2
@@ -192,7 +192,7 @@ Response matches our expectations.
 checking response:
 using message with patched conn_id for comparison
 Response matches our expectations.
-(response contains a connection id)
+(response contains a connection id)
 Dummy packets: 2
 
 ================================================
@@ -216,7 +216,7 @@ Response matches our expectations.
 ---------8<---------
 checking response:
 using message with patched conn_id for comparison
-Response matches our expectations.
+Response matches our expectations.
 (response contains a connection id)
 Dummy packets: 2
 
@@ -240,7 +240,7 @@ Response matches our expectations.
 
 ---------8<---------
 checking response:
-using message with patched conn_id for comparison
+using message with patched conn_id for comparison
 Response matches our expectations.
 (response contains a connection id)
 Dummy packets: 2
@@ -263,7 +263,7 @@ Response matches our expectations.
 a=rtpmap:99 AMR/8000
 a=ptime:40
 
----------8<---------
+---------8<---------
 checking response:
 using message with patched conn_id for comparison
 Response matches our expectations.
@@ -279,7 +279,7 @@ Response matches our expectations.
 I: %s
 L: p:20, a:AMR, nt:IN
 
----------8<---------
+---------8<---------
 checking response:
 using message with patched conn_id for comparison
 Response matches our expectations.
@@ -293,7 +293,7 @@ Response matches our expectations.
 DLCX 7 1@mgw MGCP 1.0
 I: %s
 C: 2
-
+
 ---------8<---------
 checking response:
 using message as statically defined for comparison
@@ -307,7 +307,7 @@ Response matches our expectations.
 CRCX 2 1@mgw MGCP 1.0
 M: recvonly
 C: 2
-
+
 v=0
 c=IN IP4 123.12.12.123
 m=audio 5904 RTP/AVP 97
@@ -315,7 +315,7 @@ creating message from statically defined input:
 ---------8<---------
 checking response:
 using message with patched conn_id for comparison
-Response matches our expectations.
+Response matches our expectations.
 (response contains a connection id)
 Dummy packets: 2
 
@@ -327,7 +327,7 @@ Response matches our expectations.
 
 ---------8<---------
 
-================================================
+================================================
 Testing test_messages() SHORT1
 creating message from statically defined input:
 ---------8<---------
@@ -338,7 +338,7 @@ Response matches our expectations.
 using message as statically defined for comparison
 Response matches our expectations.
 (response does not contain a connection id)
-
+
 ================================================
 Testing test_messages() SHORT2
 creating message from statically defined input:
@@ -349,7 +349,7 @@ Response matches our expectations.
 using message as statically defined for comparison
 Response matches our expectations.
 (response does not contain a connection id)
-
+
 ================================================
 Testing test_messages() SHORT3
 creating message from statically defined input:
@@ -360,7 +360,7 @@ Response matches our expectations.
 using message as statically defined for comparison
 Response matches our expectations.
 (response does not contain a connection id)
-
+
 ================================================
 Testing test_messages() SHORT4
 creating message from statically defined input:
@@ -374,7 +374,7 @@ Response matches our expectations.
 
 ================================================
 Testing test_messages() RQNT1
-creating message from statically defined input:
+creating message from statically defined input:
 ---------8<---------
 RQNT 186908780 1@mgw MGCP 1.0
 X: B244F267488
@@ -388,7 +388,7 @@ Response matches our expectations.
 
 ================================================
 Testing test_messages() RQNT2
-creating message from statically defined input:
+creating message from statically defined input:
 ---------8<---------
 RQNT 186908781 1@mgw MGCP 1.0
 X: ADD4F26746F
@@ -402,7 +402,7 @@ Response matches our expectations.
 
 ================================================
 Testing test_messages() DLCX
-creating message from statically defined input:
+creating message from statically defined input:
 ---------8<---------
 DLCX 7 1@mgw MGCP 1.0
 I: %s
@@ -424,7 +424,7 @@ Response matches our expectations.
 L: p:20
 
 v=0
-c=IN IP4 123.12.12.123
+c=IN IP4 123.12.12.123
 m=audio 5904 RTP/AVP 97
 a=rtpmap:97 GSM-EFR/8000
 a=ptime:40
@@ -438,7 +438,7 @@ Response matches our expectations.
 
 ================================================
 Testing test_messages() MDCX3
-creating message from statically defined input:
+creating message from statically defined input:
 ---------8<---------
 MDCX 18983215 1@mgw MGCP 1.0
 I: %s
@@ -452,7 +452,7 @@ Response matches our expectations.
 
 ================================================
 Testing test_messages() DLCX
-creating message from statically defined input:
+creating message from statically defined input:
 ---------8<---------
 DLCX 7 1@mgw MGCP 1.0
 I: %s
@@ -466,7 +466,7 @@ Response matches our expectations.
 
 ================================================
 Testing test_messages() CRCX
-creating message from statically defined input:
+creating message from statically defined input:
 ---------8<---------
 CRCX 2 6@mgw MGCP 1.0
 M: recvonly
@@ -489,7 +489,7 @@ Response matches our expectations.
 X-Osmo-IGN: C foo
 
 v=0
-c=IN IP4 123.12.12.123
+c=IN IP4 123.12.12.123
 m=audio 5904 RTP/AVP 97
 a=rtpmap:97 GSM-EFR/8000
 a=ptime:40
@@ -502,7 +502,7 @@ Response matches our expectations.
 Dummy packets: 2
 
 ================================================
-Testing MDCX_TOO_LONG_CI
+Testing test_messages() MDCX_TOO_LONG_CI
 creating message from statically defined input:
 ---------8<---------
 MDCX 18983224 1@mgw MGCP 1.0
@@ -526,7 +526,7 @@ Response matches our expectations.
 
 v=0
 c=IN IP4 123.12.12.123
-m=audio 5904 RTP/AVP 111
+m=audio 5904 RTP/AVP 111
 a=rtpmap:111 AMR/8000/1
 a=ptime:20
 a=fmtp:111 mode-change-capability=2; octet-align=1
@@ -538,7 +538,7 @@ Response matches our expectations.
 (response contains a connection id)
 Dummy packets: 2
 
-================================================
+================================================
 Testing test_messages() AUEP_NULL
 creating message from statically defined input:
 ---------8<---------
@@ -559,7 +559,7 @@ Response matches our expectations.
 C: 2
 L: p:20
 
-v=0
+v=0
 c=IN IP4 123.12.12.123
 m=audio 5904 RTP/AVP 97
 a=rtpmap:97 GSM-EFR/8000
@@ -572,7 +572,7 @@ Response matches our expectations.
 (response does not contain a connection id)
 
 ================================================
-Testing MDCX_NULL
+Testing test_messages() MDCX_NULL
 creating message from statically defined input:
 ---------8<---------
 MDCX 9 null@mgw MGCP 1.0
@@ -586,7 +586,7 @@ Response matches our expectations.
 
 ================================================
 Testing test_messages() DLCX_NULL
-creating message from statically defined input:
+creating message from statically defined input:
 ---------8<---------
 DLCX 8 null@mgw MGCP 1.0
 I: %s
@@ -600,7 +600,122 @@ Response matches our expectations.
 
 ================================================
 Testing test_messages() RQNT_NULL
-creating message from statically defined input:
+creating message from statically defined input:
+---------8<---------
+RQNT 186908782 null@mgw MGCP 1.0
+X: B244F267488
+S: D/9
+
+---------8<---------
+checking response:
+using message as statically defined for comparison
+Response matches our expectations.
+(response does not contain a connection id)
+
+================================================
+Testing test_messages() CRCX_EXPLICIT_EP
+creating message from statically defined input:
+---------8<---------
+CRCX 101 rtpbridge/8@mgw MGCP 1.0
+m: recvonly
+C: 2
+L: p:20
+
+v=0
+c=IN IP4 1.2.3.4
+m=audio 1234 RTP/AVP 112
+a=rtpmap:112 AMR/8000
+a=ptime:20
+
+---------8<---------
+checking response:
+using message with patched conn_id for comparison
+Response matches our expectations.
+(response contains a connection id)
+Dummy packets: 2
+
+================================================
+Testing test_messages() CRCX_TWO_PAYLOADS_1
+creating message from statically defined input:
+---------8<---------
+CRCX 102 rtpbridge/*@mgw MGCP 1.0
+m: recvonly
+C: 2
+L: p:20
+
+v=0
+c=IN IP4 1.2.3.4
+m=audio 1234 RTP/AVP 112 3
+a=rtpmap:112 AMR/8000
+a=ptime:20
+
+---------8<---------
+checking response:
+using message with patched conn_id for comparison
+Response matches our expectations.
+(response contains a connection id)
+Dummy packets: 2
+
+================================================
+Testing test_messages() CRCX_TWO_PAYLOADS_2
+creating message from statically defined input:
+---------8<---------
+CRCX 103 rtpbridge/2@mgw MGCP 1.0
+m: recvonly
+C: 2
+L: p:20
+
+v=0
+c=IN IP4 1.2.3.4
+m=audio 1234 RTP/AVP 3 112
+a=rtpmap:112 AMR/8000
+a=ptime:20
+
+---------8<---------
+checking response:
+using message with patched conn_id for comparison
+Response matches our expectations.
+(response contains a connection id)
+Dummy packets: 2
+
+================================================
+Testing test_messages() CRCX_THREE_PAYLOADS_1
+creating message from statically defined input:
+---------8<---------
+CRCX 104 rtpbridge/*@mgw MGCP 1.0
+m: recvonly
+C: 4
+L: p:20
+
+v=0
+c=IN IP4 1.2.3.4
+m=audio 1234 RTP/AVP 112 3 111
+a=rtpmap:112 AMR/8000
+a=fmtp:112 octet-align=1;mode-set=0,2,4,7
+a=rtpmap:111 GSM-HR-08/8000
+a=ptime:20
+
+---------8<---------
+checking response:
+using message with patched conn_id for comparison
+Response matches our expectations.
+(response contains a connection id)
+Dummy packets: 2
+
+================================================
+Testing test_messages() CRCX_THREE_PAYLOADS_2
+creating message from statically defined input:
+---------8<---------
+CRCX 105 rtpbridge/3@mgw MGCP 1.0
+m: recvonly
+C: 4
+L: p:20
+
+v=0
+c=IN IP4 1.2.3.4
+m=audio 1234 RTP/AVP 3 112 113
+a=rtpmap:112 AMR/8000
+a=fmtp:112 octet-align=1;mode-set=0,2,4,7
 a=rtpmap:113 AMR/8000
 a=fmtp:113 octet-align=1;mode-set=0,2,4
 a=ptime:20
@@ -638,7 +753,7 @@ using message with patched conn_id for comparison
 m: recvonly
 C: 2
 L: p:20
-
+
 v=0
 c=IN IP4 123.12.12.123
 m=audio 5904 RTP/AVP 97
@@ -662,7 +777,7 @@ using message as statically defined for comparison
 checking response:
 using message as statically defined for comparison
 Response matches our expectations.
-Re-transmitting RQNT1
+Re-transmitting RQNT1
 creating message from statically defined input:
 ---------8<---------
 RQNT 186908780 1@mgw MGCP 1.0
@@ -686,7 +801,7 @@ using message as statically defined for comparison
 checking response:
 using message as statically defined for comparison
 Response matches our expectations.
-Re-transmitting RQNT2
+Re-transmitting RQNT2
 creating message from statically defined input:
 ---------8<---------
 RQNT 186908781 1@mgw MGCP 1.0
@@ -708,7 +823,7 @@ using message with patched conn_id for comparison
 ---------8<---------
 checking response:
 using message with patched conn_id for comparison
-Response matches our expectations.
+Response matches our expectations.
 Re-transmitting MDCX3
 creating message from statically defined input:
 ---------8<---------

tests/mgcp_client/mgcp_client_test.c

@@ -107,9 +107,6 @@ void test_response_cb(struct mgcp_response *response, void *priv)
 	printf("  audio_port = %u\n", response->audio_port);
 	printf("  audio_ip = %s\n", response->audio_ip);
 	printf("  ptime = %u\n", response->ptime);
-	printf("  codecs_len = %u\n", response->codecs_len);
-	for(i=0;i<response->codecs_len;i++)
-		printf("  codecs[%u] = %u\n", i, response->codecs[i]);
 	printf("  ptmap_len = %u\n", response->ptmap_len);
 	for(i=0;i<response->ptmap_len;i++) {
 		printf("  ptmap[%u].codec = %u\n", i, response->ptmap[i].codec);
@@ -149,12 +146,11 @@ void test_mgcp_msg(void)
 		.conn_id = "11",
 		.conn_mode = MGCP_CONN_RECV_SEND,
 		.ptime = 20,
-		.codecs[0] = CODEC_GSM_8000_1,
-		.codecs[1] = CODEC_AMR_8000_1,
-		.codecs[2] = CODEC_GSMEFR_8000_1,
-		.codecs_len = 1,
-		.ptmap[0].codec = CODEC_GSMEFR_8000_1,
-		.ptmap[0].pt = 96,
+		.ptmap = {
+			{ .codec = CODEC_GSM_8000_1, .pt = CODEC_GSM_8000_1 },
+			{ .codec = CODEC_AMR_8000_1, .pt = CODEC_AMR_8000_1 },
+			{ .codec = CODEC_GSMEFR_8000_1, .pt = 96 },
+		},
 		.ptmap_len = 1,
 		.x_osmo_ign = MGCP_X_OSMO_IGN_CALLID,
 		.x_osmo_osmux_cid = -1, /* wildcard */
@@ -179,9 +175,9 @@ void test_mgcp_msg(void)
 	mgcp_msg.presence =
 	    (MGCP_MSG_PRESENCE_ENDPOINT | MGCP_MSG_PRESENCE_CALL_ID |
 	     MGCP_MSG_PRESENCE_CONN_ID | MGCP_MSG_PRESENCE_CONN_MODE);
-	mgcp_msg.codecs_len = 2;
+	mgcp_msg.ptmap_len = 2;
 	msg = mgcp_msg_gen(mgcp, &mgcp_msg);
-	mgcp_msg.codecs_len = 1;
+	mgcp_msg.ptmap_len = 1;
 	printf("%s\n", (char *)msg->data);
 
 	printf("Generated CRCX message (three codecs, one with custom pt):\n");
@@ -189,9 +185,9 @@ void test_mgcp_msg(void)
 	mgcp_msg.presence =
 	    (MGCP_MSG_PRESENCE_ENDPOINT | MGCP_MSG_PRESENCE_CALL_ID |
 	     MGCP_MSG_PRESENCE_CONN_ID | MGCP_MSG_PRESENCE_CONN_MODE);
-	mgcp_msg.codecs_len = 3;
+	mgcp_msg.ptmap_len = 3;
 	msg = mgcp_msg_gen(mgcp, &mgcp_msg);
-	mgcp_msg.codecs_len = 1;
+	mgcp_msg.ptmap_len = 1;
 	printf("%s\n", (char *)msg->data);
 
 	printf("Generated MDCX message:\n");
@@ -209,9 +205,9 @@ void test_mgcp_msg(void)
 	    (MGCP_MSG_PRESENCE_ENDPOINT | MGCP_MSG_PRESENCE_CALL_ID |
 	     MGCP_MSG_PRESENCE_CONN_ID | MGCP_MSG_PRESENCE_CONN_MODE |
 	     MGCP_MSG_PRESENCE_AUDIO_IP | MGCP_MSG_PRESENCE_AUDIO_PORT);
-	mgcp_msg.codecs_len = 2;
+	mgcp_msg.ptmap_len = 2;
 	msg = mgcp_msg_gen(mgcp, &mgcp_msg);
-	mgcp_msg.codecs_len = 1;
+	mgcp_msg.ptmap_len = 1;
 	printf("%s\n", (char *)msg->data);
 
 	printf("Generated MDCX message (three codecs, one with custom pt):\n");
@@ -220,9 +216,9 @@ void test_mgcp_msg(void)
 	    (MGCP_MSG_PRESENCE_ENDPOINT | MGCP_MSG_PRESENCE_CALL_ID |
 	     MGCP_MSG_PRESENCE_CONN_ID | MGCP_MSG_PRESENCE_CONN_MODE |
 	     MGCP_MSG_PRESENCE_AUDIO_IP | MGCP_MSG_PRESENCE_AUDIO_PORT);
-	mgcp_msg.codecs_len = 3;
+	mgcp_msg.ptmap_len = 3;
 	msg = mgcp_msg_gen(mgcp, &mgcp_msg);
-	mgcp_msg.codecs_len = 1;
+	mgcp_msg.ptmap_len = 1;
 	printf("%s\n", (char *)msg->data);
 
 	printf("Generated DLCX message:\n");
@@ -330,8 +326,10 @@ void test_mgcp_client_cancel(void)
 		.presence = (MGCP_MSG_PRESENCE_ENDPOINT | MGCP_MSG_PRESENCE_CALL_ID
 			     | MGCP_MSG_PRESENCE_CONN_ID | MGCP_MSG_PRESENCE_CONN_MODE),
 		.ptime = 20,
-		.codecs[0] = CODEC_AMR_8000_1,
-		.codecs_len = 1
+		.ptmap = {
+			{ .codec = CODEC_AMR_8000_1, .pt = CODEC_AMR_8000_1 },
+		},
+		.ptmap_len = 1
 	};
 
 	printf("\n%s():\n", __func__);
@@ -573,57 +571,6 @@ static void test_map_str_to_codec(void)
 	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;
@@ -675,6 +622,214 @@ void test_mgcp_client_e1_epname(void)
 	OSMO_ASSERT(epname == NULL);
 }
 
+struct parse_response_test {
+	const char *body;
+	int expect_rc;
+	struct mgcp_response expect_params;
+};
+
+static struct parse_response_test parse_response_tests[] = {
+	{
+		.body = "200 2 OK\r\n"
+			"I: foo\r\n"
+			"\r\n"
+			"v=0\r\n"
+			"o=- name 23 IN IP4 0.0.0.0\r\n"
+			"s=-\r\n"
+			"c=IN IP4 1.2.3.4\r\n"
+			"t=0 0\r\n"
+			"m=audio 23 RTP/AVP 112 3\r\n" /* <-- implicit: 3 = GSM-FR */
+			"a=rtpmap:112 AMR/8000\r\n"
+			"a=ptime:20\r\n",
+		.expect_rc = 0,
+		.expect_params = {
+			.audio_port = 23,
+			.audio_ip = "1.2.3.4",
+			.ptmap = {
+				{ .codec = CODEC_AMR_8000_1, .pt = 112 },
+				{ .codec = CODEC_GSM_8000_1, .pt = 3 },
+			},
+			.ptmap_len = 2,
+		},
+	},
+	{
+		.body = "200 2 OK\r\n"
+			"I: foo\r\n"
+			"\r\n"
+			"v=0\r\n"
+			"o=- name 23 IN IP4 0.0.0.0\r\n"
+			"s=-\r\n"
+			"c=IN IP4 1.2.3.4\r\n"
+			"t=0 0\r\n"
+			"m=audio 23 RTP/AVP 112 3\r\n"
+			"a=rtpmap:112 AMR/8000\r\n"
+			"a=rtpmap:3 GSM/8000\r\n" /* 3 == GSM-FR implicitly, is an explicit entry a problem? */
+			"a=ptime:20\r\n",
+		.expect_rc = 0,
+		.expect_params = {
+			.audio_port = 23,
+			.audio_ip = "1.2.3.4",
+			.ptmap = {
+				{ .codec = CODEC_AMR_8000_1, .pt = 112 },
+				{ .codec = CODEC_GSM_8000_1, .pt = 3 }, /* no, not a problem */
+			},
+			.ptmap_len = 2,
+		},
+	},
+	{
+		.body = "200 2 OK\r\n"
+			"I: foo\r\n"
+			"\r\n"
+			"v=0\r\n"
+			"o=- name 23 IN IP4 0.0.0.0\r\n"
+			"s=-\r\n"
+			"c=IN IP4 1.2.3.4\r\n"
+			"t=0 0\r\n"
+			"m=audio 23 RTP/AVP 3\r\n" /* <-- 112 is missing here. Will it still appear? */
+			"a=rtpmap:112 AMR/8000\r\n"
+			"a=ptime:20\r\n",
+		.expect_rc = 0,
+		.expect_params = {
+			.audio_port = 23,
+			.audio_ip = "1.2.3.4",
+			.ptmap = {
+				{ .codec = CODEC_GSM_8000_1, .pt = 3 },
+				{ .codec = CODEC_AMR_8000_1, .pt = 112 }, /* <-- yes, it was added to the end. */
+			},
+			.ptmap_len = 2,
+		},
+	},
+	{
+		/* test MGCP_MAX_CODECS */
+		.body = "200 2 OK\r\n"
+			"I: foo\r\n"
+			"\r\n"
+			"v=0\r\n"
+			"o=- name 23 IN IP4 0.0.0.0\r\n"
+			"s=-\r\n"
+			"c=IN IP4 1.2.3.4\r\n"
+			"t=0 0\r\n"
+			"m=audio 23 RTP/AVP 101 102 103 104 105 106 107 108 109 110\r\n" /* <-- 10 codecs max */
+			"a=rtpmap:101 AMR/8000\r\n"
+			"a=rtpmap:102 AMR/8000\r\n"
+			"a=rtpmap:103 AMR/8000\r\n"
+			"a=rtpmap:104 AMR/8000\r\n"
+			"a=rtpmap:105 AMR/8000\r\n"
+			"a=rtpmap:106 AMR/8000\r\n"
+			"a=rtpmap:107 AMR/8000\r\n"
+			"a=rtpmap:108 AMR/8000\r\n"
+			"a=rtpmap:109 AMR/8000\r\n"
+			"a=rtpmap:110 AMR/8000\r\n"
+			"a=ptime:20\r\n",
+		.expect_rc = 0,
+		.expect_params = {
+			.audio_port = 23,
+			.audio_ip = "1.2.3.4",
+			.ptmap = {
+				{ .codec = CODEC_AMR_8000_1, .pt = 101 },
+				{ .codec = CODEC_AMR_8000_1, .pt = 102 },
+				{ .codec = CODEC_AMR_8000_1, .pt = 103 },
+				{ .codec = CODEC_AMR_8000_1, .pt = 104 },
+				{ .codec = CODEC_AMR_8000_1, .pt = 105 },
+				{ .codec = CODEC_AMR_8000_1, .pt = 106 },
+				{ .codec = CODEC_AMR_8000_1, .pt = 107 },
+				{ .codec = CODEC_AMR_8000_1, .pt = 108 },
+				{ .codec = CODEC_AMR_8000_1, .pt = 109 },
+				{ .codec = CODEC_AMR_8000_1, .pt = 110 },
+			},
+			.ptmap_len = 10,
+		},
+	},
+	{
+		/* test MGCP_MAX_CODECS */
+		.body = "200 2 OK\r\n"
+			"I: foo\r\n"
+			"\r\n"
+			"v=0\r\n"
+			"o=- name 23 IN IP4 0.0.0.0\r\n"
+			"s=-\r\n"
+			"c=IN IP4 1.2.3.4\r\n"
+			"t=0 0\r\n"
+			"m=audio 23 RTP/AVP 101 102 103 104 105 106 107 108 109 110 3\r\n" /* <-- 11 > MGCP_MAX_CODECS */
+			"a=rtpmap:101 AMR/8000\r\n"
+			"a=rtpmap:102 AMR/8000\r\n"
+			"a=rtpmap:103 AMR/8000\r\n"
+			"a=rtpmap:104 AMR/8000\r\n"
+			"a=rtpmap:105 AMR/8000\r\n"
+			"a=rtpmap:106 AMR/8000\r\n"
+			"a=rtpmap:107 AMR/8000\r\n"
+			"a=rtpmap:108 AMR/8000\r\n"
+			"a=rtpmap:109 AMR/8000\r\n"
+			"a=rtpmap:110 AMR/8000\r\n"
+			"a=ptime:20\r\n",
+		.expect_rc = -EINVAL,
+	},
+};
+
+static void test_parse_response(void)
+{
+	int i;
+	int failures = 0;
+
+	for (i = 0; i < ARRAY_SIZE(parse_response_tests); i++) {
+		int rc;
+		struct parse_response_test *t = &parse_response_tests[i];
+		struct mgcp_response *r = talloc_zero(ctx, struct mgcp_response);
+		int p;
+
+		r->body = talloc_strdup(r, t->body);
+
+		//printf("\n%s() test [%d]:\n", __func__, i);
+		fprintf(stderr, "\n%s() test [%d]:\n", __func__, i);
+		fprintf(stderr, "body: \"%s\"\n", osmo_escape_str(r->body, -1));
+
+		rc = mgcp_response_parse_params(r);
+
+		fprintf(stderr, "got rc=%d\n", rc);
+		if (rc != t->expect_rc) {
+			fprintf(stderr, "FAIL: Expected rc=%d\n", t->expect_rc);
+			failures++;
+		}
+		if (rc) {
+			talloc_free(r);
+			continue;
+		}
+
+		fprintf(stderr, "got audio_ip=\"%s\"\n", r->audio_ip);
+		if (strcmp(r->audio_ip, t->expect_params.audio_ip)) {
+			fprintf(stderr, "FAIL: Expected audio_ip=\"%s\"\n", t->expect_params.audio_ip);
+			failures++;
+		}
+		fprintf(stderr, "got audio_port=%u\n", r->audio_port);
+		if (r->audio_port != t->expect_params.audio_port) {
+			fprintf(stderr, "FAIL: Expected audio_port=%u\n", t->expect_params.audio_port);
+			failures++;
+		}
+
+		for (p = 0; p < r->ptmap_len; p++) {
+			struct ptmap *got = &r->ptmap[p];
+			struct ptmap *expect = NULL;
+			fprintf(stderr, "  %d %s\n", got->pt, osmo_mgcpc_codec_name(got->codec));
+			if (p >= t->expect_params.ptmap_len) {
+				fprintf(stderr, "    - ERROR: too many codec entries\n");
+				failures++;
+				continue;
+			}
+			expect = &t->expect_params.ptmap[p];
+			if (ptmap_cmp(got, expect)) {
+				fprintf(stderr, "    - ERROR: expected: %d %s\n",
+					expect->pt, osmo_mgcpc_codec_name(expect->codec));
+				failures++;
+			}
+		}
+
+		talloc_free(r);
+	}
+
+	OSMO_ASSERT(!failures);
+}
+
+
 static const struct log_info_cat log_categories[] = {
 };
 
@@ -702,10 +857,11 @@ int main(int argc, char **argv)
 	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();
 
+	test_parse_response();
+
 	printf("Done\n");
 	fprintf(stderr, "Done\n");
 	return EXIT_SUCCESS;

tests/mgcp_client/mgcp_client_test.err

@@ -128,13 +128,56 @@ test_sdp_section_start() test [18]:
 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!
 DLMGCP MGW(mgw) Cannot compose MGCP e1-endpoint name (ds/e1-15/s-0/su8-2@mgw), E1-timeslot number (0) is invalid!
 DLMGCP MGW(mgw) Cannot compose MGCP e1-endpoint name (ds/e1-15/s-64/su8-2@mgw), E1-timeslot number (64) is invalid!
+
+test_parse_response() test [0]:
+body: "200 2 OK\r\nI: foo\r\n\r\nv=0\r\no=- name 23 IN IP4 0.0.0.0\r\ns=-\r\nc=IN IP4 1.2.3.4\r\nt=0 0\r\nm=audio 23 RTP/AVP 112 3\r\na=rtpmap:112 AMR/8000\r\na=ptime:20\r\n"
+got rc=0
+got audio_ip="1.2.3.4"
+got audio_port=23
+  112 AMR/8000/1
+  3 GSM/8000/1
+
+test_parse_response() test [1]:
+body: "200 2 OK\r\nI: foo\r\n\r\nv=0\r\no=- name 23 IN IP4 0.0.0.0\r\ns=-\r\nc=IN IP4 1.2.3.4\r\nt=0 0\r\nm=audio 23 RTP/AVP 112 3\r\na=rtpmap:112 AMR/8000\r\na=rtpmap:3 GSM/8000\r\na=ptime:20\r\n"
+got rc=0
+got audio_ip="1.2.3.4"
+got audio_port=23
+  112 AMR/8000/1
+  3 GSM/8000/1
+
+test_parse_response() test [2]:
+body: "200 2 OK\r\nI: foo\r\n\r\nv=0\r\no=- name 23 IN IP4 0.0.0.0\r\ns=-\r\nc=IN IP4 1.2.3.4\r\nt=0 0\r\nm=audio 23 RTP/AVP 3\r\na=rtpmap:112 AMR/8000\r\na=ptime:20\r\n"
+DLMGCP error in MGCP message: 'a=rtpmap:112' has no matching entry in 'm=audio ... 112'
+got rc=0
+got audio_ip="1.2.3.4"
+got audio_port=23
+  3 GSM/8000/1
+  112 AMR/8000/1
+
+test_parse_response() test [3]:
+body: "200 2 OK\r\nI: foo\r\n\r\nv=0\r\no=- name 23 IN IP4 0.0.0.0\r\ns=-\r\nc=IN IP4 1.2.3.4\r\nt=0 0\r\nm=audio 23 RTP/AVP 101 102 103 104 105 106 107 108 109 110\r\na=rtpmap:101 AMR/8000\r\na=rtpmap:102 AMR/8000\r\na=rtpmap:103 AMR/8000\r\na=rtpmap:104 AMR/8"
+got rc=0
+got audio_ip="1.2.3.4"
+got audio_port=23
+  101 AMR/8000/1
+  102 AMR/8000/1
+  103 AMR/8000/1
+  104 AMR/8000/1
+  105 AMR/8000/1
+  106 AMR/8000/1
+  107 AMR/8000/1
+  108 AMR/8000/1
+  109 AMR/8000/1
+  110 AMR/8000/1
+
+test_parse_response() test [4]:
+body: "200 2 OK\r\nI: foo\r\n\r\nv=0\r\no=- name 23 IN IP4 0.0.0.0\r\ns=-\r\nc=IN IP4 1.2.3.4\r\nt=0 0\r\nm=audio 23 RTP/AVP 101 102 103 104 105 106 107 108 109 110 3\r\na=rtpmap:101 AMR/8000\r\na=rtpmap:102 AMR/8000\r\na=rtpmap:103 AMR/8000\r\na=rtpmap:104 AMR"
+DLMGCP SDP: can parse only up to 10 payload type numbers
+DLMGCP Failed to parse SDP parameter payload types (RTP/AVP)
+got rc=-22
 Done

tests/mgcp_client/mgcp_client_test.ok

@@ -181,35 +181,6 @@ test_sdp_section_start() test [16]:
 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