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

[S] Change in osmo-bts[master]: trx: detect UL SID in EFR just like in FR
by laforge 29 Mar '23

29 Mar '23

laforge has submitted this change. ( https://gerrit.osmocom.org/c/osmo-bts/+/32095 ) Change subject: trx: detect UL SID in EFR just like in FR ...................................................................... trx: detect UL SID in EFR just like in FR The TCH/F Rx code in osmo-bts-trx uses osmo_fr_check_sid() to detect when the MS sends SID, and passes the flag to lchan_set_marker(). However, equivalent logic was missing for EFR, as until recently there was no EFR SID check function in libosmocodec. Now that we have osmo_efr_check_sid(), use it. Change-Id: Ib043e00dbf92145c2a6c32f6365517244472a922 --- M TODO-RELEASE M src/osmo-bts-trx/sched_lchan_tchf.c 2 files changed, 18 insertions(+), 0 deletions(-) Approvals: fixeria: Looks good to me, approved Jenkins Builder: Verified diff --git a/TODO-RELEASE b/TODO-RELEASE index 1833716..88ad00d 100644 --- a/TODO-RELEASE +++ b/TODO-RELEASE @@ -7,4 +7,5 @@ # If any interfaces have been added since the last public release: c:r:a + 1. # If any interfaces have been removed or changed since the last public release: c:r:0. #library what description / commit summary line +libosmocodec >1.8.0 osmo_efr_check_sid() new function libosmogsm >1.8.0 <osmocom/gsm/protocol/gsm_44_060.h> added diff --git a/src/osmo-bts-trx/sched_lchan_tchf.c b/src/osmo-bts-trx/sched_lchan_tchf.c index eff0f63..256125b 100644 --- a/src/osmo-bts-trx/sched_lchan_tchf.c +++ b/src/osmo-bts-trx/sched_lchan_tchf.c @@ -150,6 +150,8 @@ break; case GSM48_CMODE_SPEECH_EFR: /* EFR */ rc = gsm0503_tch_fr_decode(tch_data, *bursts_p, 1, 1, &n_errors, &n_bits_total); + if (rc == GSM_EFR_BYTES) /* only for valid *speech* frames */ + lchan_set_marker(osmo_efr_check_sid(tch_data, rc), lchan); /* DTXu */ break; case GSM48_CMODE_SPEECH_AMR: /* AMR */ /* the first FN 0,8,17 defines that CMI is included in frame, -- To view, visit https://gerrit.osmocom.org/c/osmo-bts/+/32095 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-bts Gerrit-Branch: master Gerrit-Change-Id: Ib043e00dbf92145c2a6c32f6365517244472a922 Gerrit-Change-Number: 32095 Gerrit-PatchSet: 3 Gerrit-Owner: falconia <falcon(a)freecalypso.org> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: fixeria <vyanitskiy(a)sysmocom.de> Gerrit-Reviewer: laforge <laforge(a)osmocom.org> Gerrit-Reviewer: pespin <pespin(a)sysmocom.de> Gerrit-MessageType: merged

1 0

[S] Change in osmo-bts[master]: sysmo: fix handling of SID in EFR
by laforge 29 Mar '23

29 Mar '23

laforge has submitted this change. ( https://gerrit.osmocom.org/c/osmo-bts/+/32096 ) ( 1 is the latest approved patch-set. No files were changed between the latest approved patch-set and the submitted one. )Change subject: sysmo: fix handling of SID in EFR ...................................................................... sysmo: fix handling of SID in EFR Handling of SID in EFR mode was broken in osmo-bts-sysmo. l1_to_rtppayload_efr(), the function for UL Rx, was using completely bogus logic (passing bits in ETSI TS 101 318 EFR format to an AMR decoding function), whereas l1if_tch_encode(), the function for DL Tx, had missing SID logic for EFR while supporting SID detection for all other codecs. The fix is to use the new osmo_efr_check_sid() function in libosmocodec. Change-Id: Ia56c1bb7432968685110456961d24a907b0a201f --- M src/osmo-bts-sysmo/tch.c 1 file changed, 23 insertions(+), 7 deletions(-) Approvals: laforge: Looks good to me, approved pespin: Looks good to me, but someone else must approve Jenkins Builder: Verified diff --git a/src/osmo-bts-sysmo/tch.c b/src/osmo-bts-sysmo/tch.c index e901d47..832991f 100644 --- a/src/osmo-bts-sysmo/tch.c +++ b/src/osmo-bts-sysmo/tch.c @@ -131,12 +131,8 @@ cur[0] |= 0xC0; #endif /* USE_L1_RTP_MODE */ - enum osmo_amr_type ft; - enum osmo_amr_quality bfi; - uint8_t cmr; - int8_t sti, cmi; - osmo_amr_rtp_dec(l1_payload, payload_len, &cmr, &cmi, &ft, &bfi, &sti); - lchan_set_marker(ft == AMR_GSM_EFR_SID, lchan); + + lchan_set_marker(osmo_efr_check_sid(l1_payload, payload_len), lchan); return msg; } @@ -403,7 +399,10 @@ *payload_type = GsmL1_TchPlType_Efr; rc = rtppayload_to_l1_efr(l1_payload, rtp_pl, rtp_pl_len); - /* FIXME: detect and save EFR SID */ + if (rc && lchan->ts->trx->bts->dtxd) + is_sid = osmo_efr_check_sid(rtp_pl, rtp_pl_len); + if (is_sid) + dtx_cache_payload(lchan, rtp_pl, rtp_pl_len, fn, -1); break; #endif case GSM48_CMODE_SPEECH_AMR: -- To view, visit https://gerrit.osmocom.org/c/osmo-bts/+/32096 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-bts Gerrit-Branch: master Gerrit-Change-Id: Ia56c1bb7432968685110456961d24a907b0a201f Gerrit-Change-Number: 32096 Gerrit-PatchSet: 3 Gerrit-Owner: falconia <falcon(a)freecalypso.org> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: laforge <laforge(a)osmocom.org> Gerrit-Reviewer: pespin <pespin(a)sysmocom.de> Gerrit-MessageType: merged

1 0

[S] Change in libosmocore[master]: gsm: Add missing TS 24.008 SM layer IEs
by laforge 29 Mar '23

29 Mar '23

Attention is currently required from: pespin. laforge has posted comments on this change. ( https://gerrit.osmocom.org/c/libosmocore/+/32109 ) Change subject: gsm: Add missing TS 24.008 SM layer IEs ...................................................................... Patch Set 2: Code-Review+2 -- To view, visit https://gerrit.osmocom.org/c/libosmocore/+/32109 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: libosmocore Gerrit-Branch: master Gerrit-Change-Id: Iec0dbf617c8d0f2c8c44156d936244cedda9b303 Gerrit-Change-Number: 32109 Gerrit-PatchSet: 2 Gerrit-Owner: pespin <pespin(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: laforge <laforge(a)osmocom.org> Gerrit-Attention: pespin <pespin(a)sysmocom.de> Gerrit-Comment-Date: Wed, 29 Mar 2023 11:58:45 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

[S] Change in libosmo-gprs[master]: gmm: Several fixes to GMMSM prim alloc functions
by laforge 29 Mar '23

29 Mar '23

Attention is currently required from: pespin. laforge has posted comments on this change. ( https://gerrit.osmocom.org/c/libosmo-gprs/+/32101 ) Change subject: gmm: Several fixes to GMMSM prim alloc functions ...................................................................... Patch Set 2: Code-Review+1 -- To view, visit https://gerrit.osmocom.org/c/libosmo-gprs/+/32101 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: libosmo-gprs Gerrit-Branch: master Gerrit-Change-Id: Ie49cb448806101a24c58a85f7073e13baccada5b Gerrit-Change-Number: 32101 Gerrit-PatchSet: 2 Gerrit-Owner: pespin <pespin(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: laforge <laforge(a)osmocom.org> Gerrit-Attention: pespin <pespin(a)sysmocom.de> Gerrit-Comment-Date: Wed, 29 Mar 2023 11:58:37 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

[S] Change in osmo-mgw[master]: Revert "mgcp_codec: do not differentiate between oa and bwe when comp...
by laforge 29 Mar '23

29 Mar '23

laforge has submitted this change. ( https://gerrit.osmocom.org/c/osmo-mgw/+/27391 ) Change subject: Revert "mgcp_codec: do not differentiate between oa and bwe when comparing codec" ...................................................................... Revert "mgcp_codec: do not differentiate between oa and bwe when comparing codec" This reverts commit e0058b7207e022b698aea10f96cc7c0b1209058a. The reason for this revert is that the solution in the reverted patch does not cover a situation where the other side announces both payload formats at the same time. It could be that the end facing to a transit network announces both formats under two different payload types. In this case no conversion would be necessary. Depending on the input format the output would be send to the transit network under the payload type that matches and no conversion would happen at all. This revert re-intruduces the problem that was fixed in the patch before. Therefore it must be merged together with the follow up patch (Ifbd201a2749009a4644a29bd77e1d0fc0c124a9d) that contains the proper fix. Change-Id: I0b2854ef2397f38606fab3425be586a3d0ca27d1 Related: OS#5461 --- M src/libosmo-mgcp/mgcp_codec.c M tests/mgcp/mgcp_test.c M tests/mgcp/mgcp_test.ok 3 files changed, 39 insertions(+), 7 deletions(-) Approvals: Jenkins Builder: Verified pespin: Looks good to me, but someone else must approve laforge: Looks good to me, approved diff --git a/src/libosmo-mgcp/mgcp_codec.c b/src/libosmo-mgcp/mgcp_codec.c index 1a8d26a..c287b7e 100644 --- a/src/libosmo-mgcp/mgcp_codec.c +++ b/src/libosmo-mgcp/mgcp_codec.c @@ -390,10 +390,10 @@ return false; if (strcmp(codec_a->subtype_name, codec_b->subtype_name)) return false; - - /* Note: AMR allows to set the RTP payload format to octet-aligned or bandwith-efficient (octet-aligned=0) - * via SDP. This difference concerns payload format only, but not the actual codec. It is not a difference - * within the meaning of this function. */ + if (!strcmp(codec_a->subtype_name, "AMR")) { + if (mgcp_codec_amr_is_octet_aligned(codec_a) != mgcp_codec_amr_is_octet_aligned(codec_b)) + return false; + } return true; } diff --git a/tests/mgcp/mgcp_test.c b/tests/mgcp/mgcp_test.c index dfaba71..444e07a 100644 --- a/tests/mgcp/mgcp_test.c +++ b/tests/mgcp/mgcp_test.c @@ -1897,13 +1897,16 @@ .codecs = { { { 111, "AMR/8000", &amr_param_octet_aligned_true, }, + { 112, "AMR/8000", &amr_param_octet_aligned_false, }, }, { { 122, "AMR/8000", &amr_param_octet_aligned_false, }, + { 121, "AMR/8000", &amr_param_octet_aligned_true, }, }, }, .expect = { - { .payload_type_map = {111, 122}, }, + { .payload_type_map = {111, 121}, }, + { .payload_type_map = {112, 122} }, { .end = true }, }, }, @@ -1912,13 +1915,15 @@ .codecs = { { { 111, "AMR/8000", &amr_param_octet_aligned_true, }, + { 112, "AMR/8000", &amr_param_octet_aligned_false, }, }, { { 122, "AMR/8000", &amr_param_octet_aligned_unset, }, }, }, .expect = { - { .payload_type_map = {111, 122}, }, + { .payload_type_map = {111, -EINVAL}, }, + { .payload_type_map = {112, 122} }, { .end = true }, }, }, @@ -1927,13 +1932,15 @@ .codecs = { { { 111, "AMR/8000", &amr_param_octet_aligned_true, }, + { 112, "AMR/8000", &amr_param_octet_aligned_false, }, }, { { 122, "AMR/8000", NULL, }, }, }, .expect = { - { .payload_type_map = {111, 122}, }, + { .payload_type_map = {111, -EINVAL}, }, + { .payload_type_map = {112, 122} }, { .end = true }, }, }, diff --git a/tests/mgcp/mgcp_test.ok b/tests/mgcp/mgcp_test.ok index 5b8d558..94fada3 100644 --- a/tests/mgcp/mgcp_test.ok +++ b/tests/mgcp/mgcp_test.ok Binary files differ -- To view, visit https://gerrit.osmocom.org/c/osmo-mgw/+/27391 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-mgw Gerrit-Branch: master Gerrit-Change-Id: I0b2854ef2397f38606fab3425be586a3d0ca27d1 Gerrit-Change-Number: 27391 Gerrit-PatchSet: 3 Gerrit-Owner: dexter <pmaier(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: laforge <laforge(a)osmocom.org> Gerrit-Reviewer: msuraev <msuraev(a)sysmocom.de> Gerrit-Reviewer: pespin <pespin(a)sysmocom.de> Gerrit-MessageType: merged

1 0

[M] Change in osmo-mgw[master]: mgcp_codec: fix oa/bwe comparison in mgcp_codec_pt_translate()
by laforge 29 Mar '23

29 Mar '23

laforge has submitted this change. ( https://gerrit.osmocom.org/c/osmo-mgw/+/27392 ) Change subject: mgcp_codec: fix oa/bwe comparison in mgcp_codec_pt_translate() ...................................................................... mgcp_codec: fix oa/bwe comparison in mgcp_codec_pt_translate() The function mgcp_codec_pt_translate is very strict when comparing the codecs to each other to find a matching payload type number to be used on the egress side. This poses a problem when one side uses AMR in bandwith-efficient, while the other side uses AMR in octet-aligned payload type format. To the pt translate function the difference in the payload format will appear as if the codec were different and eventually the payload type number cannot be translated. since osmo-mgw offers conversion between the payload type format it would be no problem to ignore the payload type format when making the translation decision. The only exception here would be if one side would announce AMR two times, the first time with octet-aligned and the second time with bandwith-efficient format. Then we would have to use the payload type number from the exact match. (and skip any formatconversion) To archive such an optimized decision we will first go through the codec lists and perform an exact match. If we don't get a match we go through the codec lists a second time, but this time we ignore the payload format. Change-Id: Ifbd201a2749009a4644a29bd77e1d0fc0c124a9d Related: OS#5461 --- M src/libosmo-mgcp/mgcp_codec.c M tests/mgcp/mgcp_test.c M tests/mgcp/mgcp_test.ok 3 files changed, 119 insertions(+), 14 deletions(-) Approvals: Jenkins Builder: Verified pespin: Looks good to me, but someone else must approve laforge: Looks good to me, approved diff --git a/src/libosmo-mgcp/mgcp_codec.c b/src/libosmo-mgcp/mgcp_codec.c index c287b7e..ccb5d77 100644 --- a/src/libosmo-mgcp/mgcp_codec.c +++ b/src/libosmo-mgcp/mgcp_codec.c @@ -376,10 +376,15 @@ return codec->param.amr_octet_aligned; } -/* Compare two codecs, all parameters must match up, except for the payload type - * number. */ +/* Compare two codecs, all parameters must match up */ static bool codecs_same(struct mgcp_rtp_codec *codec_a, 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 + * payload type numbers. Also the audio_name is not checked since it is already parsed into subtype_name, rate, + * and channels, which are checked. */ + if (strcmp(codec_a->subtype_name, codec_b->subtype_name)) + return false; if (codec_a->rate != codec_b->rate) return false; if (codec_a->channels != codec_b->channels) @@ -388,9 +393,11 @@ return false; if (codec_a->frame_duration_den != codec_b->frame_duration_den) return false; - if (strcmp(codec_a->subtype_name, codec_b->subtype_name)) - return false; - if (!strcmp(codec_a->subtype_name, "AMR")) { + + /* AMR payload may be formatted in two different payload formats, it is still the same codec but since the + * formatting of the payload is different, conversation is required, so we must treat it as a different + * codec here. */ + if (strcmp(codec_a->subtype_name, "AMR") == 0) { if (mgcp_codec_amr_is_octet_aligned(codec_a) != mgcp_codec_amr_is_octet_aligned(codec_b)) return false; } @@ -398,6 +405,26 @@ return true; } +/* 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) +{ + /* 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. + * Therefore we must insist on equal codecs but still allow different payload formatting. */ + if (strcmp(codec_a->subtype_name, codec_b->subtype_name)) + return false; + if (codec_a->rate != codec_b->rate) + return false; + if (codec_a->channels != codec_b->channels) + return false; + if (codec_a->frame_duration_num != codec_b->frame_duration_num) + return false; + if (codec_a->frame_duration_den != codec_b->frame_duration_den) + return false; + + return true; +} + /*! Translate a given payload type number that belongs to the packet of a * source connection to the equivalent payload type number that matches the * configuration of a destination connection. @@ -429,8 +456,8 @@ if (!codec_src) return -EINVAL; - /* Use the codec information from the source and try to find the - * equivalent of it on the destination side */ + /* 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_dst->codecs_assigned; OSMO_ASSERT(codecs_assigned <= MGCP_MAX_CODECS); for (i = 0; i < codecs_assigned; i++) { @@ -439,6 +466,18 @@ break; } } + + /* 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) */ + if (!codec_dst) { + for (i = 0; i < codecs_assigned; i++) { + if (codecs_convertible(codec_src, &rtp_dst->codecs[i])) { + codec_dst = &rtp_dst->codecs[i]; + break; + } + } + } + if (!codec_dst) return -EINVAL; diff --git a/tests/mgcp/mgcp_test.c b/tests/mgcp/mgcp_test.c index 444e07a..8f081a9 100644 --- a/tests/mgcp/mgcp_test.c +++ b/tests/mgcp/mgcp_test.c @@ -1851,6 +1851,7 @@ { .payload_type_map = {96, 97}, }, { .payload_type_map = {97, 96}, }, { .payload_type_map = {0, 0}, }, + { .payload_type_map = {123, -EINVAL} }, { .end = true }, }, }, @@ -1907,31 +1908,63 @@ .expect = { { .payload_type_map = {111, 121}, }, { .payload_type_map = {112, 122} }, + { .payload_type_map = {123, -EINVAL} }, { .end = true }, }, }, { - .descr = "test AMR with missing octet-aligned settings (defaults to 0)", + .descr = "test AMR with missing octet-aligned settings (oa <-> unset)", .codecs = { { { 111, "AMR/8000", &amr_param_octet_aligned_true, }, - { 112, "AMR/8000", &amr_param_octet_aligned_false, }, }, { { 122, "AMR/8000", &amr_param_octet_aligned_unset, }, }, }, .expect = { - { .payload_type_map = {111, -EINVAL}, }, - { .payload_type_map = {112, 122} }, + { .payload_type_map = {111, 122}, }, + { .payload_type_map = {55, -EINVAL}, }, { .end = true }, }, }, { - .descr = "test AMR with NULL param (defaults to 0)", + .descr = "test AMR with missing octet-aligned settings (bwe <-> unset)", .codecs = { { - { 111, "AMR/8000", &amr_param_octet_aligned_true, }, + { 111, "AMR/8000", &amr_param_octet_aligned_false, }, + }, + { + { 122, "AMR/8000", &amr_param_octet_aligned_unset, }, + }, + }, + .expect = { + { .payload_type_map = {111, 122}, }, + { .payload_type_map = {55, -EINVAL}, }, + { .end = true }, + }, + }, + { + .descr = "test AMR with NULL param (oa <-> null)", + .codecs = { + { + { 112, "AMR/8000", &amr_param_octet_aligned_true, }, + }, + { + { 122, "AMR/8000", NULL, }, + }, + }, + .expect = { + /* Note: Both 111, anbd 112 will translate to 122. The translation from 112 */ + { .payload_type_map = {112, 122} }, + { .payload_type_map = {55, -EINVAL}, }, + { .end = true }, + }, + }, + { + .descr = "test AMR with NULL param (bwe <-> null)", + .codecs = { + { { 112, "AMR/8000", &amr_param_octet_aligned_false, }, }, { @@ -1939,8 +1972,9 @@ }, }, .expect = { - { .payload_type_map = {111, -EINVAL}, }, + /* Note: Both 111, anbd 112 will translate to 122. The translation from 112 */ { .payload_type_map = {112, 122} }, + { .payload_type_map = {55, -EINVAL}, }, { .end = true }, }, }, diff --git a/tests/mgcp/mgcp_test.ok b/tests/mgcp/mgcp_test.ok index 94fada3..8b3b3d1 100644 --- a/tests/mgcp/mgcp_test.ok +++ b/tests/mgcp/mgcp_test.ok Binary files differ -- To view, visit https://gerrit.osmocom.org/c/osmo-mgw/+/27392 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-mgw Gerrit-Branch: master Gerrit-Change-Id: Ifbd201a2749009a4644a29bd77e1d0fc0c124a9d Gerrit-Change-Number: 27392 Gerrit-PatchSet: 5 Gerrit-Owner: dexter <pmaier(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: laforge <laforge(a)osmocom.org> Gerrit-Reviewer: msuraev <msuraev(a)sysmocom.de> Gerrit-Reviewer: pespin <pespin(a)sysmocom.de> Gerrit-MessageType: merged

1 0

[M] Change in osmo-mgw[master]: mgcp_codec: refactor payload type converstion
by laforge 29 Mar '23

29 Mar '23

laforge has submitted this change. ( https://gerrit.osmocom.org/c/osmo-mgw/+/32083 ) Change subject: mgcp_codec: refactor payload type converstion ...................................................................... mgcp_codec: refactor payload type converstion in mgcp_codec.c we have a function mgcp_codec_pt_translate that is used to find the equivalent payload type number on an opposite connection. This is quite specific and the resulting payload type number may still belong to a codec that might require some form of conversion. Lets refactor this so that the function just finds a convertible codec for a given connection. This also opens up other usecases. The payload type conversion like we did it before can then be done with a few lines in mgcp_network.c Related: OS#5461 Change-Id: I085260a2ca8cfecdb58656b7a046c536189e238d --- M include/osmocom/mgcp/mgcp_codec.h M src/libosmo-mgcp/mgcp_codec.c M src/libosmo-mgcp/mgcp_network.c M tests/mgcp/mgcp_test.c M tests/mgcp/mgcp_test.ok 5 files changed, 113 insertions(+), 65 deletions(-) Approvals: Jenkins Builder: Verified pespin: Looks good to me, but someone else must approve laforge: Looks good to me, approved diff --git a/include/osmocom/mgcp/mgcp_codec.h b/include/osmocom/mgcp/mgcp_codec.h index 994d770..1f29b08 100644 --- a/include/osmocom/mgcp/mgcp_codec.h +++ b/include/osmocom/mgcp/mgcp_codec.h @@ -14,8 +14,9 @@ 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); -int mgcp_codec_pt_translate(struct mgcp_conn_rtp *conn_src, struct mgcp_conn_rtp *conn_dst, int payload_type); +struct mgcp_rtp_codec *mgcp_codec_find_convertible(struct mgcp_conn_rtp *conn, struct mgcp_rtp_codec *codec); 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); diff --git a/src/libosmo-mgcp/mgcp_codec.c b/src/libosmo-mgcp/mgcp_codec.c index ccb5d77..9086047 100644 --- a/src/libosmo-mgcp/mgcp_codec.c +++ b/src/libosmo-mgcp/mgcp_codec.c @@ -425,63 +425,42 @@ return true; } -/*! Translate a given payload type number that belongs to the packet of a - * source connection to the equivalent payload type number that matches the - * configuration of a destination connection. - * \param[in] conn_src related source rtp-connection. - * \param[in] conn_dst related destination rtp-connection. - * \param[in] payload_type number from the source packet or source connection. - * \returns translated payload type number on success, -EINVAL on failure. */ -int mgcp_codec_pt_translate(struct mgcp_conn_rtp *conn_src, struct mgcp_conn_rtp *conn_dst, int payload_type) +/*! For a given codec, find a convertible codec in the given connection. + * \param[in] conn connection to search for a convertible codec + * \param[in] codec for which a convertible codec shall be found. + * \returns codec on success, -NULL on failure. */ +struct mgcp_rtp_codec *mgcp_codec_find_convertible(struct mgcp_conn_rtp *conn, struct mgcp_rtp_codec *codec) { - struct mgcp_rtp_end *rtp_src; - struct mgcp_rtp_end *rtp_dst; - struct mgcp_rtp_codec *codec_src = NULL; - struct mgcp_rtp_codec *codec_dst = NULL; + struct mgcp_rtp_end *rtp_end; unsigned int i; unsigned int codecs_assigned; + struct mgcp_rtp_codec *codec_convertible = NULL; - rtp_src = &conn_src->end; - rtp_dst = &conn_dst->end; - - /* Find the codec information that is used on the source side */ - codecs_assigned = rtp_src->codecs_assigned; - OSMO_ASSERT(codecs_assigned <= MGCP_MAX_CODECS); - for (i = 0; i < codecs_assigned; i++) { - if (payload_type == rtp_src->codecs[i].payload_type) { - codec_src = &rtp_src->codecs[i]; - break; - } - } - if (!codec_src) - return -EINVAL; + 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_dst->codecs_assigned; + codecs_assigned = rtp_end->codecs_assigned; OSMO_ASSERT(codecs_assigned <= MGCP_MAX_CODECS); for (i = 0; i < codecs_assigned; i++) { - if (codecs_same(codec_src, &rtp_dst->codecs[i])) { - codec_dst = &rtp_dst->codecs[i]; + if (codecs_same(codec, &rtp_end->codecs[i])) { + codec_convertible = &rtp_end->codecs[i]; break; } } /* 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) */ - if (!codec_dst) { + if (!codec_convertible) { for (i = 0; i < codecs_assigned; i++) { - if (codecs_convertible(codec_src, &rtp_dst->codecs[i])) { - codec_dst = &rtp_dst->codecs[i]; + if (codecs_convertible(codec, &rtp_end->codecs[i])) { + codec_convertible = &rtp_end->codecs[i]; break; } } } - if (!codec_dst) - return -EINVAL; - - return codec_dst->payload_type; + return codec_convertible; } /* Find the payload type number configured for a specific codec by SDP. @@ -508,3 +487,26 @@ } return NULL; } + +/*! Lookup a codec that is assigned to a connection by its payload type number. + * \param[in] conn related rtp-connection. + * \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_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); + + for (i = 0; i < codecs_assigned; i++) { + if (payload_type == rtp_end->codecs[i].payload_type) { + codec = &rtp_end->codecs[i]; + break; + } + } + + return codec; +} diff --git a/src/libosmo-mgcp/mgcp_network.c b/src/libosmo-mgcp/mgcp_network.c index af9ae99..ce08dd4 100644 --- a/src/libosmo-mgcp/mgcp_network.c +++ b/src/libosmo-mgcp/mgcp_network.c @@ -493,27 +493,31 @@ /* There may be different payload type numbers negotiated for two connections. * Patch the payload type of an RTP packet so that it uses the payload type * that is valid for the destination connection (conn_dst) */ -static int mgcp_patch_pt(struct mgcp_conn_rtp *conn_src, - struct mgcp_conn_rtp *conn_dst, struct msgb *msg) +static int mgcp_patch_pt(struct mgcp_conn_rtp *conn_src, struct mgcp_conn_rtp *conn_dst, struct msgb *msg) { struct rtp_hdr *rtp_hdr; - uint8_t pt_in; - int pt_out; + struct mgcp_rtp_codec *codec_src; + struct mgcp_rtp_codec *codec_dst; if (msgb_length(msg) < sizeof(struct rtp_hdr)) { LOG_CONN_RTP(conn_src, LOGL_ERROR, "RTP packet too short (%u < %zu)\n", msgb_length(msg), sizeof(struct rtp_hdr)); return -EINVAL; } - rtp_hdr = (struct rtp_hdr *)msgb_data(msg); - pt_in = rtp_hdr->payload_type; - pt_out = mgcp_codec_pt_translate(conn_src, conn_dst, pt_in); - if (pt_out < 0) + /* Find the codec information that is used on the source side */ + codec_src = mgcp_codec_from_pt(conn_src, rtp_hdr->payload_type); + if (!codec_src) return -EINVAL; - rtp_hdr->payload_type = (uint8_t) pt_out; + /* Lookup a suitable codec in the destination connection. (The codec must be of the same type or at least + * convertible) */ + codec_dst = mgcp_codec_find_convertible(conn_dst, codec_src); + if (!codec_dst) + return -EINVAL; + + rtp_hdr->payload_type = (uint8_t) codec_dst->payload_type; return 0; } diff --git a/tests/mgcp/mgcp_test.c b/tests/mgcp/mgcp_test.c index 8f081a9..bc2dae0 100644 --- a/tests/mgcp/mgcp_test.c +++ b/tests/mgcp/mgcp_test.c @@ -1769,26 +1769,26 @@ .amr_octet_aligned_present = false, }; -struct testcase_mgcp_codec_pt_translate_codec { +struct testcase_mgcp_codec_find_convertible_codec { int payload_type; const char *audio_name; const struct mgcp_codec_param *param; int expect_rc; }; -struct testcase_mgcp_codec_pt_translate_expect { +struct testcase_mgcp_codec_find_convertible_expect { bool end; int payload_type_map[2]; }; -struct testcase_mgcp_codec_pt_translate { +struct testcase_mgcp_codec_find_convertible { const char *descr; /* two conns on an endpoint, each with N configured codecs */ - struct testcase_mgcp_codec_pt_translate_codec codecs[2][10]; - struct testcase_mgcp_codec_pt_translate_expect expect[32]; + struct testcase_mgcp_codec_find_convertible_codec codecs[2][10]; + struct testcase_mgcp_codec_find_convertible_expect expect[32]; }; -static const struct testcase_mgcp_codec_pt_translate test_mgcp_codec_pt_translate_cases[] = { +static const struct testcase_mgcp_codec_find_convertible test_mgcp_codec_find_convertible_cases[] = { { .descr = "same order, but differing payload type numbers", .codecs = { @@ -2044,14 +2044,39 @@ }, }; -static void test_mgcp_codec_pt_translate(void) +static int pt_translate(struct mgcp_conn_rtp *conn, unsigned int index_src, unsigned int index_dst, int payload_type) +{ + struct mgcp_rtp_codec *codec_src = NULL; + struct mgcp_rtp_codec *codec_dst = NULL; + struct mgcp_conn_rtp *conn_src = &conn[index_src]; + struct mgcp_conn_rtp *conn_dst = &conn[index_dst]; + + /* Find the codec information that is used on the source side */ + codec_src = mgcp_codec_from_pt(conn_src, payload_type); + if (!codec_src) { + printf(" - mgcp_codec_from_pt(conn%u, %d) -> NO RESULT\n", index_src, payload_type); + return -EINVAL; + } + printf(" - mgcp_codec_from_pt(conn%u, %d) -> %s\n", index_src, payload_type, codec_src->subtype_name); + + codec_dst = mgcp_codec_find_convertible(conn_dst, codec_src); + if (!codec_dst) { + printf(" - mgcp_codec_find_convertible(conn%u, %s) -> NO RESULT\n", index_dst, codec_src->subtype_name); + return -EINVAL; + } + printf(" - mgcp_codec_find_convertible(conn%u, %s) -> %s -> %u\n", + index_dst, codec_src->subtype_name, codec_dst->subtype_name, codec_dst->payload_type); + return codec_dst->payload_type; +} + +static void test_mgcp_codec_find_convertible(void) { int i; bool ok = true; - printf("\nTesting mgcp_codec_pt_translate()\n"); + printf("\nTesting mgcp_codec_find_convertible()\n"); - for (i = 0; i < ARRAY_SIZE(test_mgcp_codec_pt_translate_cases); i++) { - const struct testcase_mgcp_codec_pt_translate *t = &test_mgcp_codec_pt_translate_cases[i]; + for (i = 0; i < ARRAY_SIZE(test_mgcp_codec_find_convertible_cases); i++) { + const struct testcase_mgcp_codec_find_convertible *t = &test_mgcp_codec_find_convertible_cases[i]; struct mgcp_conn_rtp conn[2] = {}; int rc; int conn_i; @@ -2062,7 +2087,7 @@ for (conn_i = 0; conn_i < 2; conn_i++) { printf(" - add codecs on conn%d:\n", conn_i); for (c = 0; c < ARRAY_SIZE(t->codecs[conn_i]); c++) { - const struct testcase_mgcp_codec_pt_translate_codec *codec = &t->codecs[conn_i][c]; + const struct testcase_mgcp_codec_find_convertible_codec *codec = &t->codecs[conn_i][c]; if (!codec->audio_name) break; @@ -2086,15 +2111,13 @@ } for (c = 0; c < ARRAY_SIZE(t->expect); c++) { - const struct testcase_mgcp_codec_pt_translate_expect *expect = &t->expect[c]; + const struct testcase_mgcp_codec_find_convertible_expect *expect = &t->expect[c]; int result; if (expect->end) break; - result = mgcp_codec_pt_translate(&conn[0], &conn[1], expect->payload_type_map[0]); - printf(" - mgcp_codec_pt_translate(conn0, conn1, %d) -> %d\n", - expect->payload_type_map[0], result); + result = pt_translate(conn, 0, 1, expect->payload_type_map[0]); if (result != expect->payload_type_map[1]) { printf(" ERROR: expected -> %d\n", expect->payload_type_map[1]); ok = false; @@ -2104,9 +2127,7 @@ if (expect->payload_type_map[1] < 0) continue; - result = mgcp_codec_pt_translate(&conn[1], &conn[0], expect->payload_type_map[1]); - printf(" - mgcp_codec_pt_translate(conn1, conn0, %d) -> %d\n", - expect->payload_type_map[1], result); + result = pt_translate(conn, 1, 0, expect->payload_type_map[1]); if (result != expect->payload_type_map[0]) { printf(" ERROR: expected -> %d\n", expect->payload_type_map[0]); ok = false; @@ -2274,7 +2295,7 @@ test_osmux_cid(); test_get_lco_identifier(); test_check_local_cx_options(ctx); - test_mgcp_codec_pt_translate(); + test_mgcp_codec_find_convertible(); test_conn_id_matching(); test_e1_trunk_nr_from_epname(); test_mgcp_is_rtp_dummy_payload(); diff --git a/tests/mgcp/mgcp_test.ok b/tests/mgcp/mgcp_test.ok index 8b3b3d1..4ea9a34 100644 --- a/tests/mgcp/mgcp_test.ok +++ b/tests/mgcp/mgcp_test.ok Binary files differ -- To view, visit https://gerrit.osmocom.org/c/osmo-mgw/+/32083 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-mgw Gerrit-Branch: master Gerrit-Change-Id: I085260a2ca8cfecdb58656b7a046c536189e238d Gerrit-Change-Number: 32083 Gerrit-PatchSet: 2 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-MessageType: merged

1 0

[S] Change in osmo-mgw[master]: mgcp_codec: cosmetic: remove line break in api-doc
by laforge 29 Mar '23

29 Mar '23

laforge has submitted this change. ( https://gerrit.osmocom.org/c/osmo-mgw/+/32084 ) Change subject: mgcp_codec: cosmetic: remove line break in api-doc ...................................................................... mgcp_codec: cosmetic: remove line break in api-doc We usually do not make a final line break on our api-doc strings. Change-Id: I025bbf36287ef014e294cc18a6435d7e2f9c1bff --- M src/libosmo-mgcp/mgcp_codec.c 1 file changed, 12 insertions(+), 2 deletions(-) Approvals: Jenkins Builder: Verified laforge: Looks good to me, approved diff --git a/src/libosmo-mgcp/mgcp_codec.c b/src/libosmo-mgcp/mgcp_codec.c index 9086047..003c4c4 100644 --- a/src/libosmo-mgcp/mgcp_codec.c +++ b/src/libosmo-mgcp/mgcp_codec.c @@ -470,8 +470,7 @@ * \param subtype_name SDP codec name without parameters (e.g. "AMR"). * \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. - */ + * \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 char *subtype_name, unsigned int match_nr) { -- To view, visit https://gerrit.osmocom.org/c/osmo-mgw/+/32084 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-mgw Gerrit-Branch: master Gerrit-Change-Id: I025bbf36287ef014e294cc18a6435d7e2f9c1bff Gerrit-Change-Number: 32084 Gerrit-PatchSet: 2 Gerrit-Owner: dexter <pmaier(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: laforge <laforge(a)osmocom.org> Gerrit-Reviewer: neels <nhofmeyr(a)sysmocom.de> Gerrit-CC: pespin <pespin(a)sysmocom.de> Gerrit-MessageType: merged

1 0

[M] Change in osmo-mgw[master]: mgcp_codec: refactor payload type converstion
by laforge 29 Mar '23

29 Mar '23

Attention is currently required from: pespin, dexter. laforge has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-mgw/+/32083 ) Change subject: mgcp_codec: refactor payload type converstion ...................................................................... Patch Set 2: Code-Review+2 -- To view, visit https://gerrit.osmocom.org/c/osmo-mgw/+/32083 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-mgw Gerrit-Branch: master Gerrit-Change-Id: I085260a2ca8cfecdb58656b7a046c536189e238d Gerrit-Change-Number: 32083 Gerrit-PatchSet: 2 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-Attention: pespin <pespin(a)sysmocom.de> Gerrit-Attention: dexter <pmaier(a)sysmocom.de> Gerrit-Comment-Date: Wed, 29 Mar 2023 11:58:17 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

[S] Change in libosmocore[master]: logging: add timestamp fmt 'date' = 2018-01-16,01:44:34.681
by laforge 29 Mar '23

29 Mar '23

Attention is currently required from: neels. laforge has posted comments on this change. ( https://gerrit.osmocom.org/c/libosmocore/+/5815 ) Change subject: logging: add timestamp fmt 'date' = 2018-01-16,01:44:34.681 ...................................................................... Patch Set 7: (1 comment) File include/osmocom/core/logging.h: https://gerrit.osmocom.org/c/libosmocore/+/5815/comment/ff8749a0_b65794a0 PS7, Line 307: LOG_TIMESTAMP_DATE > do we really want to insert this in the middle of the enum, which means that any program compiled wi […] ah nevermind, it's only introduced 2 patches earlier. -- To view, visit https://gerrit.osmocom.org/c/libosmocore/+/5815 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: libosmocore Gerrit-Branch: master Gerrit-Change-Id: Icbd5192ea835e24b12fe057cc1ab56e9572d75c0 Gerrit-Change-Number: 5815 Gerrit-PatchSet: 7 Gerrit-Owner: neels <nhofmeyr(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: laforge <laforge(a)osmocom.org> Gerrit-Reviewer: neels <nhofmeyr(a)sysmocom.de> Gerrit-Attention: neels <nhofmeyr(a)sysmocom.de> Gerrit-Comment-Date: Wed, 29 Mar 2023 11:57:53 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: laforge <laforge(a)osmocom.org> Gerrit-MessageType: comment

1 0