Attention is currently required from: dexter, laforge, neels.
osmith has posted comments on this change by laforge. ( https://gerrit.osmocom.org/c/pysim/+/41642?usp=email )
Change subject: setup.py: Align cmd2 minimum version with requirements.txt
......................................................................
Patch Set 1:
(1 comment)
Patchset:
PS1:
> I believe it's worth mentioning that `debian:trixie` provides `2.5. […]
Good points. I think we need to figure out which versions of cmd2 we want to support. We could do e.g. the following:
1) Use this patch as-is and just require cmd2 >= 2.6.2 for PySim. We can then remove the backwards compatibility code you mentioned in a follow-up patch. The big advantage is that users won't have ancient cmd2 versions, against which we potentially might have incompatibilities over time without realizing it. It would make sense to update the description in README.md for installing pysim to rely on pip packages instead of debian packages:
```
$ python3 -m venv .venv
$ . .venv/bin/activate
$ pip install -r requirements.txt
```
2) Require cmd2 >= 1.5 (as before) in both setup.py and requirements.txt, and force in contrib/jenkins.sh that cmd2 >= 2.6.2 is installed similar to https://gitea.osmocom.org/sim-card/pysim/commit/004b06eab1d89ba2b29d7de9b77…. This has the advantage that people can still use their older versions from distro packages.
We don't build pysim in the Osmocom OBS. Personally I would be fine with 1), but maybe others have different opinions. Thoughts?
--
To view, visit https://gerrit.osmocom.org/c/pysim/+/41642?usp=email
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings?usp=email
Gerrit-MessageType: comment
Gerrit-Project: pysim
Gerrit-Branch: master
Gerrit-Change-Id: I71cee0ec3ed2abec68ec567beaab13c868721dad
Gerrit-Change-Number: 41642
Gerrit-PatchSet: 1
Gerrit-Owner: laforge <laforge(a)osmocom.org>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: dexter <pmaier(a)sysmocom.de>
Gerrit-Reviewer: fixeria <vyanitskiy(a)sysmocom.de>
Gerrit-Reviewer: neels <nhofmeyr(a)sysmocom.de>
Gerrit-Reviewer: osmith <osmith(a)sysmocom.de>
Gerrit-Attention: neels <nhofmeyr(a)sysmocom.de>
Gerrit-Attention: laforge <laforge(a)osmocom.org>
Gerrit-Attention: dexter <pmaier(a)sysmocom.de>
Gerrit-Comment-Date: Wed, 10 Dec 2025 14:40:02 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: fixeria <vyanitskiy(a)sysmocom.de>
Attention is currently required from: dexter, laforge, neels.
fixeria has posted comments on this change by laforge. ( https://gerrit.osmocom.org/c/pysim/+/41642?usp=email )
Change subject: setup.py: Align cmd2 minimum version with requirements.txt
......................................................................
Patch Set 1:
(1 comment)
Patchset:
PS1:
I believe it's worth mentioning that `debian:trixie` provides `2.5.11`, and `pySim-shell` still works fine with that older version. The only problem was artifacts in auto-generated documentation (https://osmocom.org/issues/6776). The version was bumped here: https://gerrit.osmocom.org/c/pysim/+/40952 and honestly I am not sure if this alone is a strong reason to require `2.6.2`.
Another thing I wanted to bring up with backwards compatibility quirks in `pySim-shell.py` (grep for `cmd2.__version__`). If we go for bumping the minimum version to `2.6.2`, it no longer makes sense to have the quirks for `<2.3.0` and `<2.0.0`.
--
To view, visit https://gerrit.osmocom.org/c/pysim/+/41642?usp=email
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings?usp=email
Gerrit-MessageType: comment
Gerrit-Project: pysim
Gerrit-Branch: master
Gerrit-Change-Id: I71cee0ec3ed2abec68ec567beaab13c868721dad
Gerrit-Change-Number: 41642
Gerrit-PatchSet: 1
Gerrit-Owner: laforge <laforge(a)osmocom.org>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: dexter <pmaier(a)sysmocom.de>
Gerrit-Reviewer: fixeria <vyanitskiy(a)sysmocom.de>
Gerrit-Reviewer: neels <nhofmeyr(a)sysmocom.de>
Gerrit-Attention: neels <nhofmeyr(a)sysmocom.de>
Gerrit-Attention: laforge <laforge(a)osmocom.org>
Gerrit-Attention: dexter <pmaier(a)sysmocom.de>
Gerrit-Comment-Date: Wed, 10 Dec 2025 14:20:05 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
laforge has submitted this change. ( https://gerrit.osmocom.org/c/osmo-mgw/+/41629?usp=email )
Change subject: mgcp_client: add fmtp string to struct ptmap
......................................................................
mgcp_client: add fmtp string to struct ptmap
Allow MGCP clients to pass arbitrary fmtp strings to MGWs, beyond
the fixed set of parameters captured in legacy struct mgcp_codec_param,
and set a different fmtp string per payload type, as opposed to global.
Credit: this patch is a derivative work based on Neels Hofmeyr's
patch If58590bda8627519ff07e0b6f43aa47a274f052b from WIP branch
neels/sdp, reduced to just libosmo-mgcp-client.
Present necessity: this functional addition is needed in order to
allow osmo-bsc to pass this construct to its MGW when the CN
requested the use of TW-TS-006 extended payload format for AMR:
a=rtpmap:112 AMR/8000/1
a=fmtp:112 octet-align=1; tw-ts-006=1
AMR codec fmtp parameter tw-ts-006 (defined in TW-TS-006 spec
section B.1) is not supported by osmo-mgw; however, it is supported
by tw-e1abis-mgw, which is the OsmoBSC-compatible MGW needed for
E1 BTS with AMR and CSD.
Change-Id: I84ba2ed5ab9d379ac0b675520796446ad6ee0710
---
M TODO-RELEASE
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
4 files changed, 49 insertions(+), 30 deletions(-)
Approvals:
Jenkins Builder: Verified
laforge: Looks good to me, approved
fixeria: Looks good to me, but someone else must approve
diff --git a/TODO-RELEASE b/TODO-RELEASE
index 0ed7189..582e4ce 100644
--- a/TODO-RELEASE
+++ b/TODO-RELEASE
@@ -7,3 +7,4 @@
# 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
+libosmo-mgcp-client ABI break struct ptmap extended
diff --git a/include/osmocom/mgcp_client/mgcp_client.h b/include/osmocom/mgcp_client/mgcp_client.h
index d53f442..2cddf00 100644
--- a/include/osmocom/mgcp_client/mgcp_client.h
+++ b/include/osmocom/mgcp_client/mgcp_client.h
@@ -78,6 +78,9 @@
/*! payload type number (96-127) */
unsigned int pt;
+
+ /*! the MGCP 'a=fmtp:N <...>' string, e.g. "mode-set=1,2,3;octet-align=0". */
+ char fmtp[256];
};
int ptmap_cmp(const struct ptmap *a, const struct ptmap *b);
diff --git a/include/osmocom/mgcp_client/mgcp_client_fsm.h b/include/osmocom/mgcp_client/mgcp_client_fsm.h
index dbd5128..3e31e9c 100644
--- a/include/osmocom/mgcp_client/mgcp_client_fsm.h
+++ b/include/osmocom/mgcp_client/mgcp_client_fsm.h
@@ -57,10 +57,11 @@
* address is set. If != MGCP_CONN_NONE, force this conn mode. */
enum mgcp_connection_mode conn_mode;
- /*! If the codec requires additional format parameters (fmtp), those cann be set here, see also
- * mgcp_common.h */
- bool param_present;
- struct mgcp_codec_param param;
+ /*! Deprectated, use ptmap[].fmtp instead.
+ * Global codec params. In case the codec requires additional format parameters (fmtp), those can be set
+ * here, see also mgcp_common.h. The format parameters will be applied on all codecs where applicable. */
+ bool param_present OSMO_DEPRECATED_OUTSIDE_LIBOSMOMGCPCLIENT("use ptmap[].fmtp instead");
+ struct mgcp_codec_param param OSMO_DEPRECATED_OUTSIDE_LIBOSMOMGCPCLIENT("use ptmap[].fmtp instead");
};
struct osmo_fsm_inst *mgcp_conn_create(struct mgcp_client *mgcp, struct osmo_fsm_inst *parent_fi, uint32_t parent_term_evt,
diff --git a/src/libosmo-mgcp-client/mgcp_client.c b/src/libosmo-mgcp-client/mgcp_client.c
index 4924be4..09c273c 100644
--- a/src/libosmo-mgcp-client/mgcp_client.c
+++ b/src/libosmo-mgcp-client/mgcp_client.c
@@ -1425,34 +1425,45 @@
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++) {
- pt = mgcp_msg->ptmap[i].pt;
- switch (mgcp_msg->ptmap[i].codec) {
- case CODEC_AMR_8000_1:
- case CODEC_AMRWB_16000_1:
- if (!mgcp_msg->param.amr_octet_aligned_present)
- break;
- MSGB_PRINTF_OR_RET("a=fmtp:%u octet-align=%d\r\n",
- pt, (int)mgcp_msg->param.amr_octet_aligned);
+ for (i = 0; i < mgcp_msg->ptmap_len; i++) {
+ pt = mgcp_msg->ptmap[i].pt;
+
+ /* Add fmtp string, if any is set. */
+ if (mgcp_msg->ptmap[i].fmtp[0]) {
+ MSGB_PRINTF_OR_RET("a=fmtp:%u %s\r\n", pt,
+ mgcp_msg->ptmap[i].fmtp);
+ continue;
+ }
+
+ /* 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)
+ continue;
+
+ switch (mgcp_msg->ptmap[i].codec) {
+ case CODEC_AMR_8000_1:
+ case CODEC_AMRWB_16000_1:
+ if (!mgcp_msg->param.amr_octet_aligned_present)
break;
- case CODEC_GSM_8000_1:
- case CODEC_GSMEFR_8000_1:
- if (!mgcp_msg->param.fr_efr_twts001_present)
- break;
- MSGB_PRINTF_OR_RET("a=fmtp:%u tw-ts-001=%d\r\n",
- pt, (int)mgcp_msg->param.fr_efr_twts001);
+ MSGB_PRINTF_OR_RET("a=fmtp:%u octet-align=%d\r\n",
+ pt, (int)mgcp_msg->param.amr_octet_aligned);
+ break;
+ case CODEC_GSM_8000_1:
+ case CODEC_GSMEFR_8000_1:
+ if (!mgcp_msg->param.fr_efr_twts001_present)
break;
- case CODEC_GSMHR_8000_1:
- if (!mgcp_msg->param.hr_twts002_present)
- break;
- MSGB_PRINTF_OR_RET("a=fmtp:%u tw-ts-002=%d\r\n",
- pt, (int)mgcp_msg->param.hr_twts002);
+ MSGB_PRINTF_OR_RET("a=fmtp:%u tw-ts-001=%d\r\n",
+ pt, (int)mgcp_msg->param.fr_efr_twts001);
+ break;
+ case CODEC_GSMHR_8000_1:
+ if (!mgcp_msg->param.hr_twts002_present)
break;
- default:
- /* no parameters for the remaining codecs */
- break;
- }
+ MSGB_PRINTF_OR_RET("a=fmtp:%u tw-ts-002=%d\r\n",
+ pt, (int)mgcp_msg->param.hr_twts002);
+ break;
+ default:
+ /* no parameters for the remaining codecs */
+ break;
}
}
@@ -1686,5 +1697,8 @@
rc = OSMO_CMP(a->codec, b->codec);
if (rc)
return rc;
- return OSMO_CMP(a->pt, b->pt);
+ rc = OSMO_CMP(a->pt, b->pt);
+ if (rc)
+ return rc;
+ return strcmp(a->fmtp, b->fmtp);
}
--
To view, visit https://gerrit.osmocom.org/c/osmo-mgw/+/41629?usp=email
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings?usp=email
Gerrit-MessageType: merged
Gerrit-Project: osmo-mgw
Gerrit-Branch: master
Gerrit-Change-Id: I84ba2ed5ab9d379ac0b675520796446ad6ee0710
Gerrit-Change-Number: 41629
Gerrit-PatchSet: 1
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: neels <nhofmeyr(a)sysmocom.de>
Gerrit-CC: pespin <pespin(a)sysmocom.de>
laforge has submitted this change. ( https://gerrit.osmocom.org/c/osmo-bsc/+/41630?usp=email )
Change subject: MGW control: migrate to new API for codecs and payload types
......................................................................
MGW control: migrate to new API for codecs and payload types
struct mgcp_conn_peer in libosmo-mgcp-client API used to have a
mandatory codecs[] array for codec specification and an optional
ptmap[] array to be used only when the default payload type needs
to be overridden. But now the codecs[] array is deprecated,
and new API features (see following patches) require the use of
ptmap[] array, with mandatory specification of payload type by
the MGCP client. Convert osmo-bsc to this new API.
Change-Id: Iafc38a3da64ce7c2f060a32864174dcde9f57b56
---
M src/osmo-bsc/lchan_rtp_fsm.c
1 file changed, 59 insertions(+), 17 deletions(-)
Approvals:
Jenkins Builder: Verified
laforge: Looks good to me, approved
fixeria: Looks good to me, but someone else must approve
diff --git a/src/osmo-bsc/lchan_rtp_fsm.c b/src/osmo-bsc/lchan_rtp_fsm.c
index 559a30b..c348833 100644
--- a/src/osmo-bsc/lchan_rtp_fsm.c
+++ b/src/osmo-bsc/lchan_rtp_fsm.c
@@ -21,6 +21,7 @@
*/
#include <osmocom/core/fsm.h>
+#include <osmocom/gsm/protocol/gsm_48_103.h>
#include <osmocom/gsm/rtp_extensions.h>
#include <osmocom/netif/rtp.h>
#include <osmocom/mgcp_client/mgcp_client_endpoint_fsm.h>
@@ -894,9 +895,36 @@
}
}
+static int chan_mode_to_mgcp_aoip_pt(enum mgcp_codecs codec)
+{
+ switch (codec) {
+ case CODEC_GSM_8000_1:
+ return OSMO_AOIP_RTP_PT_FR1;
+
+ case CODEC_GSMEFR_8000_1:
+ return OSMO_AOIP_RTP_PT_EFR;
+
+ case CODEC_GSMHR_8000_1:
+ return OSMO_AOIP_RTP_PT_HR1;
+
+ case CODEC_AMR_8000_1:
+ return OSMO_AOIP_RTP_PT_AMR;
+
+ case CODEC_CLEARMODE:
+ return OSMO_AOIP_RTP_PT_CSD;
+
+ default:
+ /* Error: unknown codec */
+ return -1;
+ }
+}
+
static int chan_mode_to_mgcp_bss_pt(enum mgcp_codecs codec)
{
switch (codec) {
+ case CODEC_GSM_8000_1:
+ return RTP_PT_GSM_FULL;
+
case CODEC_GSMHR_8000_1:
return RTP_PT_GSM_HALF;
@@ -906,9 +934,11 @@
case CODEC_AMR_8000_1:
return RTP_PT_AMR;
+ case CODEC_CLEARMODE:
+ return RTP_PT_CSDATA;
+
default:
- /* Not an error, we just leave it to libosmo-mgcp-client to
- * decide over the PT. */
+ /* Error: unknown codec */
return -1;
}
}
@@ -917,34 +947,46 @@
{
enum mgcp_codecs codec = chan_mode_to_mgcp_codec(lchan->activate.ch_mode_rate.chan_mode,
lchan->type == GSM_LCHAN_TCH_H? false : true);
- int custom_pt;
+ int pt;
if (codec < 0) {
LOG_LCHAN(lchan, LOGL_ERROR,
"Unable to determine MGCP codec type for %s in chan-mode %s\n",
gsm_chan_t_name(lchan->type), gsm48_chan_mode_name(lchan->activate.ch_mode_rate.chan_mode));
- verb_info->codecs_len = 0;
+ verb_info->ptmap_len = 0;
return;
}
- verb_info->codecs[0] = codec;
- verb_info->codecs_len = 1;
-
- /* Setup custom payload types (only for BSS side and when required) */
- custom_pt = chan_mode_to_mgcp_bss_pt(codec);
- if (bss_side && custom_pt > 0) {
- verb_info->ptmap[0].codec = codec;
- verb_info->ptmap[0].pt = custom_pt;
- verb_info->ptmap_len = 1;
+ /* The new libosmo-mgcp-client API requires us to provide explicit
+ * payload type number for every codec - no more internal defaulting.
+ * Legacy payload types used on IPA/Osmocom Abis-IP are defined in
+ * <osmocom/netif/rtp.h> as RTP_PT_*, new TS 48.103 (AoIP user plane)
+ * payload types are defined in <osmocom/gsm/protocol/gsm_48_103.h>
+ * as OSMO_AOIP_RTP_PT_*.
+ */
+ if (bss_side)
+ pt = chan_mode_to_mgcp_bss_pt(codec);
+ else
+ pt = chan_mode_to_mgcp_aoip_pt(codec);
+ if (pt < 0) {
+ LOG_LCHAN(lchan, LOGL_ERROR,
+ "Unable to determine RTP payload type for %s in chan-mode %s\n",
+ gsm_chan_t_name(lchan->type), gsm48_chan_mode_name(lchan->activate.ch_mode_rate.chan_mode));
+ verb_info->ptmap_len = 0;
+ return;
}
+ verb_info->ptmap[0].codec = codec;
+ verb_info->ptmap[0].pt = pt;
+ verb_info->ptmap_len = 1;
+
/* AMR requires additional parameters to be set up (framing mode) */
- if (verb_info->codecs[0] == CODEC_AMR_8000_1) {
+ if (codec == CODEC_AMR_8000_1) {
verb_info->param_present = true;
verb_info->param.amr_octet_aligned_present = true;
}
- if (bss_side && verb_info->codecs[0] == CODEC_AMR_8000_1) {
+ if (bss_side && codec == CODEC_AMR_8000_1) {
/* FIXME: At the moment all BTSs we support are using the
* octet-aligned payload format. However, in the future
* we may support BTSs that are using bandwidth-efficient
@@ -952,7 +994,7 @@
* that distinguishes by the BTS model which mode to use. */
verb_info->param.amr_octet_aligned = true;
}
- else if (!bss_side && verb_info->codecs[0] == CODEC_AMR_8000_1) {
+ else if (!bss_side && codec == CODEC_AMR_8000_1) {
verb_info->param.amr_octet_aligned = lchan->conn->sccp.msc->amr_octet_aligned;
}
@@ -988,5 +1030,5 @@
bool mgcp_codec_is_picked(const struct mgcp_conn_peer *verb_info, enum mgcp_codecs codec)
{
- return verb_info->codecs[0] == codec;
+ return verb_info->ptmap[0].codec == codec;
}
--
To view, visit https://gerrit.osmocom.org/c/osmo-bsc/+/41630?usp=email
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings?usp=email
Gerrit-MessageType: merged
Gerrit-Project: osmo-bsc
Gerrit-Branch: master
Gerrit-Change-Id: Iafc38a3da64ce7c2f060a32864174dcde9f57b56
Gerrit-Change-Number: 41630
Gerrit-PatchSet: 1
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: neels <nhofmeyr(a)sysmocom.de>
Gerrit-CC: pespin <pespin(a)sysmocom.de>