Attention is currently required from: dexter.
fixeria has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-ttcn3-hacks/+/33418 )
Change subject: BTS_Tests: Add support for PCUIF protocol version 11
......................................................................
Patch Set 1:
(2 comments)
File library/PCUIF_CodecPort.ttcn:
https://gerrit.osmocom.org/c/osmo-ttcn3-hacks/+/33418/comment/724629bd_9147…
PS1, Line 173:
cosmetic: no need for another tab-level here
File library/PCUIF_Types.ttcn:
https://gerrit.osmocom.org/c/osmo-ttcn3-hacks/+/33418/comment/0a4b8a9f_4944…
PS1, Line 376: enc_PCUIF_pch_dt
We usually declare enc/dec functions close to the record they operate on. This is not critical, but I would recommend declaring it right after `PCUIF_pch_dt`, not near `PCUIF_Message`.
--
To view, visit https://gerrit.osmocom.org/c/osmo-ttcn3-hacks/+/33418
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings
Gerrit-Project: osmo-ttcn3-hacks
Gerrit-Branch: master
Gerrit-Change-Id: I08de02e951e10bc8b4381cc2ad32e63f2747e3c4
Gerrit-Change-Number: 33418
Gerrit-PatchSet: 1
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: fixeria <vyanitskiy(a)sysmocom.de>
Gerrit-Attention: dexter <pmaier(a)sysmocom.de>
Gerrit-Comment-Date: Sun, 25 Jun 2023 07:48:08 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Gerrit-MessageType: comment
Attention is currently required from: dexter.
fixeria has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-ttcn3-hacks/+/33417 )
Change subject: PCUIF_Types: fix record PCUIF_pch_dt
......................................................................
Patch Set 1:
(1 comment)
Commit Message:
https://gerrit.osmocom.org/c/osmo-ttcn3-hacks/+/33417/comment/87fd5328_5644…
PS1, Line 10: it would pad the data with zeros if it does not fit the
: specified length. This is unwanted
> Could you please clarify a bit more here? So we have a fixed-size buffer of 17 bytes, and you're put […]
AFAIR, the `ALIGN` attribute tells where to put the payload if it's less than the buffer size: either at the beginning of the buffer (`left`) or at the end (`right`). I am trying to understand what do you achieve by removing this attribute.
--
To view, visit https://gerrit.osmocom.org/c/osmo-ttcn3-hacks/+/33417
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings
Gerrit-Project: osmo-ttcn3-hacks
Gerrit-Branch: master
Gerrit-Change-Id: I011eb2496b1422c49736b227dfa1e2a2d6096d67
Gerrit-Change-Number: 33417
Gerrit-PatchSet: 1
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: fixeria <vyanitskiy(a)sysmocom.de>
Gerrit-Attention: dexter <pmaier(a)sysmocom.de>
Gerrit-Comment-Date: Sun, 25 Jun 2023 07:29:13 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: fixeria <vyanitskiy(a)sysmocom.de>
Gerrit-MessageType: comment
Attention is currently required from: dexter.
fixeria has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-ttcn3-hacks/+/33417 )
Change subject: PCUIF_Types: fix record PCUIF_pch_dt
......................................................................
Patch Set 1:
(1 comment)
Commit Message:
https://gerrit.osmocom.org/c/osmo-ttcn3-hacks/+/33417/comment/20b10f3a_4366…
PS1, Line 10: it would pad the data with zeros if it does not fit the
: specified length. This is unwanted
Could you please clarify a bit more here? So we have a fixed-size buffer of 17 bytes, and you're putting there a string (IMSI prefix), which is less than 17 bytes total, let's say 3 chars + `\0`. If padding is unwanted, what do you expect the RAW encoder to put in the remaining bytes then?
--
To view, visit https://gerrit.osmocom.org/c/osmo-ttcn3-hacks/+/33417
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings
Gerrit-Project: osmo-ttcn3-hacks
Gerrit-Branch: master
Gerrit-Change-Id: I011eb2496b1422c49736b227dfa1e2a2d6096d67
Gerrit-Change-Number: 33417
Gerrit-PatchSet: 1
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: fixeria <vyanitskiy(a)sysmocom.de>
Gerrit-Attention: dexter <pmaier(a)sysmocom.de>
Gerrit-Comment-Date: Sun, 25 Jun 2023 07:20:52 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Gerrit-MessageType: comment
laforge has submitted this change. ( https://gerrit.osmocom.org/c/libosmocore/+/33419 )
Change subject: ecu: add is_dtx_pause() method
......................................................................
ecu: add is_dtx_pause() method
The ECU API of libosmocodec is unfortunately a peculiar non-3GPP
entity: an ECU by itself, severed from Rx DTX handler functions,
which is a logical/conceptual function with no place in the standard
3GPP architecture. The closest thing that exists in the standard
architecture is the TFO spec (TS 28.062 section C.3.2.1.1) calling
for an ECU application, but not comfort noise generation, in the case
of destination leg doing DTXd - but even then it is not a totally
"pure" ECU like libosmocodec API, it is an ECU plus a SID preener,
and the SID preening transform is not possible within the constraints
of existing libosmocodec ECU API. Hence truly correct handling of
corner cases, particularly invalid SID, is sadly impossible in the
current libosmocodec ECU framework.
The only current user of this API is the UL path in osmo-bts-trx;
however, as described in OS#6040, we would like to move that ECU call
from osmo-bts-trx model-specific code to the common layer of osmo-bts.
The current osmo-bts-trx incarnation avoids the SID handling problem
by suppressing the call to ECU frame_out() after any SID (valid or
invalid) was received on the air, thus pausing the RTP stream instead
of emitting ECU output during DTXu pauses. We would like to retain
the same behavior when we move this ECU call to the common layer,
into its proper place _after_ the link quality check in l1sap - but
the current method of flagging post-SID state in osmo-bts-trx will
no longer work on the other side of that link quality check.
As a workaround, have the ECU remember via a separate Boolean flag
whether it is in post-SID state or not (was the most recent frame_in()
any kind of SID or not), and provide is_dtx_pause() method to
retrieve this flag - the relocated ECU call in osmo-bts UL path
will use this new is_dtx_pause() method call to decide if it should
call frame_out() or switch to pausing RTP output.
Related: OS#6040
Change-Id: I3857be84bba12aaca0c2cca91458b7e13c5a642a
---
M include/osmocom/codec/ecu.h
M src/codec/ecu.c
M src/codec/ecu_fr.c
3 files changed, 72 insertions(+), 0 deletions(-)
Approvals:
Jenkins Builder: Verified
fixeria: Looks good to me, but someone else must approve
laforge: Looks good to me, approved
diff --git a/include/osmocom/codec/ecu.h b/include/osmocom/codec/ecu.h
index 9307ea1..6492860 100644
--- a/include/osmocom/codec/ecu.h
+++ b/include/osmocom/codec/ecu.h
@@ -62,12 +62,16 @@
/* generate output data for a substitute/erroneous frame */
int osmo_ecu_frame_out(struct osmo_ecu_state *st, uint8_t *frame_out);
+/* is the stream handled by this ECU currently in a DTX pause? */
+bool osmo_ecu_is_dtx_pause(struct osmo_ecu_state *st);
+
struct osmo_ecu_ops {
struct osmo_ecu_state * (*init)(void *ctx, enum osmo_ecu_codec codec);
void (*destroy)(struct osmo_ecu_state *);
int (*frame_in)(struct osmo_ecu_state *st, bool bfi,
const uint8_t *frame, unsigned int frame_bytes);
int (*frame_out)(struct osmo_ecu_state *st, uint8_t *frame_out);
+ bool (*is_dtx_pause)(struct osmo_ecu_state *st);
};
int osmo_ecu_register(const struct osmo_ecu_ops *ops, enum osmo_ecu_codec codec);
diff --git a/src/codec/ecu.c b/src/codec/ecu.c
index def72e6..cdb3e62 100644
--- a/src/codec/ecu.c
+++ b/src/codec/ecu.c
@@ -96,6 +96,20 @@
return g_ecu_ops[st->codec]->frame_out(st, frame_out);
}
+/*! check if the current state of this ECU is a DTX pause.
+ * \param[in] st ECU state/instance on which to operate
+ * \return true if DTX pause, false otherwise */
+bool osmo_ecu_is_dtx_pause(struct osmo_ecu_state *st)
+{
+ if (st->codec >= ARRAY_SIZE(g_ecu_ops))
+ return false;
+ if (!g_ecu_ops[st->codec])
+ return false;
+ if (!g_ecu_ops[st->codec]->is_dtx_pause)
+ return false;
+ return g_ecu_ops[st->codec]->is_dtx_pause(st);
+}
+
/***********************************************************************
* low-level API for ECU implementations
***********************************************************************/
diff --git a/src/codec/ecu_fr.c b/src/codec/ecu_fr.c
index 561ad60..adde03e 100644
--- a/src/codec/ecu_fr.c
+++ b/src/codec/ecu_fr.c
@@ -95,6 +95,7 @@
uint8_t sid_xmaxc;
uint8_t sid_reemit_count;
struct osmo_prbs_state prng;
+ bool last_input_was_sid;
};
/* This function is the frame input to the ECU - all inputs to this
@@ -110,6 +111,7 @@
case OSMO_GSM631_SID_CLASS_SPEECH:
memcpy(fr->speech_frame, frame, GSM_FR_BYTES);
fr->pr_state = STATE_SPEECH;
+ fr->last_input_was_sid = false;
return;
case OSMO_GSM631_SID_CLASS_INVALID:
/* GSM 06.31 section 6.1.2 says: "an invalid SID frame
@@ -136,6 +138,7 @@
* GSM 06.31 an invalid SID is still an accepted SID frame
* for the purpose of "lost SID" logic. */
fr->sid_reemit_count = 0;
+ fr->last_input_was_sid = true;
return;
case OSMO_GSM631_SID_CLASS_VALID:
/* save LARc part */
@@ -144,6 +147,7 @@
fr->sid_xmaxc = ((frame[27] & 0x1F) << 1) | (frame[28] >> 7);
fr->pr_state = STATE_SID;
fr->sid_reemit_count = 0;
+ fr->last_input_was_sid = true;
return;
default:
/* There are only 3 possible SID classifications per GSM 06.31
@@ -320,10 +324,18 @@
return GSM_FR_BYTES;
}
+static bool ecu_fr_is_dtx_pause(struct osmo_ecu_state *st)
+{
+ struct fr_ecu_state *fr = (struct fr_ecu_state *) &st->data;
+
+ return fr->last_input_was_sid;
+}
+
static const struct osmo_ecu_ops osmo_ecu_ops_fr = {
.init = ecu_fr_init,
.frame_in = ecu_fr_frame_in,
.frame_out = ecu_fr_frame_out,
+ .is_dtx_pause = ecu_fr_is_dtx_pause,
};
static __attribute__((constructor)) void on_dso_load_ecu_fr(void)
--
To view, visit https://gerrit.osmocom.org/c/libosmocore/+/33419
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings
Gerrit-Project: libosmocore
Gerrit-Branch: master
Gerrit-Change-Id: I3857be84bba12aaca0c2cca91458b7e13c5a642a
Gerrit-Change-Number: 33419
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-MessageType: merged
Attention is currently required from: falconia, neels.
laforge has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-msc/+/29973 )
Change subject: msc_a: send MNCC_RTP_CONNECT in call waiting scenarios
......................................................................
Patch Set 1: Code-Review+1
(1 comment)
Patchset:
PS1:
funny side-note: I think the GSM specs permit something ridiculous like up to 8 concurrent calls, any one of them being active, the others waiting.
I'm sure we can merge this patch as it has little chance of breaking things, but indeed it would be great to see a somewhat more comprehensive treatment for the topic
--
To view, visit https://gerrit.osmocom.org/c/osmo-msc/+/29973
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings
Gerrit-Project: osmo-msc
Gerrit-Branch: master
Gerrit-Change-Id: Ibb62cb3c154b99769b2dfe708f73c20e8b632f5d
Gerrit-Change-Number: 29973
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-Attention: falconia <falcon(a)freecalypso.org>
Gerrit-Attention: neels <nhofmeyr(a)sysmocom.de>
Gerrit-Comment-Date: Sun, 25 Jun 2023 06:31:03 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: Yes
Gerrit-MessageType: comment
Attention is currently required from: dexter.
laforge has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-ttcn3-hacks/+/33418 )
Change subject: BTS_Tests: Add support for PCUIF protocol version 11
......................................................................
Patch Set 1: Code-Review+2
--
To view, visit https://gerrit.osmocom.org/c/osmo-ttcn3-hacks/+/33418
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings
Gerrit-Project: osmo-ttcn3-hacks
Gerrit-Branch: master
Gerrit-Change-Id: I08de02e951e10bc8b4381cc2ad32e63f2747e3c4
Gerrit-Change-Number: 33418
Gerrit-PatchSet: 1
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: dexter <pmaier(a)sysmocom.de>
Gerrit-Comment-Date: Sun, 25 Jun 2023 06:28:49 +0000
Gerrit-HasComments: No
Gerrit-Has-Labels: Yes
Gerrit-MessageType: comment