cellular-infrastructure
/
osmo-mgw
mirror of https://gerrit.osmocom.org/osmo-mgw
10
0
Fork 0
Code Issues Releases Wiki Activity

drop cfg 'sdp audio fmtp-extra' 

There is considerable code complexity in place for this ancient hack.

It dates back to 5ea1bc77a3
"
mgcp: Allow to freely control the a=fmtp line for experiments

In case of AMR one can specify the available codecs out-of-band. Allow
to configure this line statically in the configuration file.
"

Looking in mgcp_test.c output, the fmtp-extra tests do not even make
sense: they result in fmtp for pt=126 being added, even though there is
no payload type 126 listed in the SDP...

Related: OS#6313
Change-Id: Icee0cd1f5a751fa760d5a9deca29089e78e7eb93
This commit is contained in:
Neels Hofmeyr 2023-12-23 03:54:58 +01:00
parent 35a738214c
commit d0dbda4106
8 changed files with 19 additions and 87 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

1
TODO-RELEASE
View File

@ -38,3 +38,4 @@ libosmo-mgcp-client remove public API These public API items have not been calle
libosmo-mgcp-client deprecate public API New code should no longer use codecs[], instead use ptmap[].codec. There
is backwards compat code that moves codecs[] entries, if any, over to
ptmap[], so callers may migrate at own leisure.
osmo-mgw remove cfg Remove VTY config item 'sdp audio fmtp-extra' (see OS#6313)

1
include/osmocom/mgcp/mgcp_network.h
View File

@ -108,7 +108,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! */

1
include/osmocom/mgcp/mgcp_trunk.h
View File

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

2
src/libosmo-mgcp/mgcp_conn.c
View File

@ -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 */

3
src/libosmo-mgcp/mgcp_protocol.c
View File

@ -1076,9 +1076,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;

37
src/libosmo-mgcp/mgcp_sdp.c
View File

@ -488,34 +488,10 @@ static int add_audio(struct msgb *sdp, int *payload_types, unsigned int payload_
}
/* 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)
static int add_fmtp(struct msgb *sdp, struct sdp_fmtp_param *fmtp_params, unsigned int fmtp_params_len)
{
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);
@ -532,13 +508,6 @@ static int add_fmtp(struct msgb *sdp, struct sdp_fmtp_param *fmtp_params, unsign
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)
return -EINVAL;
}
rc = msgb_printf(sdp, "\r\n");
if (rc < 0)
return -EINVAL;
@ -558,7 +527,6 @@ int mgcp_write_response_sdp(const struct mgcp_endpoint *endp,
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;
@ -575,7 +543,6 @@ int mgcp_write_response_sdp(const struct mgcp_endpoint *endp,
OSMO_ASSERT(addr);
codec = conn->end.codec;
fmtp_extra = conn->end.fmtp_extra;
audio_name = codec->audio_name;
payload_type = codec->payload_type;
@ -617,7 +584,7 @@ int mgcp_write_response_sdp(const struct mgcp_endpoint *endp,
fmtp_params[0] = fmtp_param;
fmtp_params_len = 1;
}
rc = add_fmtp(sdp, fmtp_params, fmtp_params_len, fmtp_extra);
rc = add_fmtp(sdp, fmtp_params, fmtp_params_len);
if (rc < 0)
goto buffer_too_small;
}

48
src/libosmo-mgcp/mgcp_vty.c
View File

@ -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;
}

13
tests/mgcp/mgcp_test.c
View File

@ -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 \
@ -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},
@ -831,9 +827,6 @@ static void test_messages(void)
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);