[L] Change in osmo-mgw[master]: mgw: Introduce struct mgcp_codecset struct

pespin has uploaded this change for review. ( https://gerrit.osmocom.org/c/osmo-mgw/+/39215?usp=email ) Change subject: mgw: Introduce struct mgcp_codecset struct ...................................................................... mgw: Introduce struct mgcp_codecset struct We do tons of operations on 3 fields (array of codecs, len of array, selected code) which can be isolated. Right now, we are using APIs which somehow require structs 2-3 levels of encapsulation above the ones really required, which makes all code totally entangled, difficult to understand and prone to errors when changing stuff in deeply nested structs. A prove of this is how this patch affects a lot of lines in lots of places. Change-Id: Id7db7ab01d56b7fa2415123b604375e48c82ab25 --- M include/osmocom/mgcp/mgcp_codec.h M include/osmocom/mgcp/mgcp_rtp_end.h M src/libosmo-mgcp/mgcp_codec.c M src/libosmo-mgcp/mgcp_conn.c M src/libosmo-mgcp/mgcp_e1.c M src/libosmo-mgcp/mgcp_iuup.c M src/libosmo-mgcp/mgcp_network.c M src/libosmo-mgcp/mgcp_osmux.c M src/libosmo-mgcp/mgcp_protocol.c M src/libosmo-mgcp/mgcp_sdp.c M src/libosmo-mgcp/mgcp_vty.c M tests/mgcp/mgcp_test.c 12 files changed, 171 insertions(+), 169 deletions(-) git pull ssh://gerrit.osmocom.org:29418/osmo-mgw refs/changes/15/39215/1 diff --git a/include/osmocom/mgcp/mgcp_codec.h b/include/osmocom/mgcp/mgcp_codec.h index 1df8055..0d99fdf 100644 --- a/include/osmocom/mgcp/mgcp_codec.h +++ b/include/osmocom/mgcp/mgcp_codec.h @@ -26,14 +26,23 @@ struct mgcp_codec_param param; }; -struct mgcp_conn_rtp; - -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_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); bool mgcp_codec_amr_align_mode_is_indicated(const struct mgcp_rtp_codec *codec); bool mgcp_codec_amr_is_octet_aligned(const struct mgcp_rtp_codec *codec); -struct mgcp_rtp_codec *mgcp_codec_from_pt(struct mgcp_conn_rtp *conn, int payload_type); + +struct mgcp_rtp_codecset { + /* currently selected audio codec */ + struct mgcp_rtp_codec *codec; + /* array with assigned audio codecs to choose from (SDP) */ + struct mgcp_rtp_codec codecs[MGCP_MAX_CODECS]; + /* number of assigned audio codecs (SDP) */ + unsigned int codecs_assigned; +}; + +void mgcp_codecset_reset(struct mgcp_rtp_codecset *cset); +void mgcp_codecset_summary(struct mgcp_rtp_codecset *cset, const char *prefix_str); +int mgcp_codecset_add_codec(struct mgcp_rtp_codecset *cset, int payload_type, + const char *audio_name, const struct mgcp_codec_param *param); +int mgcp_codecset_decide(struct mgcp_rtp_codecset *cset_src, struct mgcp_rtp_codecset *cset_dst); +const struct mgcp_rtp_codec *mgcp_codecset_pt_find_by_subtype_name(const struct mgcp_rtp_codecset *cset, + const char *subtype_name, unsigned int match_nr); +struct mgcp_rtp_codec *mgcp_codecset_find_codec_from_pt(struct mgcp_rtp_codecset *cset, int payload_type); diff --git a/include/osmocom/mgcp/mgcp_rtp_end.h b/include/osmocom/mgcp/mgcp_rtp_end.h index 2ab300a..69208e5 100644 --- a/include/osmocom/mgcp/mgcp_rtp_end.h +++ b/include/osmocom/mgcp/mgcp_rtp_end.h @@ -17,14 +17,7 @@ /* in network byte order */ uint16_t rtcp_port; - /* currently selected audio codec */ - struct mgcp_rtp_codec *codec; - - /* array with assigned audio codecs to choose from (SDP) */ - struct mgcp_rtp_codec codecs[MGCP_MAX_CODECS]; - - /* number of assigned audio codecs (SDP) */ - unsigned int codecs_assigned; + struct mgcp_rtp_codecset cset; /* per endpoint data */ int frames_per_packet; diff --git a/src/libosmo-mgcp/mgcp_codec.c b/src/libosmo-mgcp/mgcp_codec.c index 2f1df7b..0a57667 100644 --- a/src/libosmo-mgcp/mgcp_codec.c +++ b/src/libosmo-mgcp/mgcp_codec.c @@ -28,7 +28,7 @@ /* Helper function to dump codec information of a specified codec to a printable * string, used by dump_codec_summary() */ -static char *dump_codec(struct mgcp_rtp_codec *codec) +static char *mgcp_codec_dump(struct mgcp_rtp_codec *codec) { static char str[256]; char *pt_str; @@ -49,31 +49,25 @@ } /*! Dump a summary of all negotiated codecs to debug log - * \param[in] conn related rtp-connection. */ -void mgcp_codec_summary(struct mgcp_conn_rtp *conn) + * \param[in] cset related codecset. + * \param[in] prefix_str Prefix string to print during logging. + * */ +void mgcp_codecset_summary(struct mgcp_rtp_codecset *cset, const char *prefix_str) { - struct mgcp_rtp_end *rtp; unsigned int i; - struct mgcp_rtp_codec *codec; - struct mgcp_endpoint *endp; - rtp = &conn->end; - endp = conn->conn->endp; - - if (rtp->codecs_assigned == 0) { - LOGPENDP(endp, DLMGCP, LOGL_ERROR, "conn:%s no codecs available\n", - mgcp_conn_dump(conn->conn)); + if (cset->codecs_assigned == 0) { + LOGP(DLMGCP, LOGL_ERROR, "%s no codecs available\n", prefix_str); return; } /* Store parsed codec information */ - for (i = 0; i < rtp->codecs_assigned; i++) { - codec = &rtp->codecs[i]; + for (i = 0; i < cset->codecs_assigned; i++) { + struct mgcp_rtp_codec *codec = &cset->codecs[i]; - LOGPENDP(endp, DLMGCP, LOGL_DEBUG, "conn:%s codecs[%u]:%s", - mgcp_conn_dump(conn->conn), i, dump_codec(codec)); + LOGP(DLMGCP, LOGL_DEBUG, "%s codecs[%u]:%s", prefix_str, i, mgcp_codec_dump(codec)); - if (codec == rtp->codec) + if (codec == cset->codec) LOGPC(DLMGCP, LOGL_DEBUG, " [selected]"); LOGPC(DLMGCP, LOGL_DEBUG, "\n"); @@ -81,7 +75,7 @@ } /* Initalize or reset codec information with default data. */ -static void codec_init(struct mgcp_rtp_codec *codec) +static void mgcp_codec_init(struct mgcp_rtp_codec *codec) { *codec = (struct mgcp_rtp_codec){ .payload_type = -1, @@ -94,20 +88,20 @@ }; } -static void codec_free(struct mgcp_rtp_codec *codec) +static void mgcp_codec_free(struct mgcp_rtp_codec *codec) { *codec = (struct mgcp_rtp_codec){}; } /*! Initalize or reset codec information with default data. * \param[out] conn related rtp-connection. */ -void mgcp_codec_reset_all(struct mgcp_conn_rtp *conn) +void mgcp_codecset_reset(struct mgcp_rtp_codecset *cset) { int i; - for (i = 0; i < conn->end.codecs_assigned; i++) - codec_free(&conn->end.codecs[i]); - conn->end.codecs_assigned = 0; - conn->end.codec = NULL; + for (i = 0; i < cset->codecs_assigned; i++) + mgcp_codec_free(&cset->codecs[i]); + cset->codecs_assigned = 0; + cset->codec = NULL; } /*! Add codec configuration depending on payload type and/or codec name. This @@ -118,23 +112,23 @@ * \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) +int mgcp_codecset_add_codec(struct mgcp_rtp_codecset *cset, int payload_type, const char *audio_name, const struct mgcp_codec_param *param) { int rate; int channels; struct mgcp_rtp_codec *codec; - unsigned int pt_offset = conn->end.codecs_assigned; + unsigned int pt_offset = cset->codecs_assigned; /* The amount of codecs we can store is limited, make sure we do not * overrun this limit. */ - if (conn->end.codecs_assigned >= MGCP_MAX_CODECS) + if (cset->codecs_assigned >= MGCP_MAX_CODECS) return -EINVAL; /* First unused entry */ - codec = &conn->end.codecs[conn->end.codecs_assigned]; + codec = &cset->codecs[cset->codecs_assigned]; /* Initalize the codec struct with some default data to begin with */ - codec_init(codec); + mgcp_codec_init(codec); if (payload_type != PTYPE_UNDEFINED) { /* Make sure we do not get any reserved or undefined type numbers */ @@ -268,11 +262,11 @@ } else codec->param_present = false; - conn->end.codecs_assigned++; + cset->codecs_assigned++; return 0; error: /* Make sure we leave a clean codec entry on error. */ - codec_free(codec); + mgcp_codec_free(codec); return -EINVAL; } @@ -296,7 +290,7 @@ } /* Compare two codecs, all parameters must match up */ -static bool codecs_same(struct mgcp_rtp_codec *codec_a, struct mgcp_rtp_codec *codec_b) +static bool codecs_same(const struct mgcp_rtp_codec *codec_a, const struct mgcp_rtp_codec *codec_b) { /* All codec properties must match up, except the payload type number. Even though standardisd payload numbers * exist for certain situations, the call agent may still assign them freely. Hence we must not insist on equal @@ -325,7 +319,7 @@ } /* Compare two codecs, all parameters must match up, except parameters related to payload formatting (not checked). */ -static bool codecs_convertible(struct mgcp_rtp_codec *codec_a, struct mgcp_rtp_codec *codec_b) +static bool codecs_convertible(const struct mgcp_rtp_codec *codec_a, const struct mgcp_rtp_codec *codec_b) { /* OsmoMGW currently has no ability to transcode from one codec to another. However OsmoMGW is still able to * translate between different payload formats as long as the encoded voice data itself does not change. @@ -354,21 +348,18 @@ return true; } -struct mgcp_rtp_codec *mgcp_codec_find_same(struct mgcp_conn_rtp *conn, struct mgcp_rtp_codec *codec) +struct mgcp_rtp_codec *mgcp_codecset_find_same(struct mgcp_rtp_codecset *cset, const struct mgcp_rtp_codec *codec) { - struct mgcp_rtp_end *rtp_end; unsigned int i; unsigned int codecs_assigned; - rtp_end = &conn->end; - /* Use the codec information from the source and try to find the equivalent of it on the destination side. In * the first run we will look for an exact match. */ - codecs_assigned = rtp_end->codecs_assigned; + codecs_assigned = cset->codecs_assigned; OSMO_ASSERT(codecs_assigned <= MGCP_MAX_CODECS); for (i = 0; i < codecs_assigned; i++) { - if (codecs_same(codec, &rtp_end->codecs[i])) { - return &rtp_end->codecs[i]; + if (codecs_same(codec, &cset->codecs[i])) { + return &cset->codecs[i]; break; } } @@ -377,28 +368,26 @@ } /* For a given codec, find a convertible codec in the given connection. */ -static struct mgcp_rtp_codec *codec_find_convertible(struct mgcp_conn_rtp *conn, struct mgcp_rtp_codec *codec) +static struct mgcp_rtp_codec *codecset_find_convertible(struct mgcp_rtp_codecset *cset, const struct mgcp_rtp_codec *codec) { - struct mgcp_rtp_end *rtp_end; unsigned int i; unsigned int codecs_assigned; struct mgcp_rtp_codec *codec_convertible = NULL; - rtp_end = &conn->end; /* Use the codec information from the source and try to find the equivalent of it on the destination side. In * the first run we will look for an exact match. */ - codec_convertible = mgcp_codec_find_same(conn, codec); + codec_convertible = mgcp_codecset_find_same(cset, codec); if (codec_convertible) return codec_convertible; /* In case we weren't able to find an exact match, we will try to find a match that is the same codec, but the * payload format may be different. This alternative will require a frame format conversion (i.e. AMR bwe->oe) */ - codecs_assigned = rtp_end->codecs_assigned; + codecs_assigned = cset->codecs_assigned; OSMO_ASSERT(codecs_assigned <= MGCP_MAX_CODECS); for (i = 0; i < codecs_assigned; i++) { - if (codecs_convertible(codec, &rtp_end->codecs[i])) { - codec_convertible = &rtp_end->codecs[i]; + if (codecs_convertible(codec, &cset->codecs[i])) { + codec_convertible = &cset->codecs[i]; break; } } @@ -408,20 +397,20 @@ /*! Decide for one suitable codec on both of the given connections. In case a destination connection is not available, * a tentative decision is made. - * \param[inout] conn_src related rtp-connection. - * \param[inout] conn_dst related destination rtp-connection (NULL if not present). + * \param[in] cset_src related codec set. + * \param[inout] cset_dst related destination codec set (NULL if not present). * \returns 0 on success, -EINVAL on failure. */ -int mgcp_codec_decide(struct mgcp_conn_rtp *conn_src, struct mgcp_conn_rtp *conn_dst) +int mgcp_codecset_decide(struct mgcp_rtp_codecset *cset_src, struct mgcp_rtp_codecset *cset_dst) { unsigned int i; /* In case no destination connection is available (yet), or in case the destination connection exists but has * no codecs assigned, we are forced to make a simple tentative decision: * We just use the first codec of the source connection (conn_src) */ - OSMO_ASSERT(conn_src->end.codecs_assigned <= MGCP_MAX_CODECS); - if (!conn_dst || conn_dst->end.codecs_assigned == 0) { - if (conn_src->end.codecs_assigned >= 1) { - conn_src->end.codec = &conn_src->end.codecs[0]; + OSMO_ASSERT(cset_src->codecs_assigned <= MGCP_MAX_CODECS); + if (!cset_dst || cset_dst->codecs_assigned == 0) { + if (cset_src->codecs_assigned >= 1) { + cset_src->codec = &cset_src->codecs[0]; return 0; } else return -EINVAL; @@ -430,38 +419,38 @@ /* Compare all codecs of the source connection (conn_src) to the codecs of the destination connection (conn_dst). In case * of a match set this codec on both connections. This would be an ideal selection since no codec conversion would be * required. */ - for (i = 0; i < conn_src->end.codecs_assigned; i++) { - struct mgcp_rtp_codec *codec_conn_src = &conn_src->end.codecs[i]; - struct mgcp_rtp_codec *codec_conn_dst = mgcp_codec_find_same(conn_dst, codec_conn_src); - if (codec_conn_dst) { + for (i = 0; i < cset_src->codecs_assigned; i++) { + struct mgcp_rtp_codec *codec_cset_src = &cset_src->codecs[i]; + struct mgcp_rtp_codec *codec_cset_dst = mgcp_codecset_find_same(cset_dst, codec_cset_src); + if (codec_cset_dst) { /* We found the a codec that is exactly the same (same codec, same payload format etc.) on both * sides. We now set this codec on both connections. */ - conn_dst->end.codec = codec_conn_dst; - conn_src->end.codec = codec_conn_src; + cset_dst->codec = codec_cset_dst; + cset_src->codec = codec_cset_src; return 0; } } /* In case we could not find a codec that is exactly the same, let's at least try to find a codec that we are able * to convert. */ - for (i = 0; i < conn_src->end.codecs_assigned; i++) { - struct mgcp_rtp_codec *codec_conn_src = &conn_src->end.codecs[i]; - struct mgcp_rtp_codec *codec_conn_dst = codec_find_convertible(conn_dst, codec_conn_src); - if (codec_conn_dst) { + for (i = 0; i < cset_src->codecs_assigned; i++) { + struct mgcp_rtp_codec *codec_cset_src = &cset_src->codecs[i]; + struct mgcp_rtp_codec *codec_cset_dst = codecset_find_convertible(cset_dst, codec_cset_src); + if (codec_cset_dst) { /* We found the a codec that we can convert to. Set each side to its codec. */ - conn_dst->end.codec = codec_conn_dst; - conn_src->end.codec = codec_conn_src; + cset_dst->codec = codec_cset_dst; + cset_src->codec = codec_cset_src; return 0; } } - if (conn_dst->end.codecs_assigned) - conn_dst->end.codec = &conn_dst->end.codecs[0]; + if (cset_dst->codecs_assigned) + cset_dst->codec = &cset_dst->codecs[0]; else return -EINVAL; - if (conn_src->end.codecs_assigned) - conn_src->end.codec = &conn_src->end.codecs[0]; + if (cset_src->codecs_assigned) + cset_src->codec = &cset_src->codecs[0]; else return -EINVAL; @@ -488,38 +477,36 @@ * \param match_nr Index for the match found, first being match_nr == 0. Iterate all matches by calling multiple times * with incrementing match_nr. * \return codec definition for that conn matching the subtype_name, or NULL if no such match_nr is found. */ -const struct mgcp_rtp_codec *mgcp_codec_pt_find_by_subtype_name(struct mgcp_conn_rtp *conn, +const struct mgcp_rtp_codec *mgcp_codecset_pt_find_by_subtype_name(const struct mgcp_rtp_codecset *cset, const char *subtype_name, unsigned int match_nr) { int i; - for (i = 0; i < conn->end.codecs_assigned; i++) { - if (!strcmp(conn->end.codecs[i].subtype_name, subtype_name)) { + for (i = 0; i < cset->codecs_assigned; i++) { + if (!strcmp(cset->codecs[i].subtype_name, subtype_name)) { if (match_nr) { match_nr--; continue; } - return &conn->end.codecs[i]; + return &cset->codecs[i]; } } return NULL; } /*! Lookup a codec that is assigned to a connection by its payload type number. - * \param[in] conn related rtp-connection. + * \param[in] cset related codec set. * \param[in] payload_type number of the codec to look up. * \returns pointer to codec struct on success, NULL on failure. */ -struct mgcp_rtp_codec *mgcp_codec_from_pt(struct mgcp_conn_rtp *conn, int payload_type) +struct mgcp_rtp_codec *mgcp_codecset_find_codec_from_pt(struct mgcp_rtp_codecset *cset, int payload_type) { - struct mgcp_rtp_end *rtp_end = &conn->end; - unsigned int codecs_assigned = rtp_end->codecs_assigned; struct mgcp_rtp_codec *codec = NULL; size_t i; - OSMO_ASSERT(codecs_assigned <= MGCP_MAX_CODECS); + OSMO_ASSERT(cset->codecs_assigned <= MGCP_MAX_CODECS); - for (i = 0; i < codecs_assigned; i++) { - if (payload_type == rtp_end->codecs[i].payload_type) { - codec = &rtp_end->codecs[i]; + for (i = 0; i < cset->codecs_assigned; i++) { + if (payload_type == cset->codecs[i].payload_type) { + codec = &cset->codecs[i]; break; } } diff --git a/src/libosmo-mgcp/mgcp_conn.c b/src/libosmo-mgcp/mgcp_conn.c index 8682e4b..68b6b39 100644 --- a/src/libosmo-mgcp/mgcp_conn.c +++ b/src/libosmo-mgcp/mgcp_conn.c @@ -115,7 +115,7 @@ mgcp_rtp_end_init(&conn_rtp->end); /* Make sure codec table is reset */ - mgcp_codec_reset_all(conn_rtp); + mgcp_codecset_reset(&conn_rtp->end.cset); return 0; } @@ -129,7 +129,7 @@ mgcp_conn_iuup_cleanup(conn_rtp); mgcp_rtp_end_free_port(&conn_rtp->end); rate_ctr_group_free(conn_rtp->ctrg); - mgcp_codec_reset_all(conn_rtp); + mgcp_codecset_reset(&conn_rtp->end.cset); } void mgcp_conn_watchdog_cb(void *data) diff --git a/src/libosmo-mgcp/mgcp_e1.c b/src/libosmo-mgcp/mgcp_e1.c index 428d652..8a64157 100644 --- a/src/libosmo-mgcp/mgcp_e1.c +++ b/src/libosmo-mgcp/mgcp_e1.c @@ -660,7 +660,7 @@ conn = mgcp_endp_get_conn_oldest(endp); OSMO_ASSERT(conn); conn_rtp = mgcp_conn_get_conn_rtp(conn); - codec = conn_rtp->end.codec; + codec = conn_rtp->end.cset.codec; OSMO_ASSERT(codec); /* Update codec information */ diff --git a/src/libosmo-mgcp/mgcp_iuup.c b/src/libosmo-mgcp/mgcp_iuup.c index a284070..468ea06 100644 --- a/src/libosmo-mgcp/mgcp_iuup.c +++ b/src/libosmo-mgcp/mgcp_iuup.c @@ -264,6 +264,7 @@ uint8_t *amr_data; struct rtp_hdr *rtp_hdr; struct amr_hdr *amr_hdr; + struct mgcp_rtp_codec *dst_codec; int rc; ft = osmo_amr_bytes_to_ft(msgb_l3len(msg)); @@ -275,7 +276,8 @@ } msgb_pull_to_l3(msg); - if (mgcp_codec_amr_is_octet_aligned(conn_rtp_dst->end.codec)) { + dst_codec = conn_rtp_dst->end.cset.codec; + if (mgcp_codec_amr_is_octet_aligned(dst_codec)) { LOGP(DLMGCP, LOGL_DEBUG, "Convert IuUP -> AMR OA: ft %d, len %d\n", ft, msgb_length(msg)); amr_hdr = (struct amr_hdr *) msgb_push(msg, sizeof(struct amr_hdr)); amr_hdr->cmr = 15; /* no change */ @@ -303,7 +305,7 @@ .extension = 0, .padding = 0, .version = 0, - .payload_type = conn_rtp_dst->end.codec->payload_type, + .payload_type = dst_codec->payload_type, .marker = 0, .sequence = frame_nr, .timestamp = 0, @@ -502,7 +504,7 @@ * ignored by the receiver, but still it's useful for debug purposes * to set it. Moreover, it seems ip.access nano3g produces much worse * audio output on the air side if timestamp is not set properly. */ - hdr->timestamp = osmo_htonl(mgcp_get_current_ts(rtp_end->codec->rate)); + hdr->timestamp = osmo_htonl(mgcp_get_current_ts(rtp_end->cset.codec->rate)); hdr->sequence = osmo_htons(rtp_state->alt_rtp_tx_sequence); hdr->ssrc = rtp_state->alt_rtp_tx_ssrc; rtp_state->alt_rtp_tx_sequence++; @@ -550,7 +552,7 @@ .extension = 0, .padding = 0, .version = 2, - .payload_type = conn_rtp_dst->end.codec->payload_type, + .payload_type = conn_rtp_dst->end.cset.codec->payload_type, .marker = 0, .sequence = 0, .timestamp = 0, @@ -645,6 +647,7 @@ struct rtp_hdr *rtph; int rc = -1; int iuup_length = 0; + struct mgcp_rtp_codec *src_codec; int8_t rfci; /* Tx RNL-DATA.req */ @@ -657,13 +660,14 @@ /* TODO: CMR handling & multiple frames handling */ - if (strcmp(conn_src_rtp->end.codec->subtype_name, "AMR") != 0) { + src_codec = conn_src_rtp->end.cset.codec; + if (strcmp(src_codec->subtype_name, "AMR") != 0) { LOG_CONN_RTP(conn_src_rtp, LOGL_ERROR, "Bridge RTP=>IuUP: Bridging src codec %s to IuUP AMR not supported\n", - conn_src_rtp->end.codec->subtype_name); + src_codec->subtype_name); goto free_ret; } - if (mgcp_codec_amr_is_octet_aligned(conn_src_rtp->end.codec)) { + if (mgcp_codec_amr_is_octet_aligned(src_codec)) { struct amr_hdr *amr_hdr = (struct amr_hdr *) msgb_data(msg); if (msgb_length(msg) < (sizeof(*amr_hdr))) { LOG_CONN_RTP(conn_src_rtp, LOGL_NOTICE, diff --git a/src/libosmo-mgcp/mgcp_network.c b/src/libosmo-mgcp/mgcp_network.c index 27cca09..476e3b1 100644 --- a/src/libosmo-mgcp/mgcp_network.c +++ b/src/libosmo-mgcp/mgcp_network.c @@ -332,7 +332,7 @@ osmo_sockaddr_ntop(&addr->u.sa, ipbuf), osmo_sockaddr_port(&addr->u.sa)); } else { - tsdelta = rtp_end->codec->rate * 20 / 1000; + tsdelta = rtp_end->cset.codec->rate * 20 / 1000; LOGPENDP(endp, DRTP, LOGL_NOTICE, "Fixed packet duration and last timestamp delta " "are not available, " @@ -496,11 +496,11 @@ } rtp_hdr = (struct rtp_hdr *)msgb_data(msg); - if (!conn_dst->end.codec) { + if (!conn_dst->end.cset.codec) { LOG_CONN_RTP(conn_dst, LOGL_NOTICE, "no codec set on destination connection!\n"); return -EINVAL; } - rtp_hdr->payload_type = (uint8_t) conn_dst->end.codec->payload_type; + rtp_hdr->payload_type = (uint8_t) conn_dst->end.cset.codec->payload_type; return 0; } @@ -523,7 +523,8 @@ uint32_t timestamp, ssrc; bool marker_bit; struct rtp_hdr *rtp_hdr; - int payload = rtp_end->codec->payload_type; + struct mgcp_rtp_codec *codec = rtp_end->cset.codec; + int payload = codec->payload_type; unsigned int len = msgb_length(msg); if (len < sizeof(*rtp_hdr)) @@ -532,7 +533,7 @@ rtp_hdr = (struct rtp_hdr *)msgb_data(msg); seq = ntohs(rtp_hdr->sequence); timestamp = ntohl(rtp_hdr->timestamp); - arrival_time = mgcp_get_current_ts(rtp_end->codec->rate); + arrival_time = mgcp_get_current_ts(codec->rate); ssrc = ntohl(rtp_hdr->ssrc); marker_bit = !!rtp_hdr->marker; transit = arrival_time - timestamp; @@ -560,7 +561,7 @@ osmo_sockaddr_port(&addr->u.sa)); if (state->packet_duration == 0) { state->packet_duration = - rtp_end->codec->rate * 20 / 1000; + codec->rate * 20 / 1000; LOGPENDP(endp, DRTP, LOGL_NOTICE, "fixed packet duration is not available, " "using fixed 20ms instead: %d from %s:%d\n", @@ -816,8 +817,8 @@ return; hdr->version = 2; - hdr->payload_type = rtp_end->codec->payload_type; - hdr->timestamp = osmo_htonl(mgcp_get_current_ts(rtp_end->codec->rate)); + hdr->payload_type = rtp_end->cset.codec->payload_type; + hdr->timestamp = osmo_htonl(mgcp_get_current_ts(rtp_end->cset.codec->rate)); hdr->sequence = osmo_htons(state->alt_rtp_tx_sequence); hdr->ssrc = state->alt_rtp_tx_ssrc; } @@ -1183,6 +1184,8 @@ osmo_sockaddr_port(&rtp_end->addr.u.sa), ntohs(rtp_end->rtcp_port) ); } else if (is_rtp) { + struct mgcp_rtp_codec *src_codec; + struct mgcp_rtp_codec *dst_codec; /* Make sure we have a valid RTP header, in cases where no RTP * header is present, we will generate one. */ gen_rtp_header(msg, rtp_end, rtp_state); @@ -1198,19 +1201,21 @@ if (addr) mgcp_patch_and_count(endp, rtp_state, rtp_end, addr, msg); + src_codec = conn_src->end.cset.codec; + dst_codec = conn_dst->end.cset.codec; 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); + } else if (mgcp_codec_amr_align_mode_is_indicated(dst_codec)) { + rc = amr_oa_bwe_convert(endp, msg, dst_codec->param.amr_octet_aligned); 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"); + dst_codec->param.amr_octet_aligned ? "octet-aligned" : "bandwidth-efficient"); msgb_free(msg); return rc; } } else if (rtp_end->rfc5993_hr_convert && - strcmp(conn_src->end.codec->subtype_name, "GSM-HR-08") == 0) { + strcmp(src_codec->subtype_name, "GSM-HR-08") == 0) { rc = rfc5993_hr_convert(endp, msg); if (rc < 0) { LOGPENDP(endp, DRTP, LOGL_ERROR, "Error while converting to GSM-HR-08\n"); @@ -1434,7 +1439,7 @@ } /* Forward to E1 */ - return mgcp_e1_send_rtp(conn->endp, conn_src->end.codec, msg); + return mgcp_e1_send_rtp(conn->endp, conn_src->end.cset.codec, msg); } /*! cleanup an endpoint when a connection on an RTP bridge endpoint is removed. @@ -1556,18 +1561,18 @@ /* Handle AMR frame format conversion (octet-aligned vs. bandwith-efficient) */ if (mc->proto == MGCP_PROTO_RTP - && conn_src->end.codec - && mgcp_codec_amr_align_mode_is_indicated(conn_src->end.codec)) { + && conn_src->end.cset.codec + && mgcp_codec_amr_align_mode_is_indicated(conn_src->end.cset.codec)) { /* 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) goto out_free; - if (((bool)oa) != conn_src->end.codec->param.amr_octet_aligned) { + if (((bool)oa) != conn_src->end.cset.codec->param.amr_octet_aligned) { 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), conn_src->end.cset.codec->param.amr_octet_aligned, oa); goto out_free; } } diff --git a/src/libosmo-mgcp/mgcp_osmux.c b/src/libosmo-mgcp/mgcp_osmux.c index 0c285dd..31f128a 100644 --- a/src/libosmo-mgcp/mgcp_osmux.c +++ b/src/libosmo-mgcp/mgcp_osmux.c @@ -651,7 +651,7 @@ osmux_xfrm_output_set_rtp_ssrc(conn->osmux.out, (conn->osmux.remote_cid * rtp_ssrc_winlen) + (random() % rtp_ssrc_winlen)); - osmux_xfrm_output_set_rtp_pl_type(conn->osmux.out, conn->end.codec->payload_type); + osmux_xfrm_output_set_rtp_pl_type(conn->osmux.out, conn->end.cset.codec->payload_type); osmux_xfrm_output_set_tx_cb(conn->osmux.out, scheduled_from_osmux_tx_rtp_cb, conn); diff --git a/src/libosmo-mgcp/mgcp_protocol.c b/src/libosmo-mgcp/mgcp_protocol.c index eef9d5f..45f2c4f 100644 --- a/src/libosmo-mgcp/mgcp_protocol.c +++ b/src/libosmo-mgcp/mgcp_protocol.c @@ -744,19 +744,20 @@ const struct mgcp_rtp_end *rtp) { int f = 0; + struct mgcp_rtp_codec *codec = rtp->cset.codec; /* Get the number of frames per channel and packet */ if (rtp->frames_per_packet) f = rtp->frames_per_packet; - else if (rtp->packet_duration_ms && rtp->codec->frame_duration_num) { - int den = 1000 * rtp->codec->frame_duration_num; - f = (rtp->packet_duration_ms * rtp->codec->frame_duration_den + + else if (rtp->packet_duration_ms && codec->frame_duration_num) { + int den = 1000 * codec->frame_duration_num; + f = (rtp->packet_duration_ms * codec->frame_duration_den + den / 2) / den; } - return rtp->codec->rate * f * rtp->codec->frame_duration_num / - rtp->codec->frame_duration_den; + return codec->rate * f * codec->frame_duration_num / + codec->frame_duration_den; } /*! Initializes osmux socket if not yet initialized. Parses Osmux CID from MGCP line. @@ -784,6 +785,7 @@ struct mgcp_endpoint *endp = rq->endp; struct mgcp_conn *conn_dst; struct mgcp_conn_rtp *conn_dst_rtp; + struct mgcp_rtp_codecset *cset = &conn->end.cset; int rc; char *cmd; @@ -797,7 +799,7 @@ if (have_sdp) { /* If we have SDP, we ignore the local connection options and * use only the SDP information. */ - mgcp_codec_reset_all(conn); + mgcp_codecset_reset(cset); rc = mgcp_parse_sdp_data(endp, conn, rq->pdata); if (rc != 0) { LOGPCONN(conn->conn, DLMGCP, LOGL_ERROR, @@ -809,19 +811,19 @@ } else if (endp->local_options.codec) { /* When no SDP is available, we use the codec information from * the local connection options (if present) */ - mgcp_codec_reset_all(conn); - rc = mgcp_codec_add(conn, PTYPE_UNDEFINED, endp->local_options.codec, NULL); + mgcp_codecset_reset(cset); + rc = mgcp_codecset_add_codec(cset, PTYPE_UNDEFINED, endp->local_options.codec, NULL); if (rc != 0) goto error; } /* Make sure we always set a sane default codec */ - if (conn->end.codecs_assigned == 0) { + if (cset->codecs_assigned == 0) { /* When SDP and/or LCO did not supply any codec information, * than it makes sense to pick a sane default: (payload-type 0, * PCMU), see also: OS#2658 */ - mgcp_codec_reset_all(conn); - rc = mgcp_codec_add(conn, 0, NULL, NULL); + mgcp_codecset_reset(cset); + rc = mgcp_codecset_add_codec(cset, 0, NULL, NULL); if (rc != 0) goto error; } @@ -834,7 +836,7 @@ conn_dst_rtp = NULL; /* Make codec decision */ - if (mgcp_codec_decide(conn, conn_dst_rtp) != 0) + if (mgcp_codecset_decide(&conn->end.cset, conn_dst_rtp ? &conn_dst_rtp->end.cset : NULL) != 0) goto error; return 0; @@ -1074,7 +1076,7 @@ /* Handle codec information and decide for a suitable codec */ rc = handle_codec_info(conn_rtp, rq, have_sdp, true); - mgcp_codec_summary(conn_rtp); + mgcp_codecset_summary(&conn_rtp->end.cset, mgcp_conn_dump(conn)); if (rc) { error_code = rc; rate_ctr_inc(rate_ctr_group_get_ctr(rate_ctrs, MGCP_CRCX_FAIL_CODEC_NEGOTIATION)); @@ -1082,7 +1084,8 @@ } /* Upgrade the conn type RTP_DEFAULT->RTP_IUUP if needed based on requested codec: */ /* TODO: "codec" probably needs to be moved from endp to conn */ - if (conn_rtp->type == MGCP_RTP_DEFAULT && strcmp(conn_rtp->end.codec->subtype_name, "VND.3GPP.IUFP") == 0) { + if (conn_rtp->type == MGCP_RTP_DEFAULT && + strcmp(conn_rtp->end.cset.codec->subtype_name, "VND.3GPP.IUFP") == 0) { rc = mgcp_conn_iuup_init(conn_rtp); } @@ -1285,14 +1288,15 @@ /* Handle codec information and decide for a suitable codec */ rc = handle_codec_info(conn_rtp, rq, have_sdp, false); - mgcp_codec_summary(conn_rtp); + mgcp_codecset_summary(&conn_rtp->end.cset, mgcp_conn_dump(conn)); if (rc) { error_code = rc; goto error3; } /* Upgrade the conn type RTP_DEFAULT->RTP_IUUP if needed based on requested codec: */ /* TODO: "codec" probably needs to be moved from endp to conn */ - if (conn_rtp->type == MGCP_RTP_DEFAULT && strcmp(conn_rtp->end.codec->subtype_name, "VND.3GPP.IUFP") == 0) + if (conn_rtp->type == MGCP_RTP_DEFAULT && + strcmp(conn_rtp->end.cset.codec->subtype_name, "VND.3GPP.IUFP") == 0) rc = mgcp_conn_iuup_init(conn_rtp); if (mgcp_conn_rtp_is_osmux(conn_rtp)) { diff --git a/src/libosmo-mgcp/mgcp_sdp.c b/src/libosmo-mgcp/mgcp_sdp.c index 77825f6..4d13388 100644 --- a/src/libosmo-mgcp/mgcp_sdp.c +++ b/src/libosmo-mgcp/mgcp_sdp.c @@ -422,7 +422,7 @@ /* 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); + rc = mgcp_codecset_add_codec(&conn->end.cset, codecs[i].payload_type, codecs[i].map_line, codec_param); if (rc < 0) LOGPENDP(endp, DLMGCP, LOGL_NOTICE, "failed to add codec\n"); } @@ -437,8 +437,8 @@ LOGPC(DLMGCP, LOGL_NOTICE, "none"); for (i = 0; i < codecs_used; i++) { LOGPC(DLMGCP, LOGL_NOTICE, "%d=%s", - rtp->codecs[i].payload_type, - strlen(rtp->codecs[i].subtype_name) ? rtp->codecs[i].subtype_name : "unknown"); + rtp->cset.codecs[i].payload_type, + strlen(rtp->cset.codecs[i].subtype_name) ? rtp->cset.codecs[i].subtype_name : "unknown"); LOGPC(DLMGCP, LOGL_NOTICE, " "); } LOGPC(DLMGCP, LOGL_NOTICE, "\n"); @@ -542,7 +542,7 @@ OSMO_ASSERT(sdp); OSMO_ASSERT(addr); - codec = conn->end.codec; + codec = conn->end.cset.codec; audio_name = codec->audio_name; payload_type = codec->payload_type; diff --git a/src/libosmo-mgcp/mgcp_vty.c b/src/libosmo-mgcp/mgcp_vty.c index 8e0360f..e90ceaa 100644 --- a/src/libosmo-mgcp/mgcp_vty.c +++ b/src/libosmo-mgcp/mgcp_vty.c @@ -165,7 +165,7 @@ { struct mgcp_rtp_state *state = &conn->state; struct mgcp_rtp_end *end = &conn->end; - struct mgcp_rtp_codec *codec = end->codec; + struct mgcp_rtp_codec *codec = end->cset.codec; struct rate_ctr *tx_packets, *tx_bytes; struct rate_ctr *rx_packets, *rx_bytes; struct rate_ctr *dropped_packets; diff --git a/tests/mgcp/mgcp_test.c b/tests/mgcp/mgcp_test.c index c257d6a..9aa7326 100644 --- a/tests/mgcp/mgcp_test.c +++ b/tests/mgcp/mgcp_test.c @@ -1023,14 +1023,14 @@ fprintf(stderr, "endpoint:%s: " "payload type %d (expected %d)\n", last_endpoint, - conn->end.codec->payload_type, t->ptype); + conn->end.cset.codec->payload_type, t->ptype); if (t->ptype != PTYPE_IGNORE) - OSMO_ASSERT(conn->end.codec->payload_type == + OSMO_ASSERT(conn->end.cset.codec->payload_type == t->ptype); /* Reset them again for next test */ - conn->end.codec->payload_type = PTYPE_NONE; + conn->end.cset.codec->payload_type = PTYPE_NONE; } } @@ -1466,8 +1466,8 @@ rtp = &conn->end; - OSMO_ASSERT(mgcp_codec_add(conn, PTYPE_UNDEFINED, "AMR/8000/1", NULL) == 0); - rtp->codec = &rtp->codecs[0]; + OSMO_ASSERT(mgcp_codecset_add_codec(&conn->end.cset, PTYPE_UNDEFINED, "AMR/8000/1", NULL) == 0); + rtp->cset.codec = &rtp->cset.codecs[0]; for (i = 0; i < ARRAY_SIZE(test_rtp_packets1); ++i) { struct rtp_packet_info *info = test_rtp_packets1 + i; @@ -1550,7 +1550,7 @@ OSMO_ASSERT(endp); conn = mgcp_endp_get_conn_rtp(endp, conn_id); OSMO_ASSERT(conn); - OSMO_ASSERT(conn->end.codec->payload_type == 18); + OSMO_ASSERT(conn->end.cset.codec->payload_type == 18); /* Allocate 2@mgw with three codecs, last one ignored */ last_endpoint[0] = '\0'; @@ -1566,7 +1566,7 @@ OSMO_ASSERT(endp); conn = mgcp_endp_get_conn_rtp(endp, conn_id); OSMO_ASSERT(conn); - OSMO_ASSERT(conn->end.codec->payload_type == 18); + OSMO_ASSERT(conn->end.cset.codec->payload_type == 18); /* Allocate 3@mgw with no codecs, check for PT == 0 */ /* Note: It usually makes no sense to leave the payload type list @@ -1587,7 +1587,7 @@ OSMO_ASSERT(endp); conn = mgcp_endp_get_conn_rtp(endp, conn_id); OSMO_ASSERT(conn); - OSMO_ASSERT(conn->end.codec->payload_type == 0); + OSMO_ASSERT(conn->end.cset.codec->payload_type == 0); /* Allocate 4@mgw with a single codec */ last_endpoint[0] = '\0'; @@ -1603,7 +1603,7 @@ OSMO_ASSERT(endp); conn = mgcp_endp_get_conn_rtp(endp, conn_id); OSMO_ASSERT(conn); - OSMO_ASSERT(conn->end.codec->payload_type == 18); + OSMO_ASSERT(conn->end.cset.codec->payload_type == 18); /* Allocate 5@mgw and let osmo-mgw pick a codec from the list */ last_endpoint[0] = '\0'; @@ -1619,7 +1619,7 @@ OSMO_ASSERT(endp); conn = mgcp_endp_get_conn_rtp(endp, conn_id); OSMO_ASSERT(conn); - OSMO_ASSERT(conn->end.codec->payload_type == 0); + OSMO_ASSERT(conn->end.cset.codec->payload_type == 0); inp = create_msg(MDCX_NAT_DUMMY, conn_id); last_endpoint[0] = '\0'; @@ -1631,7 +1631,7 @@ OSMO_ASSERT(endp); conn = mgcp_endp_get_conn_rtp(endp, conn_id); OSMO_ASSERT(conn); - OSMO_ASSERT(conn->end.codec->payload_type == 3); + OSMO_ASSERT(conn->end.cset.codec->payload_type == 3); OSMO_ASSERT(osmo_sockaddr_port(&conn->end.addr.u.sa) == 16434); memset(&addr, 0, sizeof(addr)); inet_aton("8.8.8.8", &addr); @@ -1662,7 +1662,7 @@ OSMO_ASSERT(endp); conn = mgcp_endp_get_conn_rtp(endp, conn_id); OSMO_ASSERT(conn); - OSMO_ASSERT(conn->end.codec->payload_type == 0); + OSMO_ASSERT(conn->end.cset.codec->payload_type == 0); mgcp_endpoints_release(trunk); talloc_free(cfg); @@ -2195,7 +2195,7 @@ int payload_type_conn_dst; printf(" - mgcp_codec_decide(&conn[%u], &conn[%u]):\n", index_conn_src, index_conn_dst); - if (mgcp_codec_decide(&conn[index_conn_src], &conn[index_conn_dst]) != 0) { + if (mgcp_codecset_decide(&conn[index_conn_src].end.cset, &conn[index_conn_dst].end.cset) != 0) { if (expect->payload_type_map[index_conn_src] == -EINVAL && expect->payload_type_map[index_conn_dst] == -EINVAL) printf(" codec decision failed (expected)!\n"); @@ -2205,19 +2205,19 @@ } } else { printf(" Codec decision result:\n"); - if (conn[index_conn_src].end.codec) { - payload_type_conn_src = conn[index_conn_src].end.codec->payload_type; + if (conn[index_conn_src].end.cset.codec) { + payload_type_conn_src = conn[index_conn_src].end.cset.codec->payload_type; printf(" conn[%u]: codec:%s, pt:%d\n", - index_conn_src, conn[index_conn_src].end.codec->subtype_name, payload_type_conn_src); + index_conn_src, conn[index_conn_src].end.cset.codec->subtype_name, payload_type_conn_src); } else { payload_type_conn_src = -EINVAL; printf(" conn[%u]: codec:none, pt:none\n", index_conn_src); } - if (conn[index_conn_dst].end.codec) { - payload_type_conn_dst = conn[index_conn_dst].end.codec->payload_type; + if (conn[index_conn_dst].end.cset.codec) { + payload_type_conn_dst = conn[index_conn_dst].end.cset.codec->payload_type; printf(" conn[%u]: codec:%s, pt:%d\n", - index_conn_dst, conn[index_conn_dst].end.codec->subtype_name, + index_conn_dst, conn[index_conn_dst].end.cset.codec->subtype_name, payload_type_conn_dst); } else { payload_type_conn_dst = -EINVAL; @@ -2265,8 +2265,8 @@ if (!codec->audio_name) break; - rc = mgcp_codec_add(&conn[conn_i], codec->payload_type, codec->audio_name, - codec->param); + rc = mgcp_codecset_add_codec(&conn[conn_i].end.cset, codec->payload_type, + codec->audio_name, codec->param); printf(" %2d: %3d %s%s -> rc=%d\n", c, codec->payload_type, codec->audio_name, codec->param ? -- To view, visit https://gerrit.osmocom.org/c/osmo-mgw/+/39215?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings?usp=email Gerrit-MessageType: newchange Gerrit-Project: osmo-mgw Gerrit-Branch: master Gerrit-Change-Id: Id7db7ab01d56b7fa2415123b604375e48c82ab25 Gerrit-Change-Number: 39215 Gerrit-PatchSet: 1 Gerrit-Owner: pespin &lt;pespin(a)sysmocom.de&gt;