dexter has uploaded this change for review. (
https://gerrit.osmocom.org/c/osmo-mgw/+/18645 )
Change subject: mgcp_trunk: remove audio_name and audio_payloa
......................................................................
mgcp_trunk: remove audio_name and audio_payloa
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
---
M include/osmocom/mgcp/mgcp_trunk.h
M src/libosmo-mgcp/mgcp_codec.c
M src/libosmo-mgcp/mgcp_vty.c
M tests/mgcp/mgcp_test.c
4 files changed, 9 insertions(+), 53 deletions(-)
git pull ssh://gerrit.osmocom.org:29418/osmo-mgw refs/changes/45/18645/1
diff --git a/include/osmocom/mgcp/mgcp_trunk.h
b/include/osmocom/mgcp/mgcp_trunk.h
index 71d89ac..386e540 100644
--- a/include/osmocom/mgcp/mgcp_trunk.h
+++ b/include/osmocom/mgcp/mgcp_trunk.h
@@ -9,8 +9,6 @@
int trunk_type;
char *audio_fmtp_extra;
- char *audio_name;
- int audio_payload;
int audio_send_ptime;
int audio_send_name;
int audio_loop;
diff --git a/src/libosmo-mgcp/mgcp_codec.c b/src/libosmo-mgcp/mgcp_codec.c
index c251317..9ac5fbb 100644
--- a/src/libosmo-mgcp/mgcp_codec.c
+++ b/src/libosmo-mgcp/mgcp_codec.c
@@ -280,36 +280,16 @@
* 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"
- * from the audio name that is stored in the trunk configuration.
- * 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;
+ /* FIXME: implement meaningful checks to make sure that the given codec
+ * is compatible with the given endpoint */
- /* Finally we check if the subtype_name we have generated from the
- * 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;
+ return true;
}
/*! Decide for one suitable codec
diff --git a/src/libosmo-mgcp/mgcp_vty.c b/src/libosmo-mgcp/mgcp_vty.c
index d332d75..f10f326 100644
--- a/src/libosmo-mgcp/mgcp_vty.c
+++ b/src/libosmo-mgcp/mgcp_vty.c
@@ -114,12 +114,6 @@
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);
@@ -600,13 +594,11 @@
#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;
}
@@ -615,12 +607,11 @@
"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;
}
@@ -845,10 +836,6 @@
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",
@@ -909,15 +896,11 @@
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;
}
@@ -925,14 +908,11 @@
"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;
}
diff --git a/tests/mgcp/mgcp_test.c b/tests/mgcp/mgcp_test.c
index ed0fda0..d0da18b 100644
--- a/tests/mgcp/mgcp_test.c
+++ b/tests/mgcp/mgcp_test.c
@@ -1458,11 +1458,9 @@
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 @@
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;
--
To view, visit https://gerrit.osmocom.org/c/osmo-mgw/+/18645
To unsubscribe, or for help writing mail filters, visit
https://gerrit.osmocom.org/settings
Gerrit-Project: osmo-mgw
Gerrit-Branch: master
Gerrit-Change-Id: Ia050ec3cd34b410dfe089c41b977ae3d5aed7354
Gerrit-Change-Number: 18645
Gerrit-PatchSet: 1
Gerrit-Owner: dexter <pma...@sysmocom.de>
Gerrit-MessageType: newchange
