Bump version: 1.6.0.30-832bc-dirty → 1.7.0

libosmocore required version increased due to include used from
libosmo-netif including an include from libosmocore which in previous
versions misses including an include from a symbol used.

Change-Id: I1d5f14b1ad36b2ed94343fca71fdc622424403d3

configure.ac

@@ -39,9 +39,9 @@ AC_SEARCH_LIBS([dlopen], [dl dld], [LIBRARY_DL="$LIBS";LIBS=""])
 AC_SUBST(LIBRARY_DL)
 
 
-PKG_CHECK_MODULES(LIBOSMOCORE, libosmocore >= 1.0.0)
-PKG_CHECK_MODULES(LIBOSMOGSM, libosmogsm >= 1.0.0)
-PKG_CHECK_MODULES(LIBOSMOVTY, libosmovty >= 1.0.0)
+PKG_CHECK_MODULES(LIBOSMOCORE, libosmocore >= 1.1.0)
+PKG_CHECK_MODULES(LIBOSMOGSM, libosmogsm >= 1.1.0)
+PKG_CHECK_MODULES(LIBOSMOVTY, libosmovty >= 1.1.0)
 PKG_CHECK_MODULES(LIBOSMONETIF, libosmo-netif >= 0.6.0)
 
 AC_ARG_ENABLE(sanitize,

debian/changelog

@@ -1,3 +1,45 @@
+osmo-mgw (1.7.0) unstable; urgency=medium
+
+  [ Neels Hofmeyr ]
+  * rename codecs_cmp() to codecs_same()
+  * mgcp_codec: constify 'param' arg
+  * fix crashes: don't assert on incoming RTP packet size
+  * mgcp_send(): stop looping on conversion error
+  * mgcp_codec: split codec_free() off of codec_init()
+  * fix memleak: actually free strings in mgcp_codec_reset_all()
+  * mgcp_test: extend / rewrite test_mgcp_codec_pt_translate()
+  * test_mgcp_codec_pt_translate(): more tests
+  * differentiate AMR octet-aligned=0 vs =1
+  * ptmap: implicitly match  '/8000' and '/8000/1'
+  * mgcp_codec: codec_set(): log about all possible errors
+  * mgcp_codec_add: fix audio_name size check
+  * explicitly free codecs in mgcp_rtp_conn_cleanup()
+  * tweak mgcp_parse_audio_ptime_rtpmap()
+  * SDP: store all ptmap entries
+  * mgcp_client_fsm cleanup: Do not assert on DLCX failure
+  * clear pending requests on MGCP failure
+  * client: endp fsm: add notify struct, prep for cancel-notify
+  * client: endp fsm: clear ci[] before dispatching DLCX success
+  * client: endp fsm: allow cancelling a notify event
+  * client: endp fsm: add osmo_mgcpc_ep_ci_ep()
+  * accept MGCP without SDP
+  * fix use-after-free: require new fsm deferred dealloc, check for term
+
+  [ Pau Espin Pedrol ]
+  * mgcp_test: Correctly release all endpoints allocated
+  * mgw: Allocate mgcp_conn instance under tcfg->endpoints
+
+  [ Harald Welte ]
+  * manual: Fix copy+paste error
+  * mgcp_client: Check for osmo_fsm_register() error return value
+  * Move fsm_mgcp_client regstration to __attribute__((contructor))
+  * exit(2) on unsupported positional arguments on command line
+
+  [ Oliver Smith ]
+  * osmoappdesc.py: switch to python 3
+
+ -- Pau Espin Pedrol <pespin@sysmocom.de>  Fri, 03 Jan 2020 13:35:09 +0100
+
 osmo-mgw (1.6.0) unstable; urgency=medium
 
   [ Oliver Smith ]

doc/manuals/chapters/running.adoc

@@ -12,7 +12,7 @@ arguments:
 *-h, --help*::
 	Print a short help message about the supported options
 *-V, --version*::
-	Print the compile-time version number of the OsmoBTS program
+	Print the compile-time version number of the program
 *-D, --daemonize*::
 	Fork the process as a daemon into background.
 *-c, --config-file 'CONFIGFILE'*::

include/osmocom/mgcp/mgcp_codec.h

@@ -2,6 +2,6 @@
 
 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, struct mgcp_codec_param *param);
+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_decide(struct mgcp_conn_rtp *conn);
 int mgcp_codec_pt_translate(struct mgcp_conn_rtp *conn_src, struct mgcp_conn_rtp *conn_dst, int payload_type);

include/osmocom/mgcp_client/mgcp_client_endpoint_fsm.h

@@ -29,6 +29,9 @@ void osmo_mgcpc_ep_ci_request(struct osmo_mgcpc_ep_ci *ci,
 			      uint32_t event_success, uint32_t event_failure,
 			      void *notify_data);
 
+void osmo_mgcpc_ep_cancel_notify(struct osmo_mgcpc_ep *ep, struct osmo_fsm_inst *notify);
+struct osmo_mgcpc_ep *osmo_mgcpc_ep_ci_ep(struct osmo_mgcpc_ep_ci *ci);
+
 /*! Dispatch a DLCX for the given connection.
  * \param ci  Connection identifier as obtained from osmo_mgcpc_ep_ci_add().
  */

osmoappdesc.py

@@ -1,4 +1,4 @@
-#!/usr/bin/env python
+#!/usr/bin/env python3
 
 # (C) 2013 by Katerina Barone-Adesi <kat.obsc@gmail.com>
 # This program is free software: you can redistribute it and/or modify

src/libosmo-mgcp-client/Makefile.am

@@ -20,7 +20,7 @@ AM_LDFLAGS = \
 
 # This is not at all related to the release version, but a range of supported
 # API versions. Read TODO_RELEASE in the source tree's root!
-MGCP_CLIENT_LIBVERSION=6:0:0
+MGCP_CLIENT_LIBVERSION=7:0:1
 
 lib_LTLIBRARIES = \
 	libosmo-mgcp-client.la \

src/libosmo-mgcp-client/mgcp_client.c

@@ -337,48 +337,34 @@ static int mgcp_parse_audio_ptime_rtpmap(struct mgcp_response *r, const char *li
 {
 	unsigned int pt;
 	char codec_resp[64];
-	unsigned int codec;
+	enum mgcp_codecs codec;
 
+#define A_PTIME "a=ptime:"
+#define A_RTPMAP "a=rtpmap:"
 
-	if (strstr(line, "ptime")) {
-		if (sscanf(line, "a=ptime:%u", &r->ptime) != 1)
-			goto response_parse_failure_ptime;
-	} else if (strstr(line, "rtpmap")) {
-		if (sscanf(line, "a=rtpmap:%d %63s", &pt, codec_resp) == 2) {
-			/* The MGW may assign an own payload type in the
-			 * response if the choosen codec falls into the IANA
-			 * assigned dynamic payload type range (96-127).
-			 * Normally the MGW should obey the 3gpp payload type
-			 * assignments, which are fixed, so we likely wont see
-			 * anything unexpected here. In order to be sure that
-			 * we will now check the codec string and if the result
-			 * does not match to what is IANA / 3gpp assigned, we
-			 * will create an entry in the ptmap table so we can
-			 * lookup later what has been assigned. */
-			codec = map_str_to_codec(codec_resp);
-			if (codec != pt) {
-				if (r->ptmap_len < ARRAY_SIZE(r->ptmap)) {
-					r->ptmap[r->ptmap_len].pt = pt;
-					r->ptmap[r->ptmap_len].codec = codec;
-					r->ptmap_len++;
-				} else
-					goto response_parse_failure_rtpmap;
-			}
-
-		} else
-			goto response_parse_failure_rtpmap;
+	if (osmo_str_startswith(line, A_PTIME)) {
+		if (sscanf(line, A_PTIME "%u", &r->ptime) != 1) {
+			LOGP(DLMGCP, LOGL_ERROR,
+			     "Failed to parse SDP parameter, invalid ptime (%s)\n", line);
+			return -EINVAL;
+		}
+	} else if (osmo_str_startswith(line, A_RTPMAP)) {
+		if (sscanf(line, A_RTPMAP "%d %63s", &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;
+		}
+		codec = map_str_to_codec(codec_resp);
+		r->ptmap[r->ptmap_len].pt = pt;
+		r->ptmap[r->ptmap_len].codec = codec;
+		r->ptmap_len++;
 	}
 
 	return 0;
-
-response_parse_failure_ptime:
-	LOGP(DLMGCP, LOGL_ERROR,
-	     "Failed to parse SDP parameter, invalid ptime (%s)\n", line);
-	return -EINVAL;
-response_parse_failure_rtpmap:
-	LOGP(DLMGCP, LOGL_ERROR,
-	     "Failed to parse SDP parameter, invalid rtpmap (%s)\n", line);
-	return -EINVAL;
 }
 
 /* Parse a line like "c=IN IP4 10.11.12.13" */
@@ -480,9 +466,8 @@ int mgcp_response_parse_params(struct mgcp_response *r)
 	/* Find beginning of the parameter (SDP) section */
 	data_ptr = mgcp_find_section_end(data);
 	if (!data_ptr) {
-		LOGP(DLMGCP, LOGL_ERROR,
-		     "MGCP response: cannot find start of SDP parameters\n");
-		rc = -EINVAL;
+		LOGP(DLMGCP, LOGL_DEBUG, "MGCP response contains no SDP parameters\n");
+		rc = 0;
 		goto exit;
 	}
 

src/libosmo-mgcp-client/mgcp_client_endpoint_fsm.c

@@ -73,6 +73,14 @@ enum osmo_mgcpc_ep_fsm_event {
 
 static struct osmo_fsm osmo_mgcpc_ep_fsm;
 
+struct fsm_notify {
+	struct llist_head entry;
+	struct osmo_fsm_inst *fi;
+	uint32_t success;
+	uint32_t failure;
+	void *data;
+};
+
 /*! One connection on an endpoint, corresponding to a connection identifier (CI) as returned by the MGW.
  * An endpoint has a fixed number of slots of these, which may or may not be in use.
  */
@@ -87,10 +95,7 @@ struct osmo_mgcpc_ep_ci {
 	bool sent;
 	enum mgcp_verb verb;
 	struct mgcp_conn_peer verb_info;
-	struct osmo_fsm_inst *notify;
-	uint32_t notify_success;
-	uint32_t notify_failure;
-	void *notify_data;
+	struct fsm_notify notify;
 
 	bool got_port_info;
 	struct mgcp_conn_peer rtp_info;
@@ -118,6 +123,10 @@ struct osmo_mgcpc_ep {
 	/*! Endpoint connection slots. Note that each connection has its own set of FSM event numbers to signal success
 	 * and failure, depending on its index within this array. See CI_EV_SUCCESS and CI_EV_FAILURE. */
 	struct osmo_mgcpc_ep_ci ci[USABLE_CI];
+
+	/*! Internal use: if a function keeps an fsm_notify for later dispatch while already clearing or re-using the
+	 * ci[], the fsm_notify should be kept here to also get canceled by osmo_mgcpc_ep_cancel_notify(). */
+	struct llist_head background_notify;
 };
 
 const struct value_string osmo_mgcp_verb_names[] = {
@@ -246,6 +255,9 @@ struct osmo_mgcpc_ep *osmo_mgcpc_ep_fi_mgwep(struct osmo_fsm_inst *fi)
  * MGCP messages to set up the endpoint will be sent on the given mgcp_client, as soon as the first
  * osmo_mgcpc_ep_ci_request() is invoked.
  *
+ * IMPORTANT: To avoid use-after-free problems, using this FSM requires use of deferred FSM deallocation using
+ * osmo_fsm_set_dealloc_ctx(), e.g. using osmo_select_main_ctx(OTC_SELECT) with osmo_select_main_ctx() as main loop.
+ *
  * A typical sequence of events would be:
  *
  *    ep = osmo_mgcpc_ep_alloc(..., mgcp_client_rtpbridge_wildcard(client));
@@ -296,6 +308,7 @@ struct osmo_mgcpc_ep *osmo_mgcpc_ep_alloc(struct osmo_fsm_inst *parent, uint32_t
 		.fi = fi,
 		.T_defs = T_defs,
 	};
+	INIT_LLIST_HEAD(&ep->background_notify);
 	fi->priv = ep;
 
 	va_start(ap, endpoint_str_fmt);
@@ -354,24 +367,53 @@ static bool osmo_mgcpc_ep_fsm_check_state_chg_after_response(struct osmo_fsm_ins
 
 static void on_failure(struct osmo_mgcpc_ep_ci *ci)
 {
-	struct osmo_fsm_inst *notify = ci->notify;
-	uint32_t notify_failure = ci->notify_failure;
-	void *notify_data = ci->notify_data;
+	struct osmo_mgcpc_ep *ep = ci->ep;
+	struct fsm_notify notify;
+	int i;
 
 	if (!ci->occupied)
 		return;
 
+	/* When dispatching an event for this CI, the user may decide to trigger the next request for this conn right
+	 * away. So we must be ready with a cleared *ci. Store the notify separately and clear before dispatching. */
+	notify = ci->notify;
+	/* Register the planned notification in ep->background_notify so we also catch any osmo_mgcpc_ep_cancel_notify()
+	 * that might be triggered between clearing the ci and actually dispatching the event. */
+	llist_add(&notify.entry, &ep->background_notify);
+
 	*ci = (struct osmo_mgcpc_ep_ci){
 		.ep = ci->ep,
 	};
 
+	/* An MGCP failure typically means the endpoint becomes unusable, cancel all pending request (except DLCX).
+	 * Particularly, if two CRCX were scheduled and the first fails, we must no longer dispatch the second CRCX. */
+	for (i = 0; i < ARRAY_SIZE(ep->ci); i++) {
+		struct osmo_mgcpc_ep_ci *other_ci = &ep->ci[i];
+		if (other_ci == ci)
+			continue;
+		if (!other_ci->occupied)
+			continue;
+		if (!other_ci->pending)
+			continue;
+		if (other_ci->sent)
+			continue;
+		if (other_ci->verb == MGCP_VERB_DLCX)
+			continue;
+		/* Just clear the pending request, don't fire more events than below. */
+		other_ci->pending = false;
+	}
+
 	/* If this check has terminated the FSM instance, don't fire any more events to prevent use-after-free problems.
 	 * The endpoint FSM does dispatch a term event to its parent, and everything should be cleaned like that. */
-	if (!osmo_mgcpc_ep_fsm_check_state_chg_after_response(ci->ep->fi))
+	if (!osmo_mgcpc_ep_fsm_check_state_chg_after_response(ep->fi)) {
+		/* The ep has deallocated, no need to llist_del(&notify.entry) here. */
 		return;
+	}
 
-	if (notify)
-		osmo_fsm_inst_dispatch(notify, notify_failure, notify_data);
+	if (notify.fi)
+		osmo_fsm_inst_dispatch(notify.fi, notify.failure, notify.data);
+
+	llist_del(&notify.entry);
 }
 
 static int update_endpoint_name(struct osmo_mgcpc_ep_ci *ci, const char *new_endpoint_name)
@@ -452,10 +494,10 @@ static void on_success(struct osmo_mgcpc_ep_ci *ci, void *data)
 	LOG_CI(ci, LOGL_DEBUG, "received successful response to %s: RTP=%s%s\n",
 	       osmo_mgcp_verb_name(ci->verb),
 	       mgcp_conn_peer_name(ci->got_port_info? &ci->rtp_info : NULL),
-	       ci->notify ? "" : " (not sending a notification)");
+	       ci->notify.fi ? "" : " (not sending a notification)");
 
-	if (ci->notify)
-		osmo_fsm_inst_dispatch(ci->notify, ci->notify_success, ci->notify_data);
+	if (ci->notify.fi)
+		osmo_fsm_inst_dispatch(ci->notify.fi, ci->notify.success, ci->notify.data);
 
 	osmo_mgcpc_ep_fsm_check_state_chg_after_response(ci->ep->fi);
 }
@@ -516,6 +558,11 @@ static const struct osmo_tdef_state_timeout osmo_mgcpc_ep_fsm_timeouts[32] = {
 				     ((struct osmo_mgcpc_ep*)fi->priv)->T_defs, 5)
 
 /*! Dispatch an actual CRCX/MDCX/DLCX message for this connection.
+ *
+ * If the 'notify' instance deallocates before it received a notification of event_success or event_failure,
+ * osmo_mgcpc_ep_ci_cancel_notify() or osmo_mgcpc_ep_cancel_notify() must be called. It is not harmful to cancel
+ * notification after an event has been received.
+ *
  * \param ci  Connection identifier as obtained from osmo_mgcpc_ep_ci_add().
  * \param verb  MGCP operation to dispatch.
  * \param verb_info  Parameters for the MGCP operation.
@@ -564,16 +611,18 @@ void osmo_mgcpc_ep_ci_request(struct osmo_mgcpc_ep_ci *ci,
 		.occupied = true,
 		/* .pending = true follows below */
 		.verb = verb,
-		.notify = notify,
-		.notify_success = event_success,
-		.notify_failure = event_failure,
-		.notify_data = notify_data,
+		.notify = {
+			.fi = notify,
+			.success = event_success,
+			.failure = event_failure,
+			.data = notify_data,
+		}
 	};
 	osmo_strlcpy(cleared_ci.label, ci->label, sizeof(cleared_ci.label));
 	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));
+	LOG_CI_VERB(ci, LOGL_DEBUG, "notify=%s\n", osmo_fsm_inst_name(ci->notify.fi));
 
 	if (verb_info)
 		ci->verb_info = *verb_info;
@@ -631,10 +680,39 @@ dispatch_error:
 		osmo_fsm_inst_dispatch(notify, event_failure, notify_data);
 }
 
+/*! No longer notify for any state changes for any conns of this endpoint.
+ * Useful if the notify instance passed to osmo_mgcpc_ep_ci_request() is about to deallocate.
+ * \param ep  The endpoint FSM instance.
+ * \param notify  Which target to cancel notification for, if NULL cancel all notifications. */
+void osmo_mgcpc_ep_cancel_notify(struct osmo_mgcpc_ep *ep, struct osmo_fsm_inst *notify)
+{
+	struct fsm_notify *n;
+	int i;
+	for (i = 0; i < ARRAY_SIZE(ep->ci); i++) {
+		struct osmo_mgcpc_ep_ci *ci = &ep->ci[i];
+		if (!notify || ci->notify.fi == notify)
+			ci->notify.fi = NULL;
+	}
+	llist_for_each_entry(n, &ep->background_notify, entry) {
+		if (!notify || n->fi == notify)
+			n->fi = NULL;
+	}
+
+}
+
+/* Return the osmo_mgcpc_ep that this conn belongs to. */
+struct osmo_mgcpc_ep *osmo_mgcpc_ep_ci_ep(struct osmo_mgcpc_ep_ci *conn)
+{
+	if (!conn)
+		return NULL;
+	return conn->ep;
+}
+
 static int send_verb(struct osmo_mgcpc_ep_ci *ci)
 {
 	int rc;
 	struct osmo_mgcpc_ep *ep = ci->ep;
+	struct fsm_notify notify;
 
 	if (!ci->occupied || !ci->pending || ci->sent)
 		return 0;
@@ -673,11 +751,14 @@ static int send_verb(struct osmo_mgcpc_ep_ci *ci)
 		       osmo_mgcp_verb_name(ci->verb), ci->mgcp_ci_str);
 		/* The way this is designed, we actually need to forget all about the ci right away. */
 		mgcp_conn_delete(ci->mgcp_client_fi);
-		if (ci->notify)
-			osmo_fsm_inst_dispatch(ci->notify, ci->notify_success, ci->notify_data);
+		notify = ci->notify;
 		*ci = (struct osmo_mgcpc_ep_ci){
 			.ep = ep,
 		};
+		/* When dispatching an event for this CI, the user may decide to trigger the next request for this conn
+		 * right away. So we must be ready with a cleared *ci. */
+		if (notify.fi)
+			osmo_fsm_inst_dispatch(notify.fi, notify.success, notify.data);
 		break;
 
 	default:
@@ -692,6 +773,7 @@ void osmo_mgcpc_ep_clear(struct osmo_mgcpc_ep *ep)
 {
 	if (!ep)
 		return;
+	osmo_mgcpc_ep_cancel_notify(ep, NULL);
 	osmo_fsm_inst_term(ep->fi, OSMO_FSM_TERM_REGULAR, 0);
 }
 

src/libosmo-mgcp-client/mgcp_client_fsm.c

@@ -525,8 +525,10 @@ static void fsm_cleanup_cb(struct osmo_fsm_inst *fi, enum osmo_fsm_term_cause ca
 		LOGPFSML(fi, LOGL_ERROR,
 			 "MGW/DLCX: abrupt FSM termination with connections still present, sending unconditional DLCX...\n");
 		msg = make_dlcx_msg(mgcp_ctx);
-		OSMO_ASSERT(msg);
-		mgcp_client_tx(mgcp, msg, NULL, NULL);
+		if (!msg)
+			LOGPFSML(fi, LOGL_ERROR, "MGW/DLCX: Error composing DLCX message\n");
+		else
+			mgcp_client_tx(mgcp, msg, NULL, NULL);
 	}
 
 	talloc_free(mgcp_ctx);
@@ -604,7 +606,6 @@ struct osmo_fsm_inst *mgcp_conn_create(struct mgcp_client *mgcp, struct osmo_fsm
 				       uint32_t parent_term_evt, uint32_t parent_evt, struct mgcp_conn_peer *conn_peer)
 {
 	struct mgcp_ctx *mgcp_ctx;
-	static bool fsm_registered = false;
 	struct osmo_fsm_inst *fi;
 	struct in_addr ip_test;
 
@@ -616,12 +617,6 @@ struct osmo_fsm_inst *mgcp_conn_create(struct mgcp_client *mgcp, struct osmo_fsm
 	if (conn_peer->port && inet_aton(conn_peer->addr, &ip_test) == 0)
 		return NULL;
 
-	/* Register the fsm description (if not already done) */
-	if (fsm_registered == false) {
-		osmo_fsm_register(&fsm_mgcp_client);
-		fsm_registered = true;
-	}
-
 	/* Allocate and configure a new fsm instance */
 	fi = osmo_fsm_inst_alloc_child(&fsm_mgcp_client, parent_fi, parent_term_evt);
 	OSMO_ASSERT(fi);
@@ -706,6 +701,9 @@ void mgcp_conn_delete(struct osmo_fsm_inst *fi)
 
 	OSMO_ASSERT(mgcp_ctx);
 
+	if (fi->proc.terminating)
+		return;
+
 	/* Unlink FSM from parent */
 	osmo_fsm_inst_unlink_parent(fi, NULL);
 
@@ -743,3 +741,8 @@ const char *osmo_mgcpc_conn_peer_name(const struct mgcp_conn_peer *info)
 		return "empty";
 	return buf;
 }
+
+static __attribute__((constructor)) void osmo_mgcp_client_fsm_init()
+{
+	OSMO_ASSERT(osmo_fsm_register(&fsm_mgcp_client) == 0);
+}

src/libosmo-mgcp/mgcp_codec.c

@@ -76,36 +76,61 @@ void mgcp_codec_summary(struct mgcp_conn_rtp *conn)
 }
 
 /* Initalize or reset codec information with default data. */
-void codec_init(struct mgcp_rtp_codec *codec)
+static void codec_init(struct mgcp_rtp_codec *codec)
+{
+	*codec = (struct mgcp_rtp_codec){
+		.payload_type = -1,
+		.frame_duration_num = DEFAULT_RTP_AUDIO_FRAME_DUR_NUM,
+		.frame_duration_den = DEFAULT_RTP_AUDIO_FRAME_DUR_DEN,
+		.rate = DEFAULT_RTP_AUDIO_DEFAULT_RATE,
+		.channels = DEFAULT_RTP_AUDIO_DEFAULT_CHANNELS,
+	};
+}
+
+static void codec_free(struct mgcp_rtp_codec *codec)
 {
 	if (codec->subtype_name)
 		talloc_free(codec->subtype_name);
 	if (codec->audio_name)
 		talloc_free(codec->audio_name);
-	memset(codec, 0, sizeof(*codec));
-	codec->payload_type = -1;
-	codec->frame_duration_num = DEFAULT_RTP_AUDIO_FRAME_DUR_NUM;
-	codec->frame_duration_den = DEFAULT_RTP_AUDIO_FRAME_DUR_DEN;
-	codec->rate = DEFAULT_RTP_AUDIO_DEFAULT_RATE;
-	codec->channels = DEFAULT_RTP_AUDIO_DEFAULT_CHANNELS;
+	*codec = (struct mgcp_rtp_codec){};
 }
 
 /*! Initalize or reset codec information with default data.
  *  \param[out] conn related rtp-connection. */
 void mgcp_codec_reset_all(struct mgcp_conn_rtp *conn)
 {
-	memset(conn->end.codecs, 0, sizeof(conn->end.codecs));
+	int i;
+	for (i = 0; i < conn->end.codecs_assigned; i++)
+		codec_free(&conn->end.codecs[i]);
 	conn->end.codecs_assigned = 0;
 	conn->end.codec = NULL;
 }
 
-/* Set members of struct mgcp_rtp_codec, extrapolate in missing information. Param audio_name is expected in uppercase. */
-static int codec_set(void *ctx, struct mgcp_rtp_codec *codec, int payload_type, const char *audio_name,
-		     unsigned int pt_offset, struct mgcp_codec_param *param)
+/*! 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)
 {
 	int rate;
 	int channels;
 	char audio_codec[64];
+	struct mgcp_rtp_codec *codec;
+	unsigned int pt_offset = conn->end.codecs_assigned;
+	void *ctx = conn->conn;
+
+	/* The amount of codecs we can store is limited, make sure we do not
+	 * overrun this limit. */
+	if (conn->end.codecs_assigned >= MGCP_MAX_CODECS)
+		return -EINVAL;
+
+	/* First unused entry */
+	codec = &conn->end.codecs[conn->end.codecs_assigned];
 
 	/* Initalize the codec struct with some default data to begin with */
 	codec_init(codec);
@@ -113,12 +138,13 @@ static int codec_set(void *ctx, struct mgcp_rtp_codec *codec, int payload_type,
 	if (payload_type != PTYPE_UNDEFINED) {
 		/* Make sure we do not get any reserved or undefined type numbers */
 		/* See also: https://www.iana.org/assignments/rtp-parameters/rtp-parameters.xhtml */
-		if (payload_type == 1 || payload_type == 2 || payload_type == 19)
-			goto error;
-		if (payload_type >= 72 && payload_type <= 76)
-			goto error;
-		if (payload_type >= 127)
+		if ((payload_type == 1 || payload_type == 2 || payload_type == 19)
+		    || (payload_type >= 72 && payload_type <= 76)
+		    || (payload_type >= 127)) {
+			LOGP(DLMGCP, LOGL_ERROR, "Cannot add codec, payload type number %d is reserved\n",
+			     payload_type);
 			goto error;
+		}
 
 		codec->payload_type = payload_type;
 	}
@@ -144,6 +170,8 @@ static int codec_set(void *ctx, struct mgcp_rtp_codec *codec, int payload_type,
 			/* The given payload type is not known to us, or it
 			 * it is a dynamic payload type for which we do not
 			 * know the audio name. We must give up here */
+			LOGP(DLMGCP, LOGL_ERROR, "No audio codec name given, and payload type %d unknown\n",
+			     payload_type);
 			goto error;
 		}
 	}
@@ -151,16 +179,23 @@ static int codec_set(void *ctx, struct mgcp_rtp_codec *codec, int payload_type,
 	/* Now we extract the codec subtype name, rate and channels. The latter
 	 * two are optional. If they are not present we use the safe defaults
 	 * above. */
-	if (strlen(audio_name) > sizeof(audio_codec))
+	if (strlen(audio_name) >= sizeof(audio_codec)) {
+		LOGP(DLMGCP, LOGL_ERROR, "Audio codec too long: %s\n", osmo_quote_str(audio_name, -1));
 		goto error;
+	}
 	channels = DEFAULT_RTP_AUDIO_DEFAULT_CHANNELS;
 	rate = DEFAULT_RTP_AUDIO_DEFAULT_RATE;
-	if (sscanf(audio_name, "%63[^/]/%d/%d", audio_codec, &rate, &channels) < 1)
+	if (sscanf(audio_name, "%63[^/]/%d/%d", audio_codec, &rate, &channels) < 1) {
+		LOGP(DLMGCP, LOGL_ERROR, "Invalid audio codec: %s\n", osmo_quote_str(audio_name, -1));
 		goto error;
+	}
 
 	/* Note: We only accept configurations with one audio channel! */
-	if (channels != 1)
+	if (channels != 1) {
+		LOGP(DLMGCP, LOGL_ERROR, "Cannot handle audio codec with more than one channel: %s\n",
+		     osmo_quote_str(audio_name, -1));
 		goto error;
+	}
 
 	codec->rate = rate;
 	codec->channels = channels;
@@ -178,6 +213,7 @@ static int codec_set(void *ctx, struct mgcp_rtp_codec *codec, int payload_type,
 
 	/* Derive the payload type if it is unknown */
 	if (codec->payload_type == PTYPE_UNDEFINED) {
+		/* TODO: This is semi dead code, see OS#4150 */
 
 		/* For the known codecs from the static range we restore
 		 * the IANA or 3GPP assigned payload type number */
@@ -213,9 +249,14 @@ static int codec_set(void *ctx, struct mgcp_rtp_codec *codec, int payload_type,
 		 * 110 onwards 3gpp defines prefered codec types, which are
 		 * also fixed, see above)  */
 		if (codec->payload_type < 0) {
+			/* FIXME: pt_offset is completely unrelated and useless here, any of those numbers may already
+			 * have been added to the codecs. Instead, there should be an iterator checking for an actually
+			 * unused dynamic payload type number. */
 			codec->payload_type = 96 + pt_offset;
-			if (codec->payload_type > 109)
+			if (codec->payload_type > 109) {
+				LOGP(DLMGCP, LOGL_ERROR, "Ran out of payload type numbers to assign dynamically\n");
 				goto error;
+			}
 		}
 	}
 
@@ -226,41 +267,14 @@ static int codec_set(void *ctx, struct mgcp_rtp_codec *codec, int payload_type,
 	} else
 		codec->param_present = false;
 
+	conn->end.codecs_assigned++;
 	return 0;
 error:
 	/* Make sure we leave a clean codec entry on error. */
-	codec_init(codec);
-	memset(codec, 0, sizeof(*codec));
+	codec_free(codec);
 	return -EINVAL;
 }
 
-/*! 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, struct mgcp_codec_param *param)
-{
-	int rc;
-
-	/* The amount of codecs we can store is limited, make sure we do not
-	 * overrun this limit. */
-	if (conn->end.codecs_assigned >= MGCP_MAX_CODECS)
-		return -EINVAL;
-
-	rc = codec_set(conn->conn, &conn->end.codecs[conn->end.codecs_assigned], payload_type, audio_name,
-		       conn->end.codecs_assigned, param);
-	if (rc != 0)
-		return -EINVAL;
-
-	conn->end.codecs_assigned++;
-
-	return 0;
-}
-
 /* Check if the given codec is applicable on the specified endpoint
  * Helper function for mgcp_codec_decide() */
 static bool is_codec_compatible(const struct mgcp_endpoint *endp, const struct mgcp_rtp_codec *codec)
@@ -350,9 +364,28 @@ int mgcp_codec_decide(struct mgcp_conn_rtp *conn)
 	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
+ */
+static bool 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;
+}
+
 /* Compare two codecs, all parameters must match up, except for the payload type
  * number. */
-static bool codecs_cmp(struct mgcp_rtp_codec *codec_a, struct mgcp_rtp_codec *codec_b)
+static bool codecs_same(struct mgcp_rtp_codec *codec_a, struct mgcp_rtp_codec *codec_b)
 {
 	if (codec_a->rate != codec_b->rate)
 		return false;
@@ -362,10 +395,12 @@ static bool codecs_cmp(struct mgcp_rtp_codec *codec_a, struct mgcp_rtp_codec *co
 		return false;
 	if (codec_a->frame_duration_den != codec_b->frame_duration_den)
 		return false;
-	if (strcmp(codec_a->audio_name, codec_b->audio_name))
-		return false;
 	if (strcmp(codec_a->subtype_name, codec_b->subtype_name))
 		return false;
+	if (!strcmp(codec_a->subtype_name, "AMR")) {
+		if (amr_is_octet_aligned(codec_a) != amr_is_octet_aligned(codec_b))
+			return false;
+	}
 
 	return true;
 }
@@ -406,7 +441,7 @@ int mgcp_codec_pt_translate(struct mgcp_conn_rtp *conn_src, struct mgcp_conn_rtp
 	codecs_assigned = rtp_dst->codecs_assigned;
 	OSMO_ASSERT(codecs_assigned <= MGCP_MAX_CODECS);
 	for (i = 0; i < codecs_assigned; i++) {
-		if (codecs_cmp(codec_src, &rtp_dst->codecs[i])) {
+		if (codecs_same(codec_src, &rtp_dst->codecs[i])) {
 			codec_dst = &rtp_dst->codecs[i];
 			break;
 		}

src/libosmo-mgcp/mgcp_conn.c

@@ -129,6 +129,7 @@ static void mgcp_rtp_conn_cleanup(struct mgcp_conn_rtp *conn_rtp)
 		conn_osmux_disable(conn_rtp);
 	mgcp_free_rtp_port(&conn_rtp->end);
 	rate_ctr_group_free(conn_rtp->rate_ctr_group);
+	mgcp_codec_reset_all(conn_rtp);
 }
 
 void mgcp_conn_watchdog_cb(void *data)

src/libosmo-mgcp/mgcp_network.c

@@ -475,7 +475,9 @@ static int mgcp_patch_pt(struct mgcp_conn_rtp *conn_src,
 	uint8_t pt_in;
 	int pt_out;
 
-	OSMO_ASSERT(len >= sizeof(struct rtp_hdr));
+	if (len < sizeof(struct rtp_hdr))
+		return -EINVAL;
+
 	rtp_hdr = (struct rtp_hdr *)data;
 
 	pt_in = rtp_hdr->payload_type;
@@ -640,8 +642,9 @@ void mgcp_patch_and_count(struct mgcp_endpoint *endp,
 /* There are different dialects used to format and transfer voice data. When
  * the receiving end expects GSM-HR data to be formated after RFC 5993, this
  * function is used to convert between RFC 5993 and TS 101318, which we normally
- * use. */
-static void rfc5993_hr_convert(struct mgcp_endpoint *endp, char *data, int *len)
+ * use.
+ * Return 0 on sucess, negative on errors like invalid data length. */
+static int rfc5993_hr_convert(struct mgcp_endpoint *endp, char *data, int *len)
 {
 	/* NOTE: *data has an overall length of RTP_BUF_SIZE, so there is
 	 * plenty of space available to store the slightly larger, converted
@@ -649,7 +652,12 @@ static void rfc5993_hr_convert(struct mgcp_endpoint *endp, char *data, int *len)
 
 	struct rtp_hdr *rtp_hdr;
 
-	OSMO_ASSERT(*len >= sizeof(struct rtp_hdr));
+	if (*len < sizeof(struct rtp_hdr)) {
+		LOGPENDP(endp, DRTP, LOGL_ERROR, "AMR RTP packet too short (%d < %zu)\n",
+			 *len, sizeof(struct rtp_hdr));
+		return -EINVAL;
+	}
+
 	rtp_hdr = (struct rtp_hdr *)data;
 
 	if (*len == GSM_HR_BYTES + sizeof(struct rtp_hdr)) {
@@ -667,7 +675,9 @@ static void rfc5993_hr_convert(struct mgcp_endpoint *endp, char *data, int *len)
 		 * packet. This is not supported yet. */
 		LOGPENDP(endp, DRTP, LOGL_ERROR,
 			 "cannot figure out how to convert RTP packet\n");
+		return -ENOTSUP;
 	}
+	return 0;
 }
 
 /* For AMR RTP two framing modes are defined RFC3267. There is a bandwith
@@ -685,7 +695,11 @@ static int amr_oa_bwe_convert(struct mgcp_endpoint *endp, char *data, int *len,
 	unsigned int payload_len;
 	int rc;
 
-	OSMO_ASSERT(*len >= sizeof(struct rtp_hdr));
+	if (*len < sizeof(struct rtp_hdr)) {
+		LOGPENDP(endp, DRTP, LOGL_ERROR, "AMR RTP packet too short (%d < %zu)\n", *len, sizeof(struct rtp_hdr));
+		return -EINVAL;
+	}
+
 	rtp_hdr = (struct rtp_hdr *)data;
 
 	payload_len = *len - sizeof(struct rtp_hdr);
@@ -735,18 +749,23 @@ static bool amr_oa_bwe_convert_indicated(struct mgcp_rtp_codec *codec)
 }
 
 
-/* Check if a given RTP with AMR payload for octet-aligned mode */
-static bool amr_oa_check(char *data, int len)
+/* Return whether an RTP packet with AMR payload is in octet-aligned mode.
+ * Return 0 if in bandwidth-efficient mode, 1 for octet-aligned mode, and negative if the RTP data is invalid. */
+static int amr_oa_check(char *data, int len)
 {
 	struct rtp_hdr *rtp_hdr;
 	unsigned int payload_len;
 
-	OSMO_ASSERT(len >= sizeof(struct rtp_hdr));
+	if (len < sizeof(struct rtp_hdr))
+		return -EINVAL;
+
 	rtp_hdr = (struct rtp_hdr *)data;
 
 	payload_len = len - sizeof(struct rtp_hdr);
+	if (payload_len < sizeof(struct amr_hdr))
+		return -EINVAL;
 
-	return osmo_amr_is_oa(rtp_hdr->data, payload_len);
+	return osmo_amr_is_oa(rtp_hdr->data, payload_len) ? 1 : 0;
 }
 
 /* Forward data to a debug tap. This is debug function that is intended for
@@ -851,13 +870,23 @@ int mgcp_send(struct mgcp_endpoint *endp, int is_rtp, struct sockaddr_in *addr,
 						     addr, buf, buflen);
 
 			if (amr_oa_bwe_convert_indicated(conn_dst->end.codec)) {
-				amr_oa_bwe_convert(endp, buf, &buflen,
-						   conn_dst->end.codec->param.amr_octet_aligned);
+				rc = amr_oa_bwe_convert(endp, buf, &buflen,
+							conn_dst->end.codec->param.amr_octet_aligned);
+				if (rc < 0) {
+					LOGPENDP(endp, DRTP, LOGL_ERROR,
+						 "Error in AMR octet-aligned <-> bandwidth-efficient mode conversion\n");
+					break;
+				}
 			}
 			else if (rtp_end->rfc5993_hr_convert
 			    && strcmp(conn_src->end.codec->subtype_name,
-				      "GSM-HR-08") == 0)
-				rfc5993_hr_convert(endp, buf, &buflen);
+				      "GSM-HR-08") == 0) {
+				rc = rfc5993_hr_convert(endp, buf, &buflen);
+				if (rc < 0) {
+					LOGPENDP(endp, DRTP, LOGL_ERROR, "Error while converting to GSM-HR-08\n");
+					break;
+				}
+			}
 
 			LOGPENDP(endp, DRTP, LOGL_DEBUG,
 				 "process/send to %s %s "
@@ -1340,7 +1369,10 @@ static int rtp_data_net(struct osmo_fd *fd, unsigned int what)
 	 * define, then we check if the incoming payload matches that
 	 * expectation. */
 	if (amr_oa_bwe_convert_indicated(conn_src->end.codec)) {
-		if (amr_oa_check(buf, len) != conn_src->end.codec->param.amr_octet_aligned)
+		int oa = amr_oa_check(buf, len);
+		if (oa < 0)
+			return -1;
+		if (((bool)oa) != conn_src->end.codec->param.amr_octet_aligned)
 			return -1;
 	}
 

src/libosmo-mgcp/mgcp_protocol.c

@@ -919,7 +919,7 @@ mgcp_header_done:
 	endp->callid = talloc_strdup(tcfg->endpoints, callid);
 
 	snprintf(conn_name, sizeof(conn_name), "%s", callid);
-	_conn = mgcp_conn_alloc(NULL, endp, MGCP_CONN_TYPE_RTP, conn_name);
+	_conn = mgcp_conn_alloc(tcfg->endpoints, endp, MGCP_CONN_TYPE_RTP, conn_name);
 	if (!_conn) {
 		LOGPENDP(endp, DLMGCP, LOGL_ERROR,
 			 "CRCX: unable to allocate RTP connection\n");

src/osmo-mgw/mgw_main.c

@@ -132,6 +132,10 @@ static void handle_options(int argc, char **argv)
 			break;
 		};
 	}
+	if (argc > optind) {
+		fprintf(stderr, "Unsupported positional arguments on command line\n");
+		exit(2);
+	}
 }
 
 /* Callback function to be called when the RSIP ("Reset in Progress") mgcp

tests/mgcp/mgcp_test.c

@@ -641,6 +641,13 @@ int clock_gettime(clockid_t clk_id, struct timespec *tp)
 	return real_clock_gettime(clk_id, tp);
 }
 
+static void mgcp_endpoints_release(struct mgcp_trunk_config *trunk)
+{
+	int i;
+	for (i = 1; i < trunk->number_endpoints; i++)
+		mgcp_endp_release(&trunk->endpoints[i]);
+}
+
 #define CONN_UNMODIFIED (0x1000)
 
 static void test_values(void)
@@ -742,6 +749,7 @@ static void test_messages(void)
 {
 	struct mgcp_config *cfg;
 	struct mgcp_endpoint *endp;
+	struct mgcp_trunk_config *trunk2;
 	int i;
 	struct mgcp_conn_rtp *conn = NULL;
 	char last_conn_id[256];
@@ -755,7 +763,8 @@ static void test_messages(void)
 
 	memset(last_conn_id, 0, sizeof(last_conn_id));
 
-	mgcp_endpoints_allocate(mgcp_trunk_alloc(cfg, 1));
+	trunk2 = mgcp_trunk_alloc(cfg, 1);
+	mgcp_endpoints_allocate(trunk2);
 
 	for (i = 0; i < ARRAY_SIZE(tests); i++) {
 		const struct mgcp_test *t = &tests[i];
@@ -873,12 +882,15 @@ static void test_messages(void)
 		}
 	}
 
+	mgcp_endpoints_release(trunk2);
+	mgcp_endpoints_release(&cfg->trunk);
 	talloc_free(cfg);
 }
 
 static void test_retransmission(void)
 {
 	struct mgcp_config *cfg;
+	struct mgcp_trunk_config *trunk2;
 	int i;
 	char last_conn_id[256];
 	int rc;
@@ -890,7 +902,8 @@ static void test_retransmission(void)
 
 	memset(last_conn_id, 0, sizeof(last_conn_id));
 
-	mgcp_endpoints_allocate(mgcp_trunk_alloc(cfg, 1));
+	trunk2 = mgcp_trunk_alloc(cfg, 1);
+	mgcp_endpoints_allocate(trunk2);
 
 	for (i = 0; i < ARRAY_SIZE(retransmit); i++) {
 		const struct mgcp_test *t = &retransmit[i];
@@ -930,6 +943,8 @@ static void test_retransmission(void)
 		msgb_free(msg);
 	}
 
+	mgcp_endpoints_release(trunk2);
+	mgcp_endpoints_release(&cfg->trunk);
 	talloc_free(cfg);
 }
 
@@ -943,6 +958,7 @@ static int rqnt_cb(struct mgcp_endpoint *endp, char _tone)
 static void test_rqnt_cb(void)
 {
 	struct mgcp_config *cfg;
+	struct mgcp_trunk_config *trunk2;
 	struct msgb *inp, *msg;
 	char conn_id[256];
 
@@ -952,7 +968,8 @@ static void test_rqnt_cb(void)
 	cfg->trunk.vty_number_endpoints = 64;
 	mgcp_endpoints_allocate(&cfg->trunk);
 
-	mgcp_endpoints_allocate(mgcp_trunk_alloc(cfg, 1));
+	trunk2 = mgcp_trunk_alloc(cfg, 1);
+	mgcp_endpoints_allocate(trunk2);
 
 	inp = create_msg(CRCX, NULL);
 	msg = mgcp_handle_message(cfg, inp);
@@ -981,6 +998,8 @@ static void test_rqnt_cb(void)
 	inp = create_msg(DLCX, conn_id);
 	msgb_free(mgcp_handle_message(cfg, inp));
 	msgb_free(inp);
+	mgcp_endpoints_release(trunk2);
+	mgcp_endpoints_release(&cfg->trunk);
 	talloc_free(cfg);
 }
 
@@ -1342,12 +1361,12 @@ static void test_packet_error_detection(int patch_ssrc, int patch_ts)
 static void test_multilple_codec(void)
 {
 	struct mgcp_config *cfg;
+	struct mgcp_trunk_config *trunk2;
 	struct mgcp_endpoint *endp;
 	struct msgb *inp, *resp;
 	struct in_addr addr;
 	struct mgcp_conn_rtp *conn = NULL;
 	char conn_id[256];
-	int i;
 
 	printf("Testing multiple payload types\n");
 
@@ -1355,7 +1374,9 @@ static void test_multilple_codec(void)
 	cfg->trunk.vty_number_endpoints = 64;
 	mgcp_endpoints_allocate(&cfg->trunk);
 	cfg->policy_cb = mgcp_test_policy_cb;
-	mgcp_endpoints_allocate(mgcp_trunk_alloc(cfg, 1));
+
+	trunk2 = mgcp_trunk_alloc(cfg, 1);
+	mgcp_endpoints_allocate(trunk2);
 
 	/* Allocate endpoint 1@mgw with two codecs */
 	last_endpoint = -1;
@@ -1481,9 +1502,8 @@ static void test_multilple_codec(void)
 	OSMO_ASSERT(conn);
 	OSMO_ASSERT(conn->end.codec->payload_type == 0);
 
-	for (i = 1; i < cfg->trunk.number_endpoints; i++)
-		mgcp_endp_release(&cfg->trunk.endpoints[i]);
-
+	mgcp_endpoints_release(trunk2);
+	mgcp_endpoints_release(&cfg->trunk);
 	talloc_free(cfg);
 }
 
@@ -1532,12 +1552,13 @@ static void test_no_cycle(void)
 	OSMO_ASSERT(conn->state.stats.cycles == UINT16_MAX + 1);
 	OSMO_ASSERT(conn->state.stats.max_seq == 0);
 
-	mgcp_endp_release(endp);
+	mgcp_endpoints_release(&cfg->trunk);
 	talloc_free(cfg);
 }
 
 static void test_no_name(void)
 {
+	struct mgcp_trunk_config *trunk2;
 	struct mgcp_config *cfg;
 	struct msgb *inp, *msg;
 
@@ -1550,7 +1571,8 @@ static void test_no_name(void)
 
 	cfg->policy_cb = mgcp_test_policy_cb;
 
-	mgcp_endpoints_allocate(mgcp_trunk_alloc(cfg, 1));
+	trunk2 = mgcp_trunk_alloc(cfg, 1);
+	mgcp_endpoints_allocate(trunk2);
 
 	inp = create_msg(CRCX, NULL);
 	msg = mgcp_handle_message(cfg, inp);
@@ -1563,7 +1585,8 @@ static void test_no_name(void)
 	msgb_free(inp);
 	msgb_free(msg);
 
-	mgcp_endp_release(&cfg->trunk.endpoints[1]);
+	mgcp_endpoints_release(trunk2);
+	mgcp_endpoints_release(&cfg->trunk);
 	talloc_free(cfg);
 }
 
@@ -1711,98 +1734,335 @@ static void test_check_local_cx_options(void *ctx)
 	OSMO_ASSERT(check_local_cx_options(ctx, ",,,") == -1);
 }
 
-static void test_mgcp_codec_pt_translate_pars(struct mgcp_rtp_codec *c)
-{
-	c->rate = 8000;
-	c->channels = 1;
-	c->frame_duration_num = 23;
-	c->frame_duration_den = 42;
-}
+static const struct mgcp_codec_param amr_param_octet_aligned_true = {
+	.amr_octet_aligned_present = true,
+	.amr_octet_aligned = true,
+};
+
+static const struct mgcp_codec_param amr_param_octet_aligned_false = {
+	.amr_octet_aligned_present = true,
+	.amr_octet_aligned = false,
+};
+
+static const struct mgcp_codec_param amr_param_octet_aligned_unset = {
+	.amr_octet_aligned_present = false,
+};
+
+struct testcase_mgcp_codec_pt_translate_codec {
+	int payload_type;
+	const char *audio_name;
+	const struct mgcp_codec_param *param;
+	int expect_rc;
+};
+
+struct testcase_mgcp_codec_pt_translate_expect {
+	bool end;
+	int payload_type_map[2];
+};
+
+struct testcase_mgcp_codec_pt_translate {
+	const char *descr;
+	/* two conns on an endpoint, each with N configured codecs */
+	struct testcase_mgcp_codec_pt_translate_codec codecs[2][10];
+	struct testcase_mgcp_codec_pt_translate_expect expect[32];
+};
+
+static const struct testcase_mgcp_codec_pt_translate test_mgcp_codec_pt_translate_cases[] = {
+	{
+		.descr = "same order, but differing payload type numbers",
+		.codecs = {
+			{
+				{ 112, "AMR/8000/1", &amr_param_octet_aligned_true, },
+				{ 0, "PCMU/8000/1", NULL, },
+				{ 111, "GSM-HR-08/8000/1", NULL, },
+			},
+			{
+				{ 96, "AMR/8000/1", &amr_param_octet_aligned_true, },
+				{ 0, "PCMU/8000/1", NULL, },
+				{ 97, "GSM-HR-08/8000/1", NULL, },
+			},
+		},
+		.expect = {
+			{ .payload_type_map = {112, 96}, },
+			{ .payload_type_map = {0, 0}, },
+			{ .payload_type_map = {111, 97} },
+			{ .payload_type_map = {123, -EINVAL} },
+			{ .end = true },
+		},
+	},
+	{
+		.descr = "different order and different payload type numbers",
+		.codecs = {
+			{
+				{ 0, "PCMU/8000/1", NULL, },
+				{ 111, "GSM-HR-08/8000/1", NULL, },
+				{ 112, "AMR/8000/1", &amr_param_octet_aligned_true, },
+			},
+			{
+				{ 97, "GSM-HR-08/8000/1", NULL, },
+				{ 0, "PCMU/8000/1", NULL, },
+				{ 96, "AMR/8000/1", &amr_param_octet_aligned_true, },
+			},
+		},
+		.expect = {
+			{ .payload_type_map = {112, 96}, },
+			{ .payload_type_map = {0, 0}, },
+			{ .payload_type_map = {111, 97} },
+			{ .payload_type_map = {123, -EINVAL} },
+			{ .end = true },
+		},
+	},
+	{
+		.descr = "both sides have the same payload_type numbers assigned to differing codecs",
+		.codecs = {
+			{
+				{ 0, "PCMU/8000/1", NULL, },
+				{ 96, "GSM-HR-08/8000/1", NULL, },
+				{ 97, "AMR/8000/1", &amr_param_octet_aligned_true, },
+			},
+			{
+				{ 97, "GSM-HR-08/8000/1", NULL, },
+				{ 0, "PCMU/8000/1", NULL, },
+				{ 96, "AMR/8000/1", &amr_param_octet_aligned_true, },
+			},
+		},
+		.expect = {
+			{ .payload_type_map = {96, 97}, },
+			{ .payload_type_map = {97, 96}, },
+			{ .payload_type_map = {0, 0}, },
+			{ .end = true },
+		},
+	},
+	{
+		.descr = "conn0 has no codecs",
+		.codecs = {
+			{
+				/* no codecs */
+			},
+			{
+				{ 96, "AMR/8000/1", &amr_param_octet_aligned_true, },
+				{ 0, "PCMU/8000/1", NULL, },
+				{ 97, "GSM-HR-08/8000/1", NULL, },
+			},
+		},
+		.expect = {
+			{ .payload_type_map = {112, -EINVAL}, },
+			{ .payload_type_map = {0, -EINVAL}, },
+			{ .payload_type_map = {111, -EINVAL} },
+			{ .end = true },
+		},
+	},
+	{
+		.descr = "conn1 has no codecs",
+		.codecs = {
+			{
+				{ 112, "AMR/8000/1", &amr_param_octet_aligned_true, },
+				{ 0, "PCMU/8000/1", NULL, },
+				{ 111, "GSM-HR-08/8000/1", NULL, },
+			},
+			{
+				/* no codecs */
+			},
+		},
+		.expect = {
+			{ .payload_type_map = {112, -EINVAL}, },
+			{ .payload_type_map = {0, -EINVAL}, },
+			{ .payload_type_map = {111, -EINVAL} },
+			{ .end = true },
+		},
+	},
+	{
+		.descr = "test AMR with differing octet-aligned settings",
+		.codecs = {
+			{
+				{ 111, "AMR/8000", &amr_param_octet_aligned_true, },
+				{ 112, "AMR/8000", &amr_param_octet_aligned_false, },
+			},
+			{
+				{ 122, "AMR/8000", &amr_param_octet_aligned_false, },
+				{ 121, "AMR/8000", &amr_param_octet_aligned_true, },
+			},
+		},
+		.expect = {
+			{ .payload_type_map = {111, 121}, },
+			{ .payload_type_map = {112, 122} },
+			{ .end = true },
+		},
+	},
+	{
+		.descr = "test AMR with missing octet-aligned settings (defaults to 0)",
+		.codecs = {
+			{
+				{ 111, "AMR/8000", &amr_param_octet_aligned_true, },
+				{ 112, "AMR/8000", &amr_param_octet_aligned_false, },
+			},
+			{
+				{ 122, "AMR/8000", &amr_param_octet_aligned_unset, },
+			},
+		},
+		.expect = {
+			{ .payload_type_map = {111, -EINVAL}, },
+			{ .payload_type_map = {112, 122} },
+			{ .end = true },
+		},
+	},
+	{
+		.descr = "test AMR with NULL param (defaults to 0)",
+		.codecs = {
+			{
+				{ 111, "AMR/8000", &amr_param_octet_aligned_true, },
+				{ 112, "AMR/8000", &amr_param_octet_aligned_false, },
+			},
+			{
+				{ 122, "AMR/8000", NULL, },
+			},
+		},
+		.expect = {
+			{ .payload_type_map = {111, -EINVAL}, },
+			{ .payload_type_map = {112, 122} },
+			{ .end = true },
+		},
+	},
+	{
+		.descr = "match FOO/8000/1 and FOO/8000 as identical, single channel is implicit",
+		.codecs = {
+			{
+				{ 0, "PCMU/8000/1", NULL, },
+				{ 111, "GSM-HR-08/8000/1", NULL, },
+				{ 112, "AMR/8000/1", &amr_param_octet_aligned_true, },
+			},
+			{
+				{ 97, "GSM-HR-08/8000", NULL, },
+				{ 0, "PCMU/8000", NULL, },
+				{ 96, "AMR/8000", &amr_param_octet_aligned_true, },
+			},
+		},
+		.expect = {
+			{ .payload_type_map = {112, 96}, },
+			{ .payload_type_map = {0, 0}, },
+			{ .payload_type_map = {111, 97} },
+			{ .payload_type_map = {123, -EINVAL} },
+			{ .end = true },
+		},
+	},
+	{
+		.descr = "match FOO/8000/1 and FOO as identical, 8k and single channel are implicit",
+		.codecs = {
+			{
+				{ 0, "PCMU/8000/1", NULL, },
+				{ 111, "GSM-HR-08/8000/1", NULL, },
+				{ 112, "AMR/8000/1", &amr_param_octet_aligned_true, },
+			},
+			{
+				{ 97, "GSM-HR-08", NULL, },
+				{ 0, "PCMU", NULL, },
+				{ 96, "AMR", &amr_param_octet_aligned_true, },
+			},
+		},
+		.expect = {
+			{ .payload_type_map = {112, 96}, },
+			{ .payload_type_map = {0, 0}, },
+			{ .payload_type_map = {111, 97} },
+			{ .payload_type_map = {123, -EINVAL} },
+			{ .end = true },
+		},
+	},
+	{
+		.descr = "test whether channel number matching is waterproof",
+		.codecs = {
+			{
+				{ 111, "GSM-HR-08/8000", },
+				{ 112, "GSM-HR-08/8000/2", .expect_rc = -22},
+				{ 113, "GSM-HR-08/8000/3", .expect_rc = -22},
+			},
+			{
+				{ 122, "GSM-HR-08/8000/2", .expect_rc = -22},
+				{ 121, "GSM-HR-08/8000/1", },
+			},
+		},
+		.expect = {
+			{ .payload_type_map = {111, 121}, },
+			{ .payload_type_map = {112, -EINVAL} },
+			{ .payload_type_map = {113, -EINVAL} },
+			{ .end = true },
+		},
+	},
+};
 
 static void test_mgcp_codec_pt_translate(void)
 {
-	struct mgcp_conn_rtp conn_src;
-	struct mgcp_conn_rtp conn_dst;
-	int pt_dst;
+	int i;
+	bool ok = true;
+	printf("\nTesting mgcp_codec_pt_translate()\n");
 
-	/* Setup a realistic set of codec configurations on both
-	 * ends. AMR and HR will use different payload types. PCMU
-	 * must use 0 on both ends since this is not a dynamic payload
-	 * type */
-	test_mgcp_codec_pt_translate_pars(&conn_src.end.codecs[0]);
-	test_mgcp_codec_pt_translate_pars(&conn_dst.end.codecs[0]);
-	test_mgcp_codec_pt_translate_pars(&conn_src.end.codecs[1]);
-	test_mgcp_codec_pt_translate_pars(&conn_dst.end.codecs[1]);
-	test_mgcp_codec_pt_translate_pars(&conn_src.end.codecs[2]);
-	test_mgcp_codec_pt_translate_pars(&conn_dst.end.codecs[2]);
-	conn_src.end.codecs[0].payload_type = 112;
-	conn_dst.end.codecs[0].payload_type = 96;
-	conn_src.end.codecs[1].payload_type = 0;
-	conn_dst.end.codecs[1].payload_type = 0;
-	conn_src.end.codecs[2].payload_type = 111;
-	conn_dst.end.codecs[2].payload_type = 97;
-	conn_src.end.codecs[0].audio_name = "AMR/8000/1";
-	conn_dst.end.codecs[0].audio_name = "AMR/8000/1";
-	conn_src.end.codecs[1].audio_name = "PCMU/8000/1";
-	conn_dst.end.codecs[1].audio_name = "PCMU/8000/1";
-	conn_src.end.codecs[2].audio_name = "GSM-HR-08/8000/1";
-	conn_dst.end.codecs[2].audio_name = "GSM-HR-08/8000/1";
-	conn_src.end.codecs[0].subtype_name = "AMR";
-	conn_dst.end.codecs[0].subtype_name = "AMR";
-	conn_src.end.codecs[1].subtype_name = "PCMU";
-	conn_dst.end.codecs[1].subtype_name = "PCMU";
-	conn_src.end.codecs[2].subtype_name = "GSM-HR-08";
-	conn_dst.end.codecs[2].subtype_name = "GSM-HR-08";
-	conn_src.end.codecs_assigned = 3;
-	conn_dst.end.codecs_assigned = 3;
+	for (i = 0; i < ARRAY_SIZE(test_mgcp_codec_pt_translate_cases); i++) {
+		const struct testcase_mgcp_codec_pt_translate *t = &test_mgcp_codec_pt_translate_cases[i];
+		struct mgcp_conn_rtp conn[2] = {};
+		int rc;
+		int conn_i;
+		int c;
 
-	/* We expect the function to find the PT we must use when we send the
-	 * packet out to the destination. All we know is the context for both
-	 * connections and the payload type from the source packet */
-	pt_dst =
-	    mgcp_codec_pt_translate(&conn_src, &conn_dst,
-				    conn_src.end.codecs[0].payload_type);
-	OSMO_ASSERT(pt_dst == conn_dst.end.codecs[0].payload_type);
-	pt_dst =
-	    mgcp_codec_pt_translate(&conn_src, &conn_dst,
-				    conn_src.end.codecs[1].payload_type);
-	OSMO_ASSERT(pt_dst == conn_dst.end.codecs[1].payload_type);
-	pt_dst =
-	    mgcp_codec_pt_translate(&conn_src, &conn_dst,
-				    conn_src.end.codecs[2].payload_type);
-	OSMO_ASSERT(pt_dst == conn_dst.end.codecs[2].payload_type);
+		printf("#%d: %s\n", i, t->descr);
 
-	/* Try some constellations that must fail */
-	pt_dst = mgcp_codec_pt_translate(&conn_src, &conn_dst, 123);
-	OSMO_ASSERT(pt_dst == -EINVAL);
-	conn_src.end.codecs_assigned = 0;
-	conn_dst.end.codecs_assigned = 3;
-	pt_dst =
-	    mgcp_codec_pt_translate(&conn_src, &conn_dst,
-				    conn_src.end.codecs[0].payload_type);
-	OSMO_ASSERT(pt_dst == -EINVAL);
-	pt_dst =
-	    mgcp_codec_pt_translate(&conn_src, &conn_dst,
-				    conn_src.end.codecs[1].payload_type);
-	OSMO_ASSERT(pt_dst == -EINVAL);
-	pt_dst =
-	    mgcp_codec_pt_translate(&conn_src, &conn_dst,
-				    conn_src.end.codecs[2].payload_type);
-	OSMO_ASSERT(pt_dst == -EINVAL);
-	conn_src.end.codecs_assigned = 3;
-	conn_dst.end.codecs_assigned = 0;
-	pt_dst =
-	    mgcp_codec_pt_translate(&conn_src, &conn_dst,
-				    conn_src.end.codecs[0].payload_type);
-	OSMO_ASSERT(pt_dst == -EINVAL);
-	pt_dst =
-	    mgcp_codec_pt_translate(&conn_src, &conn_dst,
-				    conn_src.end.codecs[1].payload_type);
-	OSMO_ASSERT(pt_dst == -EINVAL);
-	pt_dst =
-	    mgcp_codec_pt_translate(&conn_src, &conn_dst,
-				    conn_src.end.codecs[2].payload_type);
-	OSMO_ASSERT(pt_dst == -EINVAL);
+		for (conn_i = 0; conn_i < 2; conn_i++) {
+			printf(" - add codecs on conn%d:\n", conn_i);
+			for (c = 0; c < ARRAY_SIZE(t->codecs[conn_i]); c++) {
+				const struct testcase_mgcp_codec_pt_translate_codec *codec = &t->codecs[conn_i][c];
+				if (!codec->audio_name)
+					break;
+
+				rc = mgcp_codec_add(&conn[conn_i], codec->payload_type, codec->audio_name, codec->param);
+
+				printf("   %2d: %3d %s%s  -> rc=%d\n", c, codec->payload_type, codec->audio_name,
+				       codec->param ?
+					       (codec->param->amr_octet_aligned_present?
+							(codec->param->amr_octet_aligned ?
+								" octet-aligned=1" : " octet-aligned=0")
+							: " octet-aligned=unset")
+						: "",
+						rc);
+				if (rc != codec->expect_rc) {
+					printf("     ERROR: expected rc=%d\n", codec->expect_rc);
+					ok = false;
+				}
+			}
+			if (!c)
+				printf("    (none)\n");
+		}
+
+		for (c = 0; c < ARRAY_SIZE(t->expect); c++) {
+			const struct testcase_mgcp_codec_pt_translate_expect *expect = &t->expect[c];
+			int result;
+
+			if (expect->end)
+				break;
+
+			result = mgcp_codec_pt_translate(&conn[0], &conn[1], expect->payload_type_map[0]);
+			printf(" - mgcp_codec_pt_translate(conn0, conn1, %d) -> %d\n",
+			       expect->payload_type_map[0], result);
+			if (result != expect->payload_type_map[1]) {
+				printf("     ERROR: expected -> %d\n", expect->payload_type_map[1]);
+				ok = false;
+			}
+
+			/* If the expected result is an error, don't do reverse map test */
+			if (expect->payload_type_map[1] < 0)
+				continue;
+
+			result = mgcp_codec_pt_translate(&conn[1], &conn[0], expect->payload_type_map[1]);
+			printf(" - mgcp_codec_pt_translate(conn1, conn0, %d) -> %d\n",
+			       expect->payload_type_map[1], result);
+			if (result != expect->payload_type_map[0]) {
+				printf("     ERROR: expected -> %d\n", expect->payload_type_map[0]);
+				ok = false;
+			}
+		}
+
+		for (conn_i = 0; conn_i < 2; conn_i++)
+			mgcp_codec_reset_all(&conn[conn_i]);
+	}
+
+	OSMO_ASSERT(ok);
 }
 
 void test_conn_id_matching()

tests/mgcp/mgcp_test.ok

@@ -1218,6 +1218,148 @@ p:10, a:PCMU -> p:10, a:PCMU
 Testing get_lco_identifier()
 p:10, a:PCMU -> p:10, a:PCMU
 p:10, a:PCMU -> p:10, a:PCMU
+'XXXX, p:10, a:PCMU' -> 'p:10, a:PCMU'
+'XXXX,p:10,a:PCMU' -> 'p:10,a:PCMU'
+'10,a:PCMU' -> 'a:PCMU'
+'10, a:PCMU' -> 'a:PCMU'
+'10,a: PCMU' -> 'a: PCMU'
+'10 ,a: PCMU' -> 'a: PCMU'
+', a:PCMU' -> 'a:PCMU'
+' a:PCMU' -> 'a:PCMU'
+'' -> '(null)'
+p10, aPCMU -> (null)
+'10,a :PCMU' -> '(null)'
+
+Testing mgcp_codec_pt_translate()
+#0: same order, but differing payload type numbers
+ - add codecs on conn0:
+    0: 112 AMR/8000/1 octet-aligned=1  -> rc=0
+    1:   0 PCMU/8000/1  -> rc=0
+    2: 111 GSM-HR-08/8000/1  -> rc=0
+ - add codecs on conn1:
+    0:  96 AMR/8000/1 octet-aligned=1  -> rc=0
+    1:   0 PCMU/8000/1  -> rc=0
+    2:  97 GSM-HR-08/8000/1  -> rc=0
+ - mgcp_codec_pt_translate(conn0, conn1, 112) -> 96
+ - mgcp_codec_pt_translate(conn1, conn0, 96) -> 112
+ - mgcp_codec_pt_translate(conn0, conn1, 0) -> 0
+ - mgcp_codec_pt_translate(conn1, conn0, 0) -> 0
+ - mgcp_codec_pt_translate(conn0, conn1, 111) -> 97
+ - mgcp_codec_pt_translate(conn1, conn0, 97) -> 111
+ - mgcp_codec_pt_translate(conn0, conn1, 123) -> -22
+#1: different order and different payload type numbers
+ - add codecs on conn0:
+    0:   0 PCMU/8000/1  -> rc=0
+    1: 111 GSM-HR-08/8000/1  -> rc=0
+    2: 112 AMR/8000/1 octet-aligned=1  -> rc=0
+ - add codecs on conn1:
+    0:  97 GSM-HR-08/8000/1  -> rc=0
+    1:   0 PCMU/8000/1  -> rc=0
+    2:  96 AMR/8000/1 octet-aligned=1  -> rc=0
+ - mgcp_codec_pt_translate(conn0, conn1, 112) -> 96
+ - mgcp_codec_pt_translate(conn1, conn0, 96) -> 112
+ - mgcp_codec_pt_translate(conn0, conn1, 0) -> 0
+ - mgcp_codec_pt_translate(conn1, conn0, 0) -> 0
+ - mgcp_codec_pt_translate(conn0, conn1, 111) -> 97
+ - mgcp_codec_pt_translate(conn1, conn0, 97) -> 111
+ - mgcp_codec_pt_translate(conn0, conn1, 123) -> -22
+#2: both sides have the same payload_type numbers assigned to differing codecs
+ - add codecs on conn0:
+    0:   0 PCMU/8000/1  -> rc=0
+    1:  96 GSM-HR-08/8000/1  -> rc=0
+    2:  97 AMR/8000/1 octet-aligned=1  -> rc=0
+ - add codecs on conn1:
+    0:  97 GSM-HR-08/8000/1  -> rc=0
+    1:   0 PCMU/8000/1  -> rc=0
+    2:  96 AMR/8000/1 octet-aligned=1  -> rc=0
+ - mgcp_codec_pt_translate(conn0, conn1, 96) -> 97
+ - mgcp_codec_pt_translate(conn1, conn0, 97) -> 96
+ - mgcp_codec_pt_translate(conn0, conn1, 97) -> 96
+ - mgcp_codec_pt_translate(conn1, conn0, 96) -> 97
+ - mgcp_codec_pt_translate(conn0, conn1, 0) -> 0
+ - mgcp_codec_pt_translate(conn1, conn0, 0) -> 0
+#3: conn0 has no codecs
+ - add codecs on conn0:
+    (none)
+ - add codecs on conn1:
+    0:  96 AMR/8000/1 octet-aligned=1  -> rc=0
+    1:   0 PCMU/8000/1  -> rc=0
+    2:  97 GSM-HR-08/8000/1  -> rc=0
+ - mgcp_codec_pt_translate(conn0, conn1, 112) -> -22
+ - mgcp_codec_pt_translate(conn0, conn1, 0) -> -22
+ - mgcp_codec_pt_translate(conn0, conn1, 111) -> -22
+#4: conn1 has no codecs
+ - add codecs on conn0:
+    0: 112 AMR/8000/1 octet-aligned=1  -> rc=0
+    1:   0 PCMU/8000/1  -> rc=0
+    2: 111 GSM-HR-08/8000/1  -> rc=0
+ - add codecs on conn1:
+    (none)
+ - mgcp_codec_pt_translate(conn0, conn1, 112) -> -22
+ - mgcp_codec_pt_translate(conn0, conn1, 0) -> -22
+ - mgcp_codec_pt_translate(conn0, conn1, 111) -> -22
+#5: test AMR with differing octet-aligned settings
+ - add codecs on conn0:
+    0: 111 AMR/8000 octet-aligned=1  -> rc=0
+    1: 112 AMR/8000 octet-aligned=0  -> rc=0
+ - add codecs on conn1:
+    0: 122 AMR/8000 octet-aligned=0  -> rc=0
+    1: 121 AMR/8000 octet-aligned=1  -> rc=0
+ - mgcp_codec_pt_translate(conn0, conn1, 111) -> 121
+ - mgcp_codec_pt_translate(conn1, conn0, 121) -> 111
+ - mgcp_codec_pt_translate(conn0, conn1, 112) -> 122
+ - mgcp_codec_pt_translate(conn1, conn0, 122) -> 112
+#6: test AMR with missing octet-aligned settings (defaults to 0)
+ - add codecs on conn0:
+    0: 111 AMR/8000 octet-aligned=1  -> rc=0
+    1: 112 AMR/8000 octet-aligned=0  -> rc=0
+ - add codecs on conn1:
+    0: 122 AMR/8000 octet-aligned=unset  -> rc=0
+ - mgcp_codec_pt_translate(conn0, conn1, 111) -> -22
+ - mgcp_codec_pt_translate(conn0, conn1, 112) -> 122
+ - mgcp_codec_pt_translate(conn1, conn0, 122) -> 112
+#7: test AMR with NULL param (defaults to 0)
+ - add codecs on conn0:
+    0: 111 AMR/8000 octet-aligned=1  -> rc=0
+    1: 112 AMR/8000 octet-aligned=0  -> rc=0
+ - add codecs on conn1:
+    0: 122 AMR/8000  -> rc=0
+ - mgcp_codec_pt_translate(conn0, conn1, 111) -> -22
+ - mgcp_codec_pt_translate(conn0, conn1, 112) -> 122
+ - mgcp_codec_pt_translate(conn1, conn0, 122) -> 112
+#8: match FOO/8000/1 and FOO/8000 as identical, single channel is implicit
+ - add codecs on conn0:
+    0:   0 PCMU/8000/1  -> rc=0
+    1: 111 GSM-HR-08/8000/1  -> rc=0
+    2: 112 AMR/8000/1 octet-aligned=1  -> rc=0
+ - add codecs on conn1:
+    0:  97 GSM-HR-08/8000  -> rc=0
+    1:   0 PCMU/8000  -> rc=0
+    2:  96 AMR/8000 octet-aligned=1  -> rc=0
+ - mgcp_codec_pt_translate(conn0, conn1, 112) -> 96
+ - mgcp_codec_pt_translate(conn1, conn0, 96) -> 112
+ - mgcp_codec_pt_translate(conn0, conn1, 0) -> 0
+ - mgcp_codec_pt_translate(conn1, conn0, 0) -> 0
+ - mgcp_codec_pt_translate(conn0, conn1, 111) -> 97
+ - mgcp_codec_pt_translate(conn1, conn0, 97) -> 111
+ - mgcp_codec_pt_translate(conn0, conn1, 123) -> -22
+#9: match FOO/8000/1 and FOO as identical, 8k and single channel are implicit
+ - add codecs on conn0:
+    0:   0 PCMU/8000/1  -> rc=0
+    1: 111 GSM-HR-08/8000/1  -> rc=0
+    2: 112 AMR/8000/1 octet-aligned=1  -> rc=0
+ - add codecs on conn1:
+    0:  97 GSM-HR-08  -> rc=0
+    1:   0 PCMU  -> rc=0
+    2:  96 AMR octet-aligned=1  -> rc=0
+ - mgcp_codec_pt_translate(conn0, conn1, 112) -> 96
+ - mgcp_codec_pt_translate(conn1, conn0, 96) -> 112
+ - mgcp_codec_pt_translate(conn0, conn1, 0) -> 0
+ - mgcp_codec_pt_translate(conn1, conn0, 0) -> 0
+ - mgcp_codec_pt_translate(conn0, conn1, 111) -> 97
+ - mgcp_codec_pt_translate(conn1, conn0, 97) -> 111
+ - mgcp_codec_pt_translate(conn0, conn1, 123) -> -22
+#10: test whether channel number matching is waterproof
  - add codecs on conn0:
     0: 111 GSM-HR-08/8000  -> rc=0
     1: 112 GSM-HR-08/8000/2  -> rc=-22

tests/mgcp_client/mgcp_client_test.c

@@ -360,7 +360,7 @@ struct sdp_section_start_test {
 static struct sdp_section_start_test sdp_section_start_tests[] = {
 	{
 		.body = "",
-		.expect_rc = -EINVAL,
+		.expect_rc = 0,
 	},
 	{
 		.body = "\n\n",
@@ -399,19 +399,19 @@ static struct sdp_section_start_test sdp_section_start_tests[] = {
 		.body = "some mgcp header data\r\nand header params"
 			"\n\r\n"
 			"m=audio 23\r\n",
-		.expect_rc = -EINVAL,
+		.expect_rc = 0,
 	},
 	{
 		.body = "some mgcp header data\r\nand header params"
 			"\r\n\r"
 			"m=audio 23\r\n",
-		.expect_rc = -EINVAL,
+		.expect_rc = 0,
 	},
 	{
 		.body = "some mgcp header data\r\nand header params"
 			"\n\r\r"
 			"m=audio 23\r\n",
-		.expect_rc = -EINVAL,
+		.expect_rc = 0,
 	},
 };
 

tests/mgcp_client/mgcp_client_test.err

@@ -17,8 +17,9 @@ test_mgcp_client_cancel() done
 
 test_sdp_section_start() test [0]:
 body: ""
-DLMGCP MGCP response: cannot find start of SDP parameters
-got rc=-22
+DLMGCP MGCP response contains no SDP parameters
+got rc=0
+got audio_port=0
 
 test_sdp_section_start() test [1]:
 body: "\n\n"
@@ -52,18 +53,21 @@ got audio_port=23
 
 test_sdp_section_start() test [7]:
 body: "some mgcp header data\r\nand header params\n\r\nm=audio 23\r\n"
-DLMGCP MGCP response: cannot find start of SDP parameters
-got rc=-22
+DLMGCP MGCP response contains no SDP parameters
+got rc=0
+got audio_port=0
 
 test_sdp_section_start() test [8]:
 body: "some mgcp header data\r\nand header params\r\n\rm=audio 23\r\n"
-DLMGCP MGCP response: cannot find start of SDP parameters
-got rc=-22
+DLMGCP MGCP response contains no SDP parameters
+got rc=0
+got audio_port=0
 
 test_sdp_section_start() test [9]:
 body: "some mgcp header data\r\nand header params\n\r\rm=audio 23\r\n"
-DLMGCP MGCP response: cannot find start of SDP parameters
-got rc=-22
+DLMGCP MGCP response contains no SDP parameters
+got rc=0
+got audio_port=0
 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