This is merely a historical archive of years 2008-2021, before the migration to mailman3.
A maintained and still updated list archive can be found at https://lists.osmocom.org/hyperkitty/list/gerrit-log@lists.osmocom.org/.
dexter gerrit-no-reply at lists.osmocom.orgHello Harald Welte, Jenkins Builder, I'd like you to reexamine a change. Please visit https://gerrit.osmocom.org/4006 to look at the new patch set (#6). sdp: refactoring sdp parser/generator move SDP generator function write_response_sdp() from mgcp_protocol.c to mgcp_sdp.c move prototypes for mgcp_parse_sdp_data() and mgcp_set_audio_info() to mgcp_sdp.h change parameter list of mgcp_parse_sdp_data() so that it takes the rtp conn directly, rather than struct mgcp_rtp_end. fix sourcecode formatting in mgcp_sdp.c add doxygen comments to all public functions Change-Id: I9f88c93872ff913bc211f560b26901267f577324 --- M include/osmocom/mgcp/Makefile.am M include/osmocom/mgcp/mgcp_internal.h A include/osmocom/mgcp/mgcp_sdp.h M src/libosmo-mgcp/mgcp_protocol.c M src/libosmo-mgcp/mgcp_sdp.c 5 files changed, 211 insertions(+), 142 deletions(-) git pull ssh://gerrit.osmocom.org:29418/osmo-mgw refs/changes/06/4006/6 diff --git a/include/osmocom/mgcp/Makefile.am b/include/osmocom/mgcp/Makefile.am index 646b887..cd8f599 100644 --- a/include/osmocom/mgcp/Makefile.am +++ b/include/osmocom/mgcp/Makefile.am @@ -4,4 +4,5 @@ mgcp_conn.h \ mgcp_stat.h \ mgcp_ep.h \ + mgcp_sdp.h \ $(NULL) diff --git a/include/osmocom/mgcp/mgcp_internal.h b/include/osmocom/mgcp/mgcp_internal.h index 3a22d51..777d787 100644 --- a/include/osmocom/mgcp/mgcp_internal.h +++ b/include/osmocom/mgcp/mgcp_internal.h @@ -317,9 +317,6 @@ #define DEFAULT_RTP_AUDIO_DEFAULT_CHANNELS 1 #define PTYPE_UNDEFINED (-1) -int mgcp_parse_sdp_data(struct mgcp_endpoint *endp, struct mgcp_rtp_end *rtp, struct mgcp_parse_data *p); -int mgcp_set_audio_info(void *ctx, struct mgcp_rtp_codec *codec, - int payload_type, const char *audio_name); /*! get the ip-address where the mgw application is bound on. * \param[in] endp mgcp endpoint, that holds a copy of the VTY parameters diff --git a/include/osmocom/mgcp/mgcp_sdp.h b/include/osmocom/mgcp/mgcp_sdp.h new file mode 100644 index 0000000..0eb376d --- /dev/null +++ b/include/osmocom/mgcp/mgcp_sdp.h @@ -0,0 +1,33 @@ +/* + * SDP generation and parsing + * + * (C) 2009-2015 by Holger Hans Peter Freyther <zecke at selfish.org> + * (C) 2009-2014 by On-Waves + * All Rights Reserved + * + * 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/>. + * + */ + +#pragma once + +int mgcp_parse_sdp_data(struct mgcp_endpoint *endp, struct mgcp_conn_rtp *conn, + struct mgcp_parse_data *p); + +int mgcp_set_audio_info(void *ctx, struct mgcp_rtp_codec *codec, + int payload_type, const char *audio_name); + +int mgcp_write_response_sdp(struct mgcp_endpoint *endp, + struct mgcp_conn_rtp *conn, char *sdp_record, + size_t size, const char *addr); diff --git a/src/libosmo-mgcp/mgcp_protocol.c b/src/libosmo-mgcp/mgcp_protocol.c index ac25894..5d4dd47 100644 --- a/src/libosmo-mgcp/mgcp_protocol.c +++ b/src/libosmo-mgcp/mgcp_protocol.c @@ -38,6 +38,7 @@ #include <osmocom/mgcp/mgcp_stat.h> #include <osmocom/mgcp/mgcp_msg.h> #include <osmocom/mgcp/mgcp_ep.h> +#include <osmocom/mgcp/mgcp_sdp.h> struct mgcp_request { char *name; @@ -190,80 +191,6 @@ return create_resp(endp, code, " FAIL", msg, trans, NULL, NULL); } -static int write_response_sdp(struct mgcp_endpoint *endp, - struct mgcp_conn_rtp *conn, - char *sdp_record, size_t size, const char *addr) -{ - const char *fmtp_extra; - const char *audio_name; - int payload_type; - int len; - int nchars; - - if (!conn) - return -1; - - endp->cfg->get_net_downlink_format_cb(endp, &payload_type, - &audio_name, &fmtp_extra, conn); - - len = snprintf(sdp_record, size, - "v=0\r\n" - "o=- %u 23 IN IP4 %s\r\n" - "s=-\r\n" - "c=IN IP4 %s\r\n" - "t=0 0\r\n", conn->conn->id, addr, addr); - - if (len < 0 || len >= size) - goto buffer_too_small; - - if (payload_type >= 0) { - nchars = snprintf(sdp_record + len, size - len, - "m=audio %d RTP/AVP %d\r\n", - conn->end.local_port, payload_type); - if (nchars < 0 || nchars >= size - len) - goto buffer_too_small; - - len += nchars; - - if (audio_name && endp->tcfg->audio_send_name) { - nchars = snprintf(sdp_record + len, size - len, - "a=rtpmap:%d %s\r\n", - payload_type, audio_name); - - if (nchars < 0 || nchars >= size - len) - goto buffer_too_small; - - len += nchars; - } - - if (fmtp_extra) { - nchars = snprintf(sdp_record + len, size - len, - "%s\r\n", fmtp_extra); - - if (nchars < 0 || nchars >= size - len) - goto buffer_too_small; - - len += nchars; - } - } - if (conn->end.packet_duration_ms > 0 && endp->tcfg->audio_send_ptime) { - nchars = snprintf(sdp_record + len, size - len, - "a=ptime:%u\r\n", - conn->end.packet_duration_ms); - if (nchars < 0 || nchars >= size - len) - goto buffer_too_small; - - len += nchars; - } - - return len; - -buffer_too_small: - LOGP(DLMGCP, LOGL_ERROR, "SDP buffer too small: %zu (needed %d)\n", - size, len); - return -1; -} - /* Format MGCP response string (with SDP attached) */ static struct msgb *create_response_with_sdp(struct mgcp_endpoint *endp, struct mgcp_conn_rtp *conn, @@ -291,8 +218,8 @@ if (len < 0) return NULL; - nchars = write_response_sdp(endp, conn, sdp_record + len, - sizeof(sdp_record) - len - 1, addr); + nchars = mgcp_write_response_sdp(endp, conn, sdp_record + len, + sizeof(sdp_record) - len - 1, addr); if (nchars < 0) return NULL; @@ -688,7 +615,7 @@ /* set up RTP media parameters */ if (have_sdp) - mgcp_parse_sdp_data(endp, &conn->end, p); + mgcp_parse_sdp_data(endp, conn, p); else if (endp->local_options.codec) mgcp_set_audio_info(p->cfg, &conn->end.codec, PTYPE_UNDEFINED, endp->local_options.codec); @@ -835,7 +762,7 @@ conn->conn->mode = conn->conn->mode_orig; if (have_sdp) - mgcp_parse_sdp_data(endp, &conn->end, p); + mgcp_parse_sdp_data(endp, conn, p); set_local_cx_options(endp->tcfg->endpoints, &endp->local_options, local_options); diff --git a/src/libosmo-mgcp/mgcp_sdp.c b/src/libosmo-mgcp/mgcp_sdp.c index 7568351..048a533 100644 --- a/src/libosmo-mgcp/mgcp_sdp.c +++ b/src/libosmo-mgcp/mgcp_sdp.c @@ -38,6 +38,12 @@ int channels; }; +/*! set codec configuration depending on payload type and codec name. + * \endp[in] ctx talloc context + * \endp[out] codec configuration (caller provided memory) + * \endp[in] payload_type codec type id (e.g. 3 for GSM, -1 when undefined) + * \endp[in] audio_name audio codec name (e.g. "GSM/8000/1") + * \returns 0 on success, -1 on failure */ int mgcp_set_audio_info(void *ctx, struct mgcp_rtp_codec *codec, int payload_type, const char *audio_name) { @@ -55,15 +61,23 @@ if (!audio_name) { switch (payload_type) { - case 0: audio_name = "PCMU/8000/1"; break; - case 3: audio_name = "GSM/8000/1"; break; - case 8: audio_name = "PCMA/8000/1"; break; - case 18: audio_name = "G729/8000/1"; break; + case 0: + audio_name = "PCMU/8000/1"; + break; + case 3: + audio_name = "GSM/8000/1"; + break; + case 8: + audio_name = "PCMA/8000/1"; + break; + case 18: + audio_name = "G729/8000/1"; + break; default: - /* Payload type is unknown, don't change rate and - * channels. */ - /* TODO: return value? */ - return 0; + /* Payload type is unknown, don't change rate and + * channels. */ + /* TODO: return value? */ + return 0; } } @@ -107,7 +121,7 @@ return 0; } -void codecs_initialize(void *ctx, struct sdp_rtp_map *codecs, int used) +static void codecs_initialize(void *ctx, struct sdp_rtp_map *codecs, int used) { int i; @@ -137,7 +151,8 @@ } } -void codecs_update(void *ctx, struct sdp_rtp_map *codecs, int used, int payload, char *audio_name) +static void codecs_update(void *ctx, struct sdp_rtp_map *codecs, int used, + int payload, char *audio_name) { int i; @@ -148,8 +163,9 @@ if (codecs[i].payload_type != payload) continue; if (sscanf(audio_name, "%63[^/]/%d/%d", - audio_codec, &rate, &channels) < 1) { - LOGP(DLMGCP, LOGL_ERROR, "Failed to parse '%s'\n", audio_name); + audio_codec, &rate, &channels) < 1) { + LOGP(DLMGCP, LOGL_ERROR, "Failed to parse '%s'\n", + audio_name); continue; } @@ -160,29 +176,36 @@ return; } - LOGP(DLMGCP, LOGL_ERROR, "Unconfigured PT(%d) with %s\n", payload, audio_name); + LOGP(DLMGCP, LOGL_ERROR, "Unconfigured PT(%d) with %s\n", payload, + audio_name); } -int is_codec_compatible(struct mgcp_endpoint *endp, struct sdp_rtp_map *codec) +/* Check if the codec matches what is set up in the trunk config */ +static int is_codec_compatible(struct mgcp_endpoint *endp, + struct sdp_rtp_map *codec) { - char *bts_codec; + char *codec_str; char audio_codec[64]; if (!codec->codec_name) return 0; - /* - * GSM, GSM/8000 and GSM/8000/1 should all be compatible.. let's go - * by name first. - */ - bts_codec = endp->tcfg->audio_name; - if (sscanf(bts_codec, "%63[^/]/%*d/%*d", audio_codec) < 1) + /* GSM, GSM/8000 and GSM/8000/1 should all be compatible... + * let's go by name first. */ + codec_str = endp->tcfg->audio_name; + if (sscanf(codec_str, "%63[^/]/%*d/%*d", audio_codec) < 1) return 0; return strcasecmp(audio_codec, codec->codec_name) == 0; } -int mgcp_parse_sdp_data(struct mgcp_endpoint *endp, struct mgcp_rtp_end *rtp, struct mgcp_parse_data *p) +/*! analyze SDP input string. + * \endp[in] endp trunk endpoint + * \endp[in] conn associated rtp connection + * \endp[in] caller provided memory to store the parsing results + * \returns 0 on success, -1 on failure */ +int mgcp_parse_sdp_data(struct mgcp_endpoint *endp, struct mgcp_conn_rtp *conn, + struct mgcp_parse_data *p) { struct sdp_rtp_map codecs[10]; int codecs_used = 0; @@ -191,7 +214,19 @@ int i; int codecs_assigned = 0; void *tmp_ctx = talloc_new(NULL); + struct mgcp_rtp_end *rtp; + int payload; + int ptime, ptime2 = 0; + char audio_name[64]; + int port, rc; + char ipv4[16]; + + OSMO_ASSERT(endp); + OSMO_ASSERT(conn); + OSMO_ASSERT(p); + + rtp = &conn->end; memset(&codecs, 0, sizeof(codecs)); for_each_line(line, p->save) { @@ -202,41 +237,36 @@ case 'v': /* skip these SDP attributes */ break; - case 'a': { - int payload; - int ptime, ptime2 = 0; - char audio_name[64]; - - + case 'a': if (sscanf(line, "a=rtpmap:%d %63s", &payload, audio_name) == 2) { - codecs_update(tmp_ctx, codecs, codecs_used, payload, audio_name); - } else if (sscanf(line, "a=ptime:%d-%d", - &ptime, &ptime2) >= 1) { + codecs_update(tmp_ctx, codecs, + codecs_used, payload, audio_name); + } else + if (sscanf + (line, "a=ptime:%d-%d", &ptime, &ptime2) >= 1) { if (ptime2 > 0 && ptime2 != ptime) rtp->packet_duration_ms = 0; else rtp->packet_duration_ms = ptime; - } else if (sscanf(line, "a=maxptime:%d", &ptime2) == 1) { + } else if (sscanf(line, "a=maxptime:%d", &ptime2) + == 1) { maxptime = ptime2; } break; - } - case 'm': { - int port, rc; - - rc = sscanf(line, "m=audio %d RTP/AVP %d %d %d %d %d %d %d %d %d %d", - &port, - &codecs[0].payload_type, - &codecs[1].payload_type, - &codecs[2].payload_type, - &codecs[3].payload_type, - &codecs[4].payload_type, - &codecs[5].payload_type, - &codecs[6].payload_type, - &codecs[7].payload_type, - &codecs[8].payload_type, - &codecs[9].payload_type); + case 'm': + rc = sscanf(line, + "m=audio %d RTP/AVP %d %d %d %d %d %d %d %d %d %d", + &port, &codecs[0].payload_type, + &codecs[1].payload_type, + &codecs[2].payload_type, + &codecs[3].payload_type, + &codecs[4].payload_type, + &codecs[5].payload_type, + &codecs[6].payload_type, + &codecs[7].payload_type, + &codecs[8].payload_type, + &codecs[9].payload_type); if (rc >= 2) { rtp->rtp_port = htons(port); rtp->rtcp_port = htons(port + 1); @@ -244,20 +274,18 @@ codecs_initialize(tmp_ctx, codecs, codecs_used); } break; - } - case 'c': { - char ipv4[16]; + case 'c': if (sscanf(line, "c=IN IP4 %15s", ipv4) == 1) { inet_aton(ipv4, &rtp->addr); } break; - } default: if (p->endp) LOGP(DLMGCP, LOGL_NOTICE, "Unhandled SDP option: '%c'/%d on 0x%x\n", - line[0], line[0], ENDPOINT_NUMBER(p->endp)); + line[0], line[0], + ENDPOINT_NUMBER(p->endp)); else LOGP(DLMGCP, LOGL_NOTICE, "Unhandled SDP option: '%c'/%d\n", @@ -269,25 +297,24 @@ /* Now select the primary and alt_codec */ for (i = 0; i < codecs_used && codecs_assigned < 2; ++i) { struct mgcp_rtp_codec *codec = codecs_assigned == 0 ? - &rtp->codec : &rtp->alt_codec; + &rtp->codec : &rtp->alt_codec; if (endp->tcfg->no_audio_transcoding && - !is_codec_compatible(endp, &codecs[i])) { + !is_codec_compatible(endp, &codecs[i])) { LOGP(DLMGCP, LOGL_NOTICE, "Skipping codec %s\n", - codecs[i].codec_name); + codecs[i].codec_name); continue; } mgcp_set_audio_info(p->cfg, codec, - codecs[i].payload_type, - codecs[i].map_line); + codecs[i].payload_type, codecs[i].map_line); codecs_assigned += 1; } if (codecs_assigned > 0) { /* TODO/XXX: Store this per codec and derive it on use */ if (maxptime >= 0 && maxptime * rtp->codec.frame_duration_den > - rtp->codec.frame_duration_num * 1500) { + rtp->codec.frame_duration_num * 1500) { /* more than 1 frame */ rtp->packet_duration_ms = 0; } @@ -296,11 +323,95 @@ "Got media info via SDP: port %d, payload %d (%s), " "duration %d, addr %s\n", ntohs(rtp->rtp_port), rtp->codec.payload_type, - rtp->codec.subtype_name ? rtp->codec.subtype_name : "unknown", - rtp->packet_duration_ms, inet_ntoa(rtp->addr)); + rtp->codec.subtype_name ? rtp-> + codec.subtype_name : "unknown", rtp->packet_duration_ms, + inet_ntoa(rtp->addr)); } talloc_free(tmp_ctx); return codecs_assigned > 0; } +/*! generate SDP response string. + * \endp[in] endp trunk endpoint + * \endp[in] conn associated rtp connection + * \endp[out] sdp_record resulting SDP string + * \endp[in] size buffer size of sdp_record + * \endp[in] addr IPV4 address string (e.g. 192.168.100.1) + * \returns 0 on success, -1 on failure */ +int mgcp_write_response_sdp(struct mgcp_endpoint *endp, + struct mgcp_conn_rtp *conn, char *sdp_record, + size_t size, const char *addr) +{ + const char *fmtp_extra; + const char *audio_name; + int payload_type; + int len; + int nchars; + + OSMO_ASSERT(endp); + OSMO_ASSERT(conn); + OSMO_ASSERT(sdp_record); + OSMO_ASSERT(size > 0); + OSMO_ASSERT(addr); + + endp->cfg->get_net_downlink_format_cb(endp, &payload_type, + &audio_name, &fmtp_extra, conn); + + len = snprintf(sdp_record, size, + "v=0\r\n" + "o=- %u 23 IN IP4 %s\r\n" + "s=-\r\n" + "c=IN IP4 %s\r\n" + "t=0 0\r\n", conn->conn->id, addr, addr); + + if (len < 0 || len >= size) + goto buffer_too_small; + + if (payload_type >= 0) { + nchars = snprintf(sdp_record + len, size - len, + "m=audio %d RTP/AVP %d\r\n", + conn->end.local_port, payload_type); + if (nchars < 0 || nchars >= size - len) + goto buffer_too_small; + + len += nchars; + + if (audio_name && endp->tcfg->audio_send_name) { + nchars = snprintf(sdp_record + len, size - len, + "a=rtpmap:%d %s\r\n", + payload_type, audio_name); + + if (nchars < 0 || nchars >= size - len) + goto buffer_too_small; + + len += nchars; + } + + if (fmtp_extra) { + nchars = snprintf(sdp_record + len, size - len, + "%s\r\n", fmtp_extra); + + if (nchars < 0 || nchars >= size - len) + goto buffer_too_small; + + len += nchars; + } + } + if (conn->end.packet_duration_ms > 0 && endp->tcfg->audio_send_ptime) { + nchars = snprintf(sdp_record + len, size - len, + "a=ptime:%u\r\n", + conn->end.packet_duration_ms); + if (nchars < 0 || nchars >= size - len) + goto buffer_too_small; + + len += nchars; + } + + return len; + +buffer_too_small: + LOGP(DLMGCP, LOGL_ERROR, "SDP buffer too small: %zu (needed %d)\n", + size, len); + return -1; +} -- To view, visit https://gerrit.osmocom.org/4006 To unsubscribe, visit https://gerrit.osmocom.org/settings Gerrit-MessageType: newpatchset Gerrit-Change-Id: I9f88c93872ff913bc211f560b26901267f577324 Gerrit-PatchSet: 6 Gerrit-Project: osmo-mgw Gerrit-Branch: master Gerrit-Owner: Neels Hofmeyr <nhofmeyr at sysmocom.de> Gerrit-Reviewer: Harald Welte <laforge at gnumonks.org> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: dexter <pmaier at sysmocom.de>