October 2023 - gerrit-log - lists.osmocom.org

[L] Change in osmo-mgw[master]: add fmtp string to ptmap: allow all possible fmtp
by neels 26 Oct '23

26 Oct '23

neels has uploaded this change for review. ( https://gerrit.osmocom.org/c/osmo-mgw/+/34900?usp=email ) Change subject: add fmtp string to ptmap: allow all possible fmtp ...................................................................... add fmtp string to ptmap: allow all possible fmtp Remove the limit of having only one AMR octet-aligned fmtp parameter per MGCP message. Instead allow any arbitrary fmtp options, one per every codec. Deprecate all use of struct mgcp_codec_param. Instead, store and pass plain fmtp strings. We need to know fmtp details only for AMR, and only to probe whether it is octet-aligned. So add a separate fmtp string parser that returns that information flexibly, as in if (osmo_mgcp_fmtp_get_int("octet-aligned", 0) == 1) ... Provide legacy shims that still act correctly for any callers that may pass the old struct mgcp_codec_param. (I'm not sure if we need to keep this, but we can always drop it in another patch.) Adjust one mgcp_test.c: instead of returning only the octet-aligned parameter, now osmo-mgw keeps and returns all the fmtp parameters that the user provided. So add the missing "mode-change-capability". Related: OS#6171 Change-Id: If58590bda8627519ff07e0b6f43aa47a274f052b --- M include/osmocom/mgcp/Makefile.am A include/osmocom/mgcp/fmtp.h M include/osmocom/mgcp/mgcp_codec.h M include/osmocom/mgcp/mgcp_network.h M include/osmocom/mgcp_client/mgcp_client.h M src/libosmo-mgcp-client/mgcp_client.c M src/libosmo-mgcp/Makefile.am A src/libosmo-mgcp/fmtp.c M src/libosmo-mgcp/mgcp_codec.c M src/libosmo-mgcp/mgcp_network.c M src/libosmo-mgcp/mgcp_protocol.c M src/libosmo-mgcp/mgcp_sdp.c M tests/mgcp/mgcp_test.c 13 files changed, 264 insertions(+), 93 deletions(-) git pull ssh://gerrit.osmocom.org:29418/osmo-mgw refs/changes/00/34900/1 diff --git a/include/osmocom/mgcp/Makefile.am b/include/osmocom/mgcp/Makefile.am index 15ff01a..5d77c09 100644 --- a/include/osmocom/mgcp/Makefile.am +++ b/include/osmocom/mgcp/Makefile.am @@ -13,4 +13,5 @@ mgcp_network.h \ mgcp_protocol.h \ mgcp_iuup.h \ + fmtp.h \ $(NULL) diff --git a/include/osmocom/mgcp/fmtp.h b/include/osmocom/mgcp/fmtp.h new file mode 100644 index 0000000..95a7504 --- /dev/null +++ b/include/osmocom/mgcp/fmtp.h @@ -0,0 +1,11 @@ +#pragma once + +#include <stddef.h> +#include <stdbool.h> + +int osmo_mgcp_fmtp_get_val(char *val, size_t val_size, const char *fmtp, const char *option_name); +int osmo_mgcp_fmtp_get_int(const char *fmtp, const char *option_name, int default_value); +bool osmo_mgcp_fmtp_is_octet_aligned(const char *fmtp); + +#define OSMO_MGCP_OCTET_ALIGNED_STR "octet-aligned" +#define OSMO_MGCP_OCTET_ALIGNED_EQUALS_STR(VAL) OSMO_MGCP_OCTET_ALIGNED_STR "=" VAL diff --git a/include/osmocom/mgcp/mgcp_codec.h b/include/osmocom/mgcp/mgcp_codec.h index cfc8ecf..4702f6d 100644 --- a/include/osmocom/mgcp/mgcp_codec.h +++ b/include/osmocom/mgcp/mgcp_codec.h @@ -13,6 +13,7 @@ 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, const struct mgcp_codec_param *param); +int mgcp_codec_add2(struct mgcp_conn_rtp *conn, int payload_type, const char *audio_name, const char *fmtp); int mgcp_codec_decide(struct mgcp_conn_rtp *conn_src, struct mgcp_conn_rtp *conn_dst); const struct mgcp_rtp_codec *mgcp_codec_pt_find_by_subtype_name(struct mgcp_conn_rtp *conn, const char *subtype_name, unsigned int match_nr); diff --git a/include/osmocom/mgcp/mgcp_network.h b/include/osmocom/mgcp/mgcp_network.h index e95907d..7f87224 100644 --- a/include/osmocom/mgcp/mgcp_network.h +++ b/include/osmocom/mgcp/mgcp_network.h @@ -83,8 +83,11 @@ char audio_name[64]; char subtype_name[64]; + /* Deprecated. Use the new fmtp string instead. */ bool param_present; struct mgcp_codec_param param; + + char fmtp[256]; }; /* 'mgcp_rtp_end': basically a wrapper around the RTP+RTCP ports */ diff --git a/include/osmocom/mgcp_client/mgcp_client.h b/include/osmocom/mgcp_client/mgcp_client.h index 31f4ae7..c238dc2 100644 --- a/include/osmocom/mgcp_client/mgcp_client.h +++ b/include/osmocom/mgcp_client/mgcp_client.h @@ -77,6 +77,11 @@ /*! payload type number (96-127) */ unsigned int pt; + + /*! the MGCP 'a=fmtp:N <...>' string, e.g. "mode-set=1,2,3;octet-align=0". + * Suggested storage: talloc allocated string with the struct mgcp_conn_peer or struct mgcp_response as talloc + * ctx. */ + const char *fmtp; }; struct mgcp_response_head { @@ -158,7 +163,8 @@ uint8_t rate, uint8_t offset); /* Invoked when an MGCP response is received or sending failed. When the - * response is passed as NULL, this indicates failure during transmission. */ + * response is passed as NULL, this indicates failure during transmission. + * Not all pointers contained in the mgcp_response */ typedef void (* mgcp_response_cb_t )(struct mgcp_response *response, void *priv); int mgcp_response_parse_params(struct mgcp_response *r); diff --git a/src/libosmo-mgcp-client/mgcp_client.c b/src/libosmo-mgcp-client/mgcp_client.c index a66b499..a1a2662 100644 --- a/src/libosmo-mgcp-client/mgcp_client.c +++ b/src/libosmo-mgcp-client/mgcp_client.c @@ -1369,19 +1369,27 @@ } MSGB_PRINTF_OR_RET("\r\n"); - /* Add optional codec parameters (fmtp) */ - if (mgcp_msg->param_present) { - for (i = 0; i < mgcp_msg->ptmap_len; i++) { - /* The following is only applicable for AMR */ - if (mgcp_msg->ptmap[i].codec != CODEC_AMR_8000_1 - && mgcp_msg->ptmap[i].codec != CODEC_AMRWB_16000_1) - continue; - pt = mgcp_msg->ptmap[i].pt; - if (mgcp_msg->param.amr_octet_aligned_present && mgcp_msg->param.amr_octet_aligned) - MSGB_PRINTF_OR_RET("a=fmtp:%u octet-align=1\r\n", pt); - else if (mgcp_msg->param.amr_octet_aligned_present && !mgcp_msg->param.amr_octet_aligned) - MSGB_PRINTF_OR_RET("a=fmtp:%u octet-align=0\r\n", pt); + for (i = 0; i < mgcp_msg->ptmap_len; i++) { + if (!mgcp_msg->ptmap[i].fmtp) { + + /* LEGACY COMPAT. Fill in fmtp with the legacy mgcp_msg->param, if any, when no individual fmtp + * is set on the codec. */ + if (mgcp_msg->param_present + && (mgcp_msg->ptmap[i].codec == CODEC_AMR_8000_1 + || mgcp_msg->ptmap[i].codec != CODEC_AMRWB_16000_1)) { + pt = mgcp_msg->ptmap[i].pt; + + if (mgcp_msg->param.amr_octet_aligned_present && mgcp_msg->param.amr_octet_aligned) + MSGB_PRINTF_OR_RET("a=fmtp:%u octet-align=1\r\n", pt); + else if (mgcp_msg->param.amr_octet_aligned_present && !mgcp_msg->param.amr_octet_aligned) + MSGB_PRINTF_OR_RET("a=fmtp:%u octet-align=0\r\n", pt); + + } + + continue; } + + MSGB_PRINTF_OR_RET("a=fmtp:%u %s\r\n", mgcp_msg->ptmap[i].pt, mgcp_msg->ptmap[i].fmtp); } for (i = 0; i < mgcp_msg->ptmap_len; i++) { diff --git a/src/libosmo-mgcp/Makefile.am b/src/libosmo-mgcp/Makefile.am index fa9a8eb..df7ed1d 100644 --- a/src/libosmo-mgcp/Makefile.am +++ b/src/libosmo-mgcp/Makefile.am @@ -42,4 +42,5 @@ mgcp_ratectr.c \ mgcp_e1.c \ mgcp_iuup.c \ + fmtp.c \ $(NULL) diff --git a/src/libosmo-mgcp/fmtp.c b/src/libosmo-mgcp/fmtp.c new file mode 100644 index 0000000..6899d01 --- /dev/null +++ b/src/libosmo-mgcp/fmtp.c @@ -0,0 +1,89 @@ +/* + * (C) 2023-2015 by sysmocom - s.f.m.c. GmbH <info(a)sysmocom.de> + * All Rights Reserved + * + * Author: Neels Hofmeyr + * + * 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/>. + * + */ +#include <string.h> +#include <ctype.h> +#include <osmocom/core/utils.h> +#include <osmocom/core/logging.h> +#include <osmocom/mgcp/fmtp.h> + +static const char *fmtp_next_option(const char *fmtp) +{ + for (; fmtp && *fmtp && *fmtp != ';'; fmtp++); + for (; fmtp && isspace(*fmtp); fmtp++); + return fmtp; +} + +int osmo_mgcp_fmtp_get_val(char *val, size_t val_size, const char *fmtp, const char *option_name) +{ + const char *pos = fmtp; + const char *end; + int option_name_len = strlen(option_name); + for (; pos && *pos; pos = fmtp_next_option(pos)) { + if (!osmo_str_startswith(pos, option_name)) + continue; + pos += option_name_len; + if (*pos != '=') + continue; + pos++; + break; + } + + if (!pos || !*pos) + return 0; + + end = fmtp_next_option(pos); + OSMO_ASSERT(end); + if (val && val_size) + osmo_strlcpy(val, pos, OSMO_MIN(val_size, end - pos + 1)); + return end - pos; +} + +int osmo_mgcp_fmtp_get_int(const char *fmtp, const char *option_name, int default_value) +{ + char val[128]; + if (!osmo_mgcp_fmtp_get_val(val, sizeof(val), fmtp, option_name)) + return default_value; + if (!val[0]) + return default_value; + int i; + if (osmo_str_to_int(&i, val, 10, 0, 1)) { + /* error parsing number */ + LOGP(DLMGCP, LOGL_ERROR, "Invalid number in fmtp parameter '%s': '%s'\n", + option_name, val); + return default_value; + } + return i; +} + +/* 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 + */ +bool osmo_mgcp_fmtp_is_octet_aligned(const char *fmtp) +{ + return osmo_mgcp_fmtp_get_int(fmtp, OSMO_MGCP_OCTET_ALIGNED_STR, 0) == 1; +} diff --git a/src/libosmo-mgcp/mgcp_codec.c b/src/libosmo-mgcp/mgcp_codec.c index d340e11..714a7c5 100644 --- a/src/libosmo-mgcp/mgcp_codec.c +++ b/src/libosmo-mgcp/mgcp_codec.c @@ -24,6 +24,7 @@ #include <osmocom/mgcp/mgcp_endp.h> #include <osmocom/mgcp/mgcp_trunk.h> #include <osmocom/mgcp/mgcp_codec.h> +#include <osmocom/mgcp/fmtp.h> #include <errno.h> /* Helper function to dump codec information of a specified codec to a printable @@ -116,9 +117,9 @@ * \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). + * \param[in] fmtp 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 mgcp_codec_add2(struct mgcp_conn_rtp *conn, int payload_type, const char *audio_name, const char *fmtp) { int rate; int channels; @@ -261,12 +262,16 @@ } } - /* Copy over optional codec parameters */ - if (param) { - codec->param = *param; - codec->param_present = true; - } else - codec->param_present = false; + if (fmtp) { + OSMO_STRLCPY_ARRAY(codec->fmtp, fmtp); + if (strlen(codec->fmtp) != strlen(fmtp)) { + LOGP(DLMGCP, LOGL_ERROR, "fmtp too long: %zu > %zu\n", strlen(fmtp), strlen(codec->fmtp)); + /* let's just hope what is there is still useful, worst case the call's audio doesn't work */ + } + } + + /* legacy */ + codec->param_present = false; conn->end.codecs_assigned++; return 0; @@ -276,6 +281,28 @@ return -EINVAL; } +/*! Legacy compat, use mgcp_codec_add2() instead to be able to pass any fmtp besides AMR octet-aligned=1. + * 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) +{ + const char *fmtp = NULL; + if (param && param->amr_octet_aligned_present) { + if (param->amr_octet_aligned) + fmtp = OSMO_MGCP_OCTET_ALIGNED_EQUALS_STR("1"); + else + fmtp = OSMO_MGCP_OCTET_ALIGNED_EQUALS_STR("0"); + } + + return mgcp_codec_add2(conn, payload_type, audio_name, fmtp); +} + /* 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": @@ -288,11 +315,14 @@ */ bool mgcp_codec_amr_is_octet_aligned(const struct mgcp_rtp_codec *codec) { - if (!codec->param_present) + if (!codec->fmtp[0]) { + /* Legacy */ + if (codec->param_present && codec->param.amr_octet_aligned_present) + return codec->param.amr_octet_aligned; return false; - if (!codec->param.amr_octet_aligned_present) - return false; - return codec->param.amr_octet_aligned; + } + + return osmo_mgcp_fmtp_get_int(codec->fmtp, OSMO_MGCP_OCTET_ALIGNED_STR, 0) == 1; } /* Compare two codecs, all parameters must match up */ @@ -462,13 +492,12 @@ /* Check if the codec has a specific AMR mode (octet-aligned or bandwith-efficient) set. */ bool mgcp_codec_amr_align_mode_is_indicated(const struct mgcp_rtp_codec *codec) { - if (codec->param_present == false) - return false; - if (!codec->param.amr_octet_aligned_present) - return false; - if (strcmp(codec->subtype_name, "AMR") != 0) - return false; - return true; + if (!codec->fmtp[0]) { + /* Legacy */ + return codec->param_present && codec->param.amr_octet_aligned_present; + } + + return osmo_mgcp_fmtp_get_val(NULL, 0, codec->fmtp, OSMO_MGCP_OCTET_ALIGNED_STR) > 0; } /* Find the payload type number configured for a specific codec by SDP. diff --git a/src/libosmo-mgcp/mgcp_network.c b/src/libosmo-mgcp/mgcp_network.c index 46d0cb4..2ff7631 100644 --- a/src/libosmo-mgcp/mgcp_network.c +++ b/src/libosmo-mgcp/mgcp_network.c @@ -1212,12 +1212,12 @@ if (mgcp_conn_rtp_is_iuup(conn_dst) || mgcp_conn_rtp_is_iuup(conn_src)) { /* the iuup code will correctly transform to the correct AMR mode */ } else if (mgcp_codec_amr_align_mode_is_indicated(conn_dst->end.codec)) { - rc = amr_oa_bwe_convert(endp, msg, - conn_dst->end.codec->param.amr_octet_aligned); + bool oa = mgcp_codec_amr_is_octet_aligned(conn_dst->end.codec); + rc = amr_oa_bwe_convert(endp, msg, oa); if (rc < 0) { LOGPENDP(endp, DRTP, LOGL_ERROR, "Error in AMR octet-aligned <-> bandwidth-efficient mode conversion (target=%s)\n", - conn_dst->end.codec->param.amr_octet_aligned ? "octet-aligned" : "bandwidth-efficient"); + oa ? "octet-aligned" : "bandwidth-efficient"); break; } } else if (rtp_end->rfc5993_hr_convert && @@ -1527,16 +1527,18 @@ /* Handle AMR frame format conversion (octet-aligned vs. bandwith-efficient) */ if (mc->proto == MGCP_PROTO_RTP && mgcp_codec_amr_align_mode_is_indicated(conn_src->end.codec)) { + bool src_oa; /* Make sure that the incoming AMR frame format matches the frame format that the call agent has * communicated via SDP when the connection was created/modfied. */ int oa = amr_oa_check((char*)msgb_data(msg), msgb_length(msg)); if (oa < 0) return -1; - if (((bool)oa) != conn_src->end.codec->param.amr_octet_aligned) { + src_oa = mgcp_codec_amr_is_octet_aligned(conn_src->end.codec); + if (((bool)oa) != src_oa) { LOG_CONN_RTP(conn_src, LOGL_NOTICE, "rx_rtp(%u bytes): Expected RTP AMR octet-aligned=%u but got octet-aligned=%u." " check the config of your call-agent!\n", - msgb_length(msg), conn_src->end.codec->param.amr_octet_aligned, oa); + msgb_length(msg), src_oa, oa); return -1; } } diff --git a/src/libosmo-mgcp/mgcp_protocol.c b/src/libosmo-mgcp/mgcp_protocol.c index 809622b..136787b 100644 --- a/src/libosmo-mgcp/mgcp_protocol.c +++ b/src/libosmo-mgcp/mgcp_protocol.c @@ -1134,6 +1134,10 @@ LOGPCONN(_conn, DLMGCP, LOGL_NOTICE, "CRCX: connection successfully created\n"); + + for (int i = 0; i < conn->end.codecs_assigned; i++) + LOGPCONN(_conn, DLMGCP, LOGL_NOTICE, "fmtp=%s\n", conn->end.codecs[i].fmtp); + rate_ctr_inc(rate_ctr_group_get_ctr(rate_ctrs, MGCP_CRCX_SUCCESS)); mgcp_endp_update(endp); diff --git a/src/libosmo-mgcp/mgcp_sdp.c b/src/libosmo-mgcp/mgcp_sdp.c index 10822e5..4345a27 100644 --- a/src/libosmo-mgcp/mgcp_sdp.c +++ b/src/libosmo-mgcp/mgcp_sdp.c @@ -34,6 +34,7 @@ #include <osmocom/mgcp/mgcp_codec.h> #include <osmocom/mgcp/mgcp_sdp.h> #include <osmocom/mgcp/mgcp_protocol.h> +#include <osmocom/mgcp/fmtp.h> #include <errno.h> #include <stdlib.h> @@ -55,7 +56,7 @@ }; struct sdp_fmtp_param { int payload_type; - struct mgcp_codec_param param; + const char *fmtp; }; @@ -195,11 +196,7 @@ { char *str; char *str_ptr; - char *param_str; unsigned int pt; - unsigned int count = 0; - char delimiter; - unsigned int amr_octet_aligned; memset(fmtp_param, 0, sizeof(*fmtp_param)); @@ -218,40 +215,13 @@ goto error; fmtp_param->payload_type = pt; - /* Advance pointer to the beginning of the parameter section and - * tokenize string */ + /* Advance pointer to the beginning of the parameter section */ str_ptr = strstr(str_ptr, " "); if (!str_ptr) goto error; str_ptr++; - param_str = strtok(str_ptr, " "); - if (!param_str) - goto exit; - - while (1) { - /* Make sure that we don't get trapped in an endless loop */ - if (count > 256) - goto error; - - /* Chop off delimiters ';' at the end */ - delimiter = str_ptr[strlen(str_ptr) - 1]; - if (delimiter == ';' || delimiter == ',') - str_ptr[strlen(str_ptr) - 1] = '\0'; - - /* AMR octet aligned parameter (see also RFC 3267, section 8.3) */ - if (sscanf(param_str, "octet-align=%d", &amr_octet_aligned) == 1) { - fmtp_param->param.amr_octet_aligned_present = true; - fmtp_param->param.amr_octet_aligned = false; - if (amr_octet_aligned == 1) - fmtp_param->param.amr_octet_aligned = true; - } - - param_str = strtok(NULL, " "); - if (!param_str) - break; - count++; - } + fmtp_param->fmtp = talloc_strdup(ctx, str_ptr); exit: talloc_free(str); @@ -299,13 +269,13 @@ /* Pick optional fmtp parameters by payload type, if there are no fmtp * parameters, a nullpointer is returned */ -static struct mgcp_codec_param *param_by_pt(int pt, struct sdp_fmtp_param *fmtp_params, unsigned int fmtp_params_len) +static const char *param_by_pt(int pt, struct sdp_fmtp_param *fmtp_params, unsigned int fmtp_params_len) { unsigned int i; for (i = 0; i < fmtp_params_len; i++) { if (fmtp_params[i].payload_type == pt) - return &fmtp_params[i].param; + return fmtp_params[i].fmtp; } return NULL; @@ -326,7 +296,6 @@ unsigned int codecs_used = 0; struct sdp_fmtp_param fmtp_params[MGCP_MAX_CODECS]; unsigned int fmtp_used = 0; - struct mgcp_codec_param *codec_param; char ipbuf[INET6_ADDRSTRLEN]; char *line; unsigned int i; @@ -421,8 +390,8 @@ /* Store parsed codec information */ for (i = 0; i < codecs_used; i++) { - codec_param = param_by_pt(codecs[i].payload_type, fmtp_params, fmtp_used); - rc = mgcp_codec_add(conn, codecs[i].payload_type, codecs[i].map_line, codec_param); + const char *fmtp = param_by_pt(codecs[i].payload_type, fmtp_params, fmtp_used); + rc = mgcp_codec_add2(conn, codecs[i].payload_type, codecs[i].map_line, fmtp); if (rc < 0) LOGPENDP(endp, DLMGCP, LOGL_NOTICE, "failed to add codec\n"); } @@ -436,10 +405,12 @@ if (codecs_used == 0) LOGPC(DLMGCP, LOGL_NOTICE, "none"); for (i = 0; i < codecs_used; i++) { - LOGPC(DLMGCP, LOGL_NOTICE, "%d=%s", + LOGPC(DLMGCP, LOGL_NOTICE, " %d=%s%s%s%s", rtp->codecs[i].payload_type, - strlen(rtp->codecs[i].subtype_name) ? rtp->codecs[i].subtype_name : "unknown"); - LOGPC(DLMGCP, LOGL_NOTICE, " "); + strlen(rtp->codecs[i].subtype_name) ? rtp->codecs[i].subtype_name : "unknown", + rtp->codecs[i].fmtp[0] ? ",fmtp='" : "", + rtp->codecs[i].fmtp, + rtp->codecs[i].fmtp[0] ? "'" : ""); } LOGPC(DLMGCP, LOGL_NOTICE, "\n"); @@ -518,23 +489,23 @@ } for (i = 0; i < fmtp_params_len; i++) { + bool first = true; rc = msgb_printf(sdp, "a=fmtp:%u", fmtp_params[i].payload_type); if (rc < 0) return -EINVAL; /* Add amr octet align parameter */ - if (fmtp_params[i].param.amr_octet_aligned_present) { - if (fmtp_params[i].param.amr_octet_aligned) - rc = msgb_printf(sdp, " octet-align=1"); - else - rc = msgb_printf(sdp, " octet-align=0"); - if (rc < 0) - return -EINVAL; + if (fmtp_params[i].fmtp) { + msgb_printf(sdp, "%s%s", first ? " " : ";", fmtp_params[i].fmtp); + first = false; } /* Append extra parameters from fmtp extra */ if (fmtp_params[i].payload_type == fmtp_extra_pt) { - rc = msgb_printf(sdp, " %s", fmtp_extra_pars); + rc = msgb_printf(sdp, "%s%s", + first ? " " : ";", + fmtp_extra_pars); + first = false; if (rc < 0) return -EINVAL; } @@ -561,7 +532,6 @@ const char *fmtp_extra; const char *audio_name; int payload_type; - struct sdp_fmtp_param fmtp_param; int rc; int payload_types[1]; int local_port; @@ -613,12 +583,25 @@ goto buffer_too_small; } - if (codec->param_present) { - fmtp_param.payload_type = payload_type; - fmtp_param.param = codec->param; - fmtp_params[0] = fmtp_param; + if (codec->fmtp[0]) { + fmtp_params[0] = (struct sdp_fmtp_param){ + .payload_type = payload_type, + .fmtp = codec->fmtp, + }; fmtp_params_len = 1; } + else if (codec->param_present) { + /* Legacy */ + fmtp_params[0] = (struct sdp_fmtp_param){ + .payload_type = payload_type, + }; + fmtp_params_len = 1; + if (codec->param.amr_octet_aligned) + fmtp_params[0].fmtp = OSMO_MGCP_OCTET_ALIGNED_EQUALS_STR("1"); + else + fmtp_params[0].fmtp = OSMO_MGCP_OCTET_ALIGNED_EQUALS_STR("0"); + } + rc = add_fmtp(sdp, fmtp_params, fmtp_params_len, fmtp_extra); if (rc < 0) goto buffer_too_small; diff --git a/tests/mgcp/mgcp_test.c b/tests/mgcp/mgcp_test.c index e37bb57..f88b3e2 100644 --- a/tests/mgcp/mgcp_test.c +++ b/tests/mgcp/mgcp_test.c @@ -552,7 +552,7 @@ "t=0 0\r\n" \ "m=audio 16012 RTP/AVP 111\r\n" \ "a=rtpmap:111 AMR/8000/1\r\n" \ - "a=fmtp:111 octet-align=1\r\n" \ + "a=fmtp:111 mode-change-capability=2; octet-align=1\r\n" \ "a=ptime:20\r\n" #define CRCX_NO_LCO_NO_SDP_RET \ @@ -845,6 +845,8 @@ } } else if (check_response(msg->data, t->exp_resp) != 0) { printf("%s failed.\n", t->name); + fflush(stderr); + fflush(stdout); OSMO_ASSERT(false); } -- To view, visit https://gerrit.osmocom.org/c/osmo-mgw/+/34900?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-mgw Gerrit-Branch: master Gerrit-Change-Id: If58590bda8627519ff07e0b6f43aa47a274f052b Gerrit-Change-Number: 34900 Gerrit-PatchSet: 1 Gerrit-Owner: neels <nhofmeyr(a)sysmocom.de> Gerrit-MessageType: newchange

1 0

[M] Change in osmo-mgw[master]: mgcp_client_fsm: fix inconsistent API (param_present, param).
by neels 25 Oct '23

25 Oct '23

Attention is currently required from: dexter, pespin. neels has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-mgw/+/34351?usp=email ) Change subject: mgcp_client_fsm: fix inconsistent API (param_present, param). ...................................................................... Patch Set 8: (1 comment) File include/osmocom/mgcp/mgcp_common.h: https://gerrit.osmocom.org/c/osmo-mgw/+/34351/comment/3befaf34_62063e21 PS8, Line 73: } amr; > let's add a char* fmtp entry to struct ptmap instead. […] (that fmtp_get_int() idea would also need another argument: a default value in case the fmtp in question is omitted) -- To view, visit https://gerrit.osmocom.org/c/osmo-mgw/+/34351?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-mgw Gerrit-Branch: master Gerrit-Change-Id: I50d737f3f3d45e4004c64101700a471fe75b3436 Gerrit-Change-Number: 34351 Gerrit-PatchSet: 8 Gerrit-Owner: dexter <pmaier(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: neels <nhofmeyr(a)sysmocom.de> Gerrit-CC: pespin <pespin(a)sysmocom.de> Gerrit-Attention: pespin <pespin(a)sysmocom.de> Gerrit-Attention: dexter <pmaier(a)sysmocom.de> Gerrit-Comment-Date: Wed, 25 Oct 2023 21:45:41 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: neels <nhofmeyr(a)sysmocom.de> Gerrit-MessageType: comment

1 0

[M] Change in osmo-mgw[master]: mgcp_client_fsm: fix inconsistent API (param_present, param).
by neels 25 Oct '23

25 Oct '23

Attention is currently required from: dexter, pespin. neels has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-mgw/+/34351?usp=email ) Change subject: mgcp_client_fsm: fix inconsistent API (param_present, param). ...................................................................... Patch Set 8: (1 comment) File src/libosmo-mgcp-client/mgcp_client.c: https://gerrit.osmocom.org/c/osmo-mgw/+/34351/comment/2d415c7c_33c76717 PS8, Line 1374: OSMO_ASSERT(!(mgcp_msg->param_present && mgcp_msg->codecs_param_present)); > when we add a fmtp string to ptmap, then we don't need a codecs_param list nor a codecs_param_presen […] hmm, can we assume to control all existing osmo_mgcp_cient users? it would be very tempting to just drop the stupid 'param' completely, because its compatibility code would have to compose fmtp strings again... just thinking, probably not going to happen, right -- To view, visit https://gerrit.osmocom.org/c/osmo-mgw/+/34351?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-mgw Gerrit-Branch: master Gerrit-Change-Id: I50d737f3f3d45e4004c64101700a471fe75b3436 Gerrit-Change-Number: 34351 Gerrit-PatchSet: 8 Gerrit-Owner: dexter <pmaier(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: neels <nhofmeyr(a)sysmocom.de> Gerrit-CC: pespin <pespin(a)sysmocom.de> Gerrit-Attention: pespin <pespin(a)sysmocom.de> Gerrit-Attention: dexter <pmaier(a)sysmocom.de> Gerrit-Comment-Date: Wed, 25 Oct 2023 21:43:29 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: neels <nhofmeyr(a)sysmocom.de> Gerrit-MessageType: comment

1 0

[M] Change in osmo-mgw[master]: mgcp_client_fsm: fix inconsistent API (param_present, param).
by neels 25 Oct '23

25 Oct '23

Attention is currently required from: dexter, pespin. neels has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-mgw/+/34351?usp=email ) Change subject: mgcp_client_fsm: fix inconsistent API (param_present, param). ...................................................................... Patch Set 8: Code-Review-1 (7 comments) Patchset: PS8: this can be much simpler, especially after https://gerrit.osmocom.org/c/osmo-mgw/+/34899 ... File include/osmocom/mgcp/mgcp_common.h: https://gerrit.osmocom.org/c/osmo-mgw/+/34351/comment/169b3632_9f718fee PS8, Line 73: } amr; let's add a char* fmtp entry to struct ptmap instead. To go with it, let's add individual parsing functions like bool oa = (fmtp_get_int(ptmap->fmtp, "octet-align") == 1) The parsing should only happen when we actually need details on an AMR entry, not on every message parsing and composition. why: so what i don't like about this is that it needs to model each and every fmtp that exists in anyone's MGCP in a C struct. Much rather, I'd suggest that we simply keep an actual string of the fmtp, and evaluate that string whenever we would need the conveyed information. Like that we can transport and respond with fmtp that osmo-mgw does not even need to care about. With the C struct implementation, we silently drop all unknown fmtp in a conversation, or we need to error a lot. File include/osmocom/mgcp_client/mgcp_client_fsm.h: https://gerrit.osmocom.org/c/osmo-mgw/+/34351/comment/ba48c374_58bbd033 PS2, Line 70: struct mgcp_codec_param2 codecs_param[MGCP_MAX_CODECS]; > Ok, better sort that kind of problems *before* this patch. Instead please add a fmtp entry to the struct ptmap, so that all of pt, codec and fmtp are kept in the same place. (further below i explain why i think it should be a string) why: i don't like this at all at all, it creates a separate array that needs to be kept in sync with (in this patch) the 'codecs' list as well as the 'ptmap' list. While doing this review, i submitted a patch to combine those two into just 'ptmap', because it is complete madness to keep N separate arrays of individual struct members, when you can just keep one array of structs. It would be cool to continue in that way. File src/libosmo-mgcp-client/mgcp_client.c: https://gerrit.osmocom.org/c/osmo-mgw/+/34351/comment/3e14e32c_d1270c9c PS8, Line 1374: OSMO_ASSERT(!(mgcp_msg->param_present && mgcp_msg->codecs_param_present)); when we add a fmtp string to ptmap, then we don't need a codecs_param list nor a codecs_param_present flag. Instead I would see if in an incoming mgcp_msg, if param_present == true, apply that param only to those ptmap entries where there is no fmtp yet. It would be cooler if we could just drop param, but we need to be backwards compatible, right? https://gerrit.osmocom.org/c/osmo-mgw/+/34351/comment/0aefc4f3_c31feb32 PS8, Line 1391: for (i = 0; i < mgcp_msg->codecs_len; i++) { if you accept my suggestion in https://gerrit.osmocom.org/c/osmo-mgw/+/34899 then here you'd iterate mgcp_msg->ptmap instead. https://gerrit.osmocom.org/c/osmo-mgw/+/34351/comment/d38e13de_a81a82c0 PS8, Line 1394: continue; so this is dropping all fmtp except for AMR codecs, not ideal. Instead we could just simply store the string as it is. https://gerrit.osmocom.org/c/osmo-mgw/+/34351/comment/4a071b36_1af1b90e PS8, Line 1399: MSGB_PRINTF_OR_RET("a=fmtp:%u octet-align=0\r\n", pt); i don't like this: this is implementing codec-specific fmtp composition in osmo-mgw. Instead we should use the exact string that the user provided. If we take on fmtp composition, we are opening a can of worms. For example, the octet-align parameter is specified with a default value. It is usually only present when it should differ from the default, and omitted if it is the default. I'm not saying which one is better -- the point is that we don't want to have to make these decisions in osmo-mgw, at all. If we keep the fmtp string 1:1, we can just lean back and use whatever clients throw at us, just carry the fmtp along as they are. Only when we need a fmtp detail like for AMR OA vs BE conversion, only then do we need to validate that the fmtp parameters make any sense. Only then will we have to throw an error if the fmtp is garbage. For those cases we don't care whether a default value is present or omitted, we just need to get the bit of message handling relevant information from it. -- To view, visit https://gerrit.osmocom.org/c/osmo-mgw/+/34351?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-mgw Gerrit-Branch: master Gerrit-Change-Id: I50d737f3f3d45e4004c64101700a471fe75b3436 Gerrit-Change-Number: 34351 Gerrit-PatchSet: 8 Gerrit-Owner: dexter <pmaier(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: neels <nhofmeyr(a)sysmocom.de> Gerrit-CC: pespin <pespin(a)sysmocom.de> Gerrit-Attention: pespin <pespin(a)sysmocom.de> Gerrit-Attention: dexter <pmaier(a)sysmocom.de> Gerrit-Comment-Date: Wed, 25 Oct 2023 21:39:26 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Comment-In-Reply-To: pespin <pespin(a)sysmocom.de> Comment-In-Reply-To: dexter <pmaier(a)sysmocom.de> Gerrit-MessageType: comment

1 0

[S] Change in osmo-mgw[master]: mgcp_parse_audio_port_pt(): fix buffer overflow
by pespin 25 Oct '23

25 Oct '23

Attention is currently required from: neels. pespin has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-mgw/+/34898?usp=email ) Change subject: mgcp_parse_audio_port_pt(): fix buffer overflow ...................................................................... Patch Set 1: (1 comment) File src/libosmo-mgcp-client/mgcp_client.c: https://gerrit.osmocom.org/c/osmo-mgw/+/34898/comment/9ebb4e86_7aea0f98 PS1, Line 321: /* Do not allow excessive payload types */ why is this r->ptmap now? I see you are acessing r->codecs[count] below... -- To view, visit https://gerrit.osmocom.org/c/osmo-mgw/+/34898?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-mgw Gerrit-Branch: master Gerrit-Change-Id: I18c78d15eb1593f404b4741248225b68878b463f Gerrit-Change-Number: 34898 Gerrit-PatchSet: 1 Gerrit-Owner: neels <nhofmeyr(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-CC: pespin <pespin(a)sysmocom.de> Gerrit-Attention: neels <nhofmeyr(a)sysmocom.de> Gerrit-Comment-Date: Wed, 25 Oct 2023 21:14:57 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

[S] Change in osmo-mgw[master]: mgcp_client_fsm: explain member param in struct mgcp_conn_peer better
by neels 25 Oct '23

25 Oct '23

Attention is currently required from: dexter. neels has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-mgw/+/34350?usp=email ) Change subject: mgcp_client_fsm: explain member param in struct mgcp_conn_peer better ...................................................................... Patch Set 4: (1 comment) Patchset: PS4: ok, though it is a bit weird to improve the documentation for a struct member that we need to replace (in order to allow varying fmtp per ptmap entry) -- To view, visit https://gerrit.osmocom.org/c/osmo-mgw/+/34350?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-mgw Gerrit-Branch: master Gerrit-Change-Id: Iea4dc1e72fccaa464ce503fae88b5d8a867b1d19 Gerrit-Change-Number: 34350 Gerrit-PatchSet: 4 Gerrit-Owner: dexter <pmaier(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: laforge <laforge(a)osmocom.org> Gerrit-Reviewer: pespin <pespin(a)sysmocom.de> Gerrit-CC: neels <nhofmeyr(a)sysmocom.de> Gerrit-Attention: dexter <pmaier(a)sysmocom.de> Gerrit-Comment-Date: Wed, 25 Oct 2023 20:58:28 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

[M] Change in osmo-mgw[master]: client: collapse codecs[] and ptmap[]; allow codec variants
by neels 25 Oct '23

25 Oct '23

Hello Jenkins Builder, I'd like you to reexamine a change. Please visit https://gerrit.osmocom.org/c/osmo-mgw/+/34899?usp=email to look at the new patch set (#3). Change subject: client: collapse codecs[] and ptmap[]; allow codec variants ...................................................................... client: collapse codecs[] and ptmap[]; allow codec variants codecs[] is an array of enum osmo_mgcp_codecs. ptmap[] is an array of { enum osmo_mgcp_codecs, unsigned int ptmap }. MGCP lists first a bunch of payload type numbers and then specifies them again for details, like the numbers 112, 96, 3 in this example: m=audio <port> RTP/AVP 112 96 3 a=rtpmap:112 AMR/8000 a=rtpmap:96 VND.3GPP.IUFP/16000 a=rtpmap:3 GSM-FR/8000 So far we keep these lists in two separate arrays, in both struct mgcp_response and struct mgcp_msg: - codecs[], codecs_len stores the 'm=audio' list - ptmap[], ptmap_len stores the 'a=rtpmap' list These are semantically identical, and the separation means we cannot have the same codec twice, like AMR with different fmtp variations. It also leads to checks, conversions and dear hopes of correct ordering. So let's keep only one list with both codec and payload type number in it. The 'm=audio' list establishes the order of the pt numbers, and the 'a=rtpmap' list adds codec information to the established entries. Related: OS#6171 Change-Id: I798e02c6663376d3d52f4a74fc4b32411ce95bed --- M include/osmocom/mgcp_client/mgcp_client.h M include/osmocom/mgcp_client/mgcp_client_fsm.h M src/libosmo-mgcp-client/mgcp_client.c M src/libosmo-mgcp-client/mgcp_client_fsm.c M tests/mgcp_client/mgcp_client_test.c 5 files changed, 109 insertions(+), 63 deletions(-) git pull ssh://gerrit.osmocom.org:29418/osmo-mgw refs/changes/99/34899/3 -- To view, visit https://gerrit.osmocom.org/c/osmo-mgw/+/34899?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-mgw Gerrit-Branch: master Gerrit-Change-Id: I798e02c6663376d3d52f4a74fc4b32411ce95bed Gerrit-Change-Number: 34899 Gerrit-PatchSet: 3 Gerrit-Owner: neels <nhofmeyr(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-MessageType: newpatchset

1 0

[M] Change in osmo-mgw[master]: client: collapse codecs[] and ptmap[]; allow codec variants
by neels 25 Oct '23

25 Oct '23

Hello Jenkins Builder, I'd like you to reexamine a change. Please visit https://gerrit.osmocom.org/c/osmo-mgw/+/34899?usp=email to look at the new patch set (#2). Change subject: client: collapse codecs[] and ptmap[]; allow codec variants ...................................................................... client: collapse codecs[] and ptmap[]; allow codec variants codecs[] is an array of enum osmo_mgcp_codecs. ptmap[] is an array of { enum osmo_mgcp_codecs, unsigned int ptmap }. MGCP lists first a bunch of payload type numbers and then specifies them again for details, like the numbers 112, 96, 3 in this example: m=audio <port> RTP/AVP 112 96 3 a=rtpmap:112 AMR/8000 a=rtpmap:96 VND.3GPP.IUFP/16000 a=rtpmap:3 GSM-FR/8000 So far we keep these lists in two separate arrays, in both struct mgcp_response and struct mgcp_msg: - codecs[], codecs_len stores the 'm=audio' list - ptmap[], ptmap_len stores the 'a=rtpmap' list These are semantically identical, and the separation means we cannot have the same codec twice, like AMR with different fmtp variations. It also leads to checks, conversions and dear hopes of correct ordering. So let's keep only one list with both codec and payload type number in it. The 'm=audio' list establishes the order of the pt numbers, and the 'a=rtpmap' list adds codec information to the established entries. Change-Id: I798e02c6663376d3d52f4a74fc4b32411ce95bed --- M include/osmocom/mgcp_client/mgcp_client.h M include/osmocom/mgcp_client/mgcp_client_fsm.h M src/libosmo-mgcp-client/mgcp_client.c M src/libosmo-mgcp-client/mgcp_client_fsm.c M tests/mgcp_client/mgcp_client_test.c 5 files changed, 108 insertions(+), 63 deletions(-) git pull ssh://gerrit.osmocom.org:29418/osmo-mgw refs/changes/99/34899/2 -- To view, visit https://gerrit.osmocom.org/c/osmo-mgw/+/34899?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-mgw Gerrit-Branch: master Gerrit-Change-Id: I798e02c6663376d3d52f4a74fc4b32411ce95bed Gerrit-Change-Number: 34899 Gerrit-PatchSet: 2 Gerrit-Owner: neels <nhofmeyr(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-MessageType: newpatchset

1 0

[M] Change in osmo-mgw[master]: mgcp_client_fsm: allow the same codec multiple times in ptmap
by neels 25 Oct '23

25 Oct '23

Attention is currently required from: dexter, pespin. neels has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-mgw/+/34349?usp=email ) Change subject: mgcp_client_fsm: allow the same codec multiple times in ptmap ...................................................................... Patch Set 5: Code-Review-2 (1 comment) Patchset: PS5: i see the actual problem this patch is working around ... let's do this instead: https://gerrit.osmocom.org/c/osmo-mgw/+/34899 ttcn3 test suite says it's good. -- To view, visit https://gerrit.osmocom.org/c/osmo-mgw/+/34349?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-mgw Gerrit-Branch: master Gerrit-Change-Id: Ie13ce59d3165936a16e16588b4d58f0ce7e0ae67 Gerrit-Change-Number: 34349 Gerrit-PatchSet: 5 Gerrit-Owner: dexter <pmaier(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: neels <nhofmeyr(a)sysmocom.de> Gerrit-Reviewer: pespin <pespin(a)sysmocom.de> Gerrit-Attention: pespin <pespin(a)sysmocom.de> Gerrit-Attention: dexter <pmaier(a)sysmocom.de> Gerrit-Comment-Date: Wed, 25 Oct 2023 20:27:29 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

[S] Change in osmo-mgw[master]: mgcp_parse_audio_port_pt(): fix buffer overflow
by neels 25 Oct '23

25 Oct '23

neels has uploaded this change for review. ( https://gerrit.osmocom.org/c/osmo-mgw/+/34898?usp=email ) Change subject: mgcp_parse_audio_port_pt(): fix buffer overflow ...................................................................... mgcp_parse_audio_port_pt(): fix buffer overflow Change-Id: I18c78d15eb1593f404b4741248225b68878b463f --- M src/libosmo-mgcp-client/mgcp_client.c 1 file changed, 10 insertions(+), 1 deletion(-) git pull ssh://gerrit.osmocom.org:29418/osmo-mgw refs/changes/98/34898/1 diff --git a/src/libosmo-mgcp-client/mgcp_client.c b/src/libosmo-mgcp-client/mgcp_client.c index 5df4560..95c7a4b 100644 --- a/src/libosmo-mgcp-client/mgcp_client.c +++ b/src/libosmo-mgcp-client/mgcp_client.c @@ -319,7 +319,7 @@ pt_str = strtok(line, " "); while (1) { /* Do not allow excessive payload types */ - if (count > ARRAY_SIZE(r->codecs)) + if (count >= ARRAY_SIZE(r->ptmap)) goto response_parse_failure_pt; pt_str = strtok(NULL, " "); -- To view, visit https://gerrit.osmocom.org/c/osmo-mgw/+/34898?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-mgw Gerrit-Branch: master Gerrit-Change-Id: I18c78d15eb1593f404b4741248225b68878b463f Gerrit-Change-Number: 34898 Gerrit-PatchSet: 1 Gerrit-Owner: neels <nhofmeyr(a)sysmocom.de> Gerrit-MessageType: newchange

1 0