mgcp_trunk: remove audio_name and audio_payload

get rid of deprecated trunk parameters which seem to be leftovers
from the old osmo-bsc_mgcp implementation. This is in particular
audio_name and audio_payload in struct mgcp_trunk_config which
allowed the user to "hardcode" an andio name and payload type
via VTY configuration

The removal of the struct members above also require a change to
mgcp_codec.c. The code that is is never actively used and even
causes wrong behavior when activated (set the no-transcoding
flag in VTY). Since the code is removed also the unit tests
also require to be changed to match the new behavior.

Change-Id: Ia050ec3cd34b410dfe089c41b977ae3d5aed7354
Related: OS#2659

include/osmocom/mgcp/mgcp_trunk.h

@@ -9,8 +9,6 @@ struct mgcp_trunk {
 	int trunk_type;
 
 	char *audio_fmtp_extra;
-	char *audio_name;
-	int audio_payload;
 	int audio_send_ptime;
 	int audio_send_name;
 	int audio_loop;

src/libosmo-mgcp/mgcp_codec.c

@@ -280,36 +280,16 @@ error:
  * Helper function for mgcp_codec_decide() */
 static bool is_codec_compatible(const struct mgcp_endpoint *endp, const struct mgcp_rtp_codec *codec)
 {
-	char codec_name[64];
-
 	/* A codec name must be set, if not, this might mean that the codec
 	 * (payload type) that was assigned is unknown to us so we must stop
 	 * here. */
 	if (!codec->subtype_name)
 		return false;
 
-	/* We now extract the codec_name (letters before the /, e.g. "GSM"
+	/* FIXME: implement meaningful checks to make sure that the given codec
-	 * from the audio name that is stored in the trunk configuration.
+	 * is compatible with the given endpoint */
-	 * We do not compare to the full audio_name because we expect that
-	 * "GSM", "GSM/8000" and "GSM/8000/1" are all compatible when the
-	 * audio name of the codec is set to "GSM" */
-	if (sscanf(endp->trunk->audio_name, "%63[^/]/%*d/%*d", codec_name) < 1)
-		return false;
 
-	/* Finally we check if the subtype_name we have generated from the
+	return true;
-	 * audio_name in the trunc struct patches the codec_name of the
-	 * given codec */
-	if (strcasecmp(codec_name, codec->subtype_name) == 0)
-		return true;
-
-	/* FIXME: It is questinable that the method to pick a compatible
-	 * codec can work properly. Since this useses trunk->audio_name, as
-	 * a reference, which is set to "AMR/8000" permanently.
-	 * trunk->audio_name must be updated by the first connection that
-	 * has been made on an endpoint, so that the second connection
-	 * can make a meaningful decision here */
-
-	return false;
 }
 
 /*! Decide for one suitable codec

src/libosmo-mgcp/mgcp_vty.c

@@ -114,12 +114,6 @@ static int config_write_mgcp(struct vty *vty)
 			VTY_NEWLINE);
 	} else
 		vty_out(vty, " no rtp-patch%s", VTY_NEWLINE);
-	if (trunk->audio_payload != -1)
-		vty_out(vty, " sdp audio-payload number %d%s",
-			trunk->audio_payload, VTY_NEWLINE);
-	if (trunk->audio_name)
-		vty_out(vty, " sdp audio-payload name %s%s",
-			trunk->audio_name, VTY_NEWLINE);
 	if (trunk->audio_fmtp_extra)
 		vty_out(vty, " sdp audio fmtp-extra %s%s",
 			trunk->audio_fmtp_extra, VTY_NEWLINE);
@@ -611,13 +605,11 @@ DEFUN(cfg_mgcp_no_allow_transcoding,
 
 #define SDP_STR "SDP File related options\n"
 #define AUDIO_STR "Audio payload options\n"
-DEFUN(cfg_mgcp_sdp_payload_number,
+DEFUN_DEPRECATED(cfg_mgcp_sdp_payload_number,
       cfg_mgcp_sdp_payload_number_cmd,
       "sdp audio-payload number <0-255>",
       SDP_STR AUDIO_STR "Number\n" "Payload number\n")
 {
-	unsigned int payload = atoi(argv[0]);
-	g_cfg->virt_trunk->audio_payload = payload;
 	return CMD_SUCCESS;
 }
 
@@ -626,12 +618,11 @@ ALIAS_DEPRECATED(cfg_mgcp_sdp_payload_number,
 		 "sdp audio payload number <0-255>",
 		 SDP_STR AUDIO_STR AUDIO_STR "Number\n" "Payload number\n")
 
-DEFUN(cfg_mgcp_sdp_payload_name,
+DEFUN_DEPRECATED(cfg_mgcp_sdp_payload_name,
       cfg_mgcp_sdp_payload_name_cmd,
       "sdp audio-payload name NAME",
       SDP_STR AUDIO_STR "Name\n" "Payload name\n")
 {
-	osmo_talloc_replace_string(g_cfg, &g_cfg->virt_trunk->audio_name, argv[0]);
 	return CMD_SUCCESS;
 }
 
@@ -856,10 +847,6 @@ static int config_write_trunk(struct vty *vty)
 
 	llist_for_each_entry(trunk, &g_cfg->trunks, entry) {
 		vty_out(vty, " trunk %d%s", trunk->trunk_nr, VTY_NEWLINE);
-		vty_out(vty, "  sdp audio-payload number %d%s",
-			trunk->audio_payload, VTY_NEWLINE);
-		vty_out(vty, "  sdp audio-payload name %s%s",
-			trunk->audio_name, 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",
@@ -920,15 +907,11 @@ DEFUN(cfg_trunk_sdp_fmtp_extra,
 	return CMD_SUCCESS;
 }
 
-DEFUN(cfg_trunk_payload_number,
+DEFUN_DEPRECATED(cfg_trunk_payload_number,
       cfg_trunk_payload_number_cmd,
       "sdp audio-payload number <0-255>",
       SDP_STR AUDIO_STR "Number\n" "Payload Number\n")
 {
-	struct mgcp_trunk *trunk = vty->index;
-	unsigned int payload = atoi(argv[0]);
-
-	trunk->audio_payload = payload;
 	return CMD_SUCCESS;
 }
 
@@ -936,14 +919,11 @@ ALIAS_DEPRECATED(cfg_trunk_payload_number, cfg_trunk_payload_number_cmd_old,
 		 "sdp audio payload number <0-255>",
 		 SDP_STR AUDIO_STR AUDIO_STR "Number\n" "Payload Number\n")
 
-DEFUN(cfg_trunk_payload_name,
+DEFUN_DEPRECATED(cfg_trunk_payload_name,
       cfg_trunk_payload_name_cmd,
       "sdp audio-payload name NAME",
       SDP_STR AUDIO_STR "Payload\n" "Payload Name\n")
 {
-	struct mgcp_trunk *trunk = vty->index;
-
-	osmo_talloc_replace_string(g_cfg, &trunk->audio_name, argv[0]);
 	return CMD_SUCCESS;
 }
 

tests/mgcp/mgcp_test.c

@@ -1458,11 +1458,9 @@ static void test_multilple_codec(void)
 	OSMO_ASSERT(conn);
 	OSMO_ASSERT(conn->end.codec->payload_type == 18);
 
-	/* Allocate 5@mgw at select GSM.. */
+	/* Allocate 5@mgw and let osmo-mgw pick a codec from the list */
 	last_endpoint = -1;
 	inp = create_msg(CRCX_MULT_GSM_EXACT, NULL);
-	talloc_free(cfg->virt_trunk->audio_name);
-	cfg->virt_trunk->audio_name = "GSM/8000";
 	cfg->virt_trunk->no_audio_transcoding = 1;
 	resp = mgcp_handle_message(cfg, inp);
 	OSMO_ASSERT(get_conn_id_from_response(resp->data, conn_id,
@@ -1474,7 +1472,7 @@ static void test_multilple_codec(void)
 	endp = cfg->virt_trunk->endpoints[last_endpoint];
 	conn = mgcp_conn_get_rtp(endp, conn_id);
 	OSMO_ASSERT(conn);
-	OSMO_ASSERT(conn->end.codec->payload_type == 3);
+	OSMO_ASSERT(conn->end.codec->payload_type == 0);
 
 	inp = create_msg(MDCX_NAT_DUMMY, conn_id);
 	last_endpoint = -1;