This is merely a historical archive of years 2008-2021, before the migration to mailman3.
A maintained and still updated list archive can be found at https://lists.osmocom.org/hyperkitty/list/gerrit-log@lists.osmocom.org/.
Harald Welte gerrit-no-reply at lists.osmocom.orgHarald Welte has submitted this change and it was merged.
Change subject: DTX fix ONSET handling
......................................................................
DTX fix ONSET handling
* re-introduce ST_ONSET_F to guard from repetitive ONSET messages in case
multiple FACCH occur duriing DTX silence period.
* produce ONSET event after both SID FIRST and UPDATE in case of AMR FR.
* always dispatch E_SID_F (SID FIRST) signal if in talkspurt.
* allow E_SID_* right after ONSET (zero-length talkspurt).
* add missing E_ONSET signal description.
* fix FSM transitions for AMR HR *Inhibited and First P*.
* fix incorrect return from l1if_tch_encode() in ONSET FACCH with
incoming SID UPDATE
Change-Id: I0e9033c5f169da46aed9a0d1295faff489778dcf
Related: OS#1801
---
M include/osmo-bts/dtx_dl_amr_fsm.h
M src/common/bts.c
M src/common/dtx_dl_amr_fsm.c
M src/common/l1sap.c
M src/common/msg_utils.c
M src/osmo-bts-litecell15/l1_if.c
M src/osmo-bts-litecell15/tch.c
M src/osmo-bts-sysmo/l1_if.c
M src/osmo-bts-sysmo/tch.c
9 files changed, 66 insertions(+), 32 deletions(-)
Approvals:
Harald Welte: Looks good to me, approved
Jenkins Builder: Verified
diff --git a/include/osmo-bts/dtx_dl_amr_fsm.h b/include/osmo-bts/dtx_dl_amr_fsm.h
index 5c13c19..8b19595 100644
--- a/include/osmo-bts/dtx_dl_amr_fsm.h
+++ b/include/osmo-bts/dtx_dl_amr_fsm.h
@@ -16,6 +16,7 @@
ST_U_INH,
ST_SID_U,
ST_ONSET_V,
+ ST_ONSET_F,
ST_FACCH_V,
ST_FACCH,
};
diff --git a/src/common/bts.c b/src/common/bts.c
index 6f621c4..2005e42 100644
--- a/src/common/bts.c
+++ b/src/common/bts.c
@@ -40,6 +40,7 @@
#include <osmo-bts/abis.h>
#include <osmo-bts/bts.h>
#include <osmo-bts/bts_model.h>
+#include <osmo-bts/dtx_dl_amr_fsm.h>
#include <osmo-bts/pcu_if.h>
#include <osmo-bts/rsl.h>
#include <osmo-bts/oml.h>
@@ -176,6 +177,8 @@
INIT_LLIST_HEAD(&btsb->smscb_state.queue);
INIT_LLIST_HEAD(&btsb->oml_queue);
+ /* register DTX DL FSM */
+ osmo_fsm_register(&dtx_dl_amr_fsm);
return rc;
}
diff --git a/src/common/dtx_dl_amr_fsm.c b/src/common/dtx_dl_amr_fsm.c
index a75fd00..5075957 100644
--- a/src/common/dtx_dl_amr_fsm.c
+++ b/src/common/dtx_dl_amr_fsm.c
@@ -53,7 +53,7 @@
osmo_fsm_inst_state_chg(fi, ST_VOICE, 0, 0);
break;
case E_FACCH:
- osmo_fsm_inst_state_chg(fi, ST_FACCH, 0, 0);
+ osmo_fsm_inst_state_chg(fi, ST_ONSET_F, 0, 0);
break;
case E_COMPL:
osmo_fsm_inst_state_chg(fi, ST_SID_F2, 0, 0);
@@ -81,7 +81,10 @@
osmo_fsm_inst_state_chg(fi, ST_VOICE, 0, 0);
break;
case E_FACCH:
- osmo_fsm_inst_state_chg(fi, ST_FACCH, 0, 0);
+ osmo_fsm_inst_state_chg(fi, ST_ONSET_F, 0, 0);
+ break;
+ case E_ONSET:
+ osmo_fsm_inst_state_chg(fi, ST_ONSET_V, 0, 0);
break;
default:
LOGP(DL1P, LOGL_ERROR, "Unexpected event %d\n", event);
@@ -97,7 +100,7 @@
osmo_fsm_inst_state_chg(fi, ST_ONSET_V, 0, 0);
break;
case E_FACCH:
- osmo_fsm_inst_state_chg(fi, ST_FACCH, 0, 0);
+ osmo_fsm_inst_state_chg(fi, ST_ONSET_F, 0, 0);
break;
default:
LOGP(DL1P, LOGL_ERROR, "Unexpected event %d\n", event);
@@ -110,7 +113,7 @@
{
switch (event) {
case E_VOICE:
- osmo_fsm_inst_state_chg(fi, ST_ONSET_V, 0, 0);
+ osmo_fsm_inst_state_chg(fi, ST_VOICE, 0, 0);
break;
case E_FACCH:
osmo_fsm_inst_state_chg(fi, ST_FACCH, 0, 0);
@@ -126,7 +129,7 @@
{
switch (event) {
case E_FACCH:
- osmo_fsm_inst_state_chg(fi, ST_FACCH, 0, 0);
+ osmo_fsm_inst_state_chg(fi, ST_ONSET_F, 0, 0);
break;
case E_VOICE:
osmo_fsm_inst_state_chg(fi, ST_VOICE, 0, 0);
@@ -152,6 +155,7 @@
void dtx_fsm_onset_v(struct osmo_fsm_inst *fi, uint32_t event, void *data)
{
switch (event) {
+ case E_SID_F:
case E_SID_U:
case E_VOICE:
osmo_fsm_inst_state_chg(fi, ST_VOICE, 0, 0);
@@ -169,6 +173,7 @@
void dtx_fsm_onset_f(struct osmo_fsm_inst *fi, uint32_t event, void *data)
{
switch (event) {
+ case E_SID_F:
case E_SID_U:
break;
case E_VOICE:
@@ -234,15 +239,15 @@
start of silence period (might be interrupted in case of AMR HR) */
[ST_SID_F1]= {
.in_event_mask = X(E_SID_F) | X(E_SID_U) | X(E_VOICE) | X(E_FACCH) | X(E_COMPL) | X(E_INHIB) | X(E_ONSET),
- .out_state_mask = X(ST_SID_U) | X(ST_VOICE) | X(ST_FACCH) | X(ST_SID_F2) | X(ST_F1_INH) | X(ST_ONSET_V),
+ .out_state_mask = X(ST_SID_U) | X(ST_VOICE) | X(ST_ONSET_F) | X(ST_SID_F2) | X(ST_F1_INH) | X(ST_ONSET_V),
.name = "SID-FIRST (P1)",
.action = dtx_fsm_sid_f1,
},
/* SID-FIRST P2 (only for AMR HR):
actual start of silence period in case of AMR HR*/
[ST_SID_F2]= {
- .in_event_mask = X(E_SID_U) | X(E_VOICE) | X(E_FACCH),
- .out_state_mask = X(ST_SID_U) | X(ST_VOICE) | X(ST_FACCH),
+ .in_event_mask = X(E_SID_U) | X(E_VOICE) | X(E_FACCH) | X(E_ONSET),
+ .out_state_mask = X(ST_SID_U) | X(ST_VOICE) | X(ST_ONSET_F) | X(ST_ONSET_V),
.name = "SID-FIRST (P2)",
.action = dtx_fsm_sid_f2,
},
@@ -258,23 +263,30 @@
incoming SPEECH or FACCH (only for AMR HR) */
[ST_U_INH]= {
.in_event_mask = X(E_VOICE) | X(E_FACCH),
- .out_state_mask = X(ST_VOICE) | X(ST_FACCH_V),
+ .out_state_mask = X(ST_VOICE) | X(ST_FACCH),
.name = "SID-UPDATE (Inh)",
.action = dtx_fsm_u_inh,
},
/* Silence period with periodic comfort noise data updates */
[ST_SID_U]= {
.in_event_mask = X(E_FACCH) | X(E_VOICE) | X(E_INHIB) | X(E_SID_U) | X(E_SID_F) | X(E_ONSET),
- .out_state_mask = X(ST_FACCH) | X(ST_VOICE) | X(ST_U_INH) | X(ST_SID_U) | X(ST_ONSET_V),
+ .out_state_mask = X(ST_ONSET_F) | X(ST_VOICE) | X(ST_U_INH) | X(ST_SID_U) | X(ST_ONSET_V),
.name = "SID-UPDATE",
.action = dtx_fsm_sid_upd,
},
/* ONSET - end of silent period due to incoming SPEECH frame */
[ST_ONSET_V]= {
- .in_event_mask = X(E_VOICE) | X(E_FACCH),
+ .in_event_mask = X(E_SID_F) | X(E_SID_U) | X(E_VOICE) | X(E_FACCH),
.out_state_mask = X(ST_VOICE) | X(ST_FACCH_V),
.name = "ONSET (SPEECH)",
.action = dtx_fsm_onset_v,
+ },
+ /* ONSET - end of silent period due to incoming FACCH frame */
+ [ST_ONSET_F]= {
+ .in_event_mask = X(E_VOICE) | X(E_FACCH) | X(E_SID_U) | X(E_SID_F),
+ .out_state_mask = X(ST_VOICE) | X(ST_FACCH),
+ .name = "ONSET (FACCH)",
+ .action = dtx_fsm_onset_f,
},
/* FACCH sending state: SPEECH was observed before so once we're done
FSM should get back to VOICE state */
@@ -296,6 +308,7 @@
const struct value_string dtx_dl_amr_fsm_event_names[] = {
{ E_VOICE, "Voice" },
+ { E_ONSET, "ONSET" },
{ E_FACCH, "FACCH" },
{ E_COMPL, "Complete P1 -> P2" },
{ E_INHIB, "Inhibit" },
diff --git a/src/common/l1sap.c b/src/common/l1sap.c
index 13d8a94..ef24800 100644
--- a/src/common/l1sap.c
+++ b/src/common/l1sap.c
@@ -1145,10 +1145,12 @@
/* Init DTX DL FSM if necessary */
//FIXME: only do it for AMR TCH/*
- osmo_fsm_register(&dtx_dl_amr_fsm);
- lchan->tch.dtx.dl_amr_fsm = osmo_fsm_inst_alloc(&dtx_dl_amr_fsm,
- tall_bts_ctx, lchan,
- LOGL_DEBUG, lchan->name);
+ if (trx->bts->dtxd)
+ lchan->tch.dtx.dl_amr_fsm = osmo_fsm_inst_alloc(&dtx_dl_amr_fsm,
+ tall_bts_ctx,
+ lchan,
+ LOGL_DEBUG,
+ lchan->name);
return 0;
}
diff --git a/src/common/msg_utils.c b/src/common/msg_utils.c
index 4b21366..851aacb 100644
--- a/src/common/msg_utils.c
+++ b/src/common/msg_utils.c
@@ -33,6 +33,8 @@
#include <arpa/inet.h>
#include <errno.h>
+#define STI_BIT_MASK 16
+
static int check_fom(struct abis_om_hdr *omh, size_t len)
{
if (omh->length != len) {
@@ -182,14 +184,15 @@
return 0;
if (osmo_amr_is_speech(ft)) {
- if (lchan->tch.dtx.dl_amr_fsm->state == ST_SID_F1 ||
- lchan->tch.dtx.dl_amr_fsm->state == ST_SID_U) /* AMR HR */
- if (lchan->type == GSM_LCHAN_TCH_H && marker)
- return osmo_fsm_inst_dispatch(lchan->tch.dtx.dl_amr_fsm,
- E_INHIB,
- (void *)lchan);
- /* AMR FR */
- if (marker && lchan->tch.dtx.dl_amr_fsm->state == ST_SID_U)
+ /* AMR HR - Inhibition */
+ if (lchan->type == GSM_LCHAN_TCH_H && marker &&
+ lchan->tch.dtx.dl_amr_fsm->state == ST_SID_F1)
+ return osmo_fsm_inst_dispatch(lchan->tch.dtx.dl_amr_fsm,
+ E_INHIB, (void *)lchan);
+ /* AMR FR & HR - generic */
+ if (marker && (lchan->tch.dtx.dl_amr_fsm->state == ST_SID_F1 ||
+ lchan->tch.dtx.dl_amr_fsm->state == ST_SID_F2 ||
+ lchan->tch.dtx.dl_amr_fsm->state == ST_SID_U ))
return osmo_fsm_inst_dispatch(lchan->tch.dtx.dl_amr_fsm,
E_ONSET, (void *)lchan);
return osmo_fsm_inst_dispatch(lchan->tch.dtx.dl_amr_fsm, E_VOICE,
@@ -198,6 +201,9 @@
if (ft == AMR_SID) {
dtx_cache_payload(lchan, rtp_pl, rtp_pl_len, fn, sti);
+ if (lchan->tch.dtx.dl_amr_fsm->state == ST_VOICE)
+ return osmo_fsm_inst_dispatch(lchan->tch.dtx.dl_amr_fsm,
+ E_SID_F, (void *)lchan);
return osmo_fsm_inst_dispatch(lchan->tch.dtx.dl_amr_fsm,
sti ? E_SID_U : E_SID_F,
(void *)lchan);
@@ -215,6 +221,16 @@
return 0;
}
+static inline void dtx_sti_set(struct gsm_lchan *lchan)
+{
+ lchan->tch.dtx.cache[6 + 2] |= STI_BIT_MASK;
+}
+/* STI is located in payload byte 6, cache contains 2 byte prefix (CMR/CMI) */
+static inline void dtx_sti_unset(struct gsm_lchan *lchan)
+{
+ lchan->tch.dtx.cache[6 + 2] &= ~STI_BIT_MASK;
+}
+
/*! \brief Check if enough time has passed since last SID (if any) to repeat it
* \param[in] lchan Logical channel on which we check scheduling
* \param[in] fn Frame Number for which we check scheduling
@@ -226,9 +242,10 @@
uint32_t dx26 = 120 * (fn - lchan->tch.dtx.fn);
/* We're resuming after FACCH interruption */
- if (lchan->tch.dtx.dl_amr_fsm->state == ST_FACCH) {
+ if (lchan->tch.dtx.dl_amr_fsm->state == ST_FACCH ||
+ lchan->tch.dtx.dl_amr_fsm->state == ST_ONSET_F) {
/* force STI bit to 0 so cache is treated as SID FIRST */
- lchan->tch.dtx.cache[6 + 2] &= ~16;
+ dtx_sti_unset(lchan);
lchan->tch.dtx.is_update = false;
osmo_fsm_inst_dispatch(lchan->tch.dtx.dl_amr_fsm, E_SID_F,
(void *)lchan);
@@ -306,7 +323,7 @@
lchan->tch.dtx.fn = fn;
/* enforce SID UPDATE for next repetition - it might have
been altered by FACCH handling */
- lchan->tch.dtx.cache[6 + 2] |= 16;
+ dtx_sti_set(lchan);
if (lchan->tch.dtx.dl_amr_fsm->state == ST_SID_U)
lchan->tch.dtx.is_update = true;
return lchan->tch.dtx.len + 1;
diff --git a/src/osmo-bts-litecell15/l1_if.c b/src/osmo-bts-litecell15/l1_if.c
index 795172b..0b1bad4 100644
--- a/src/osmo-bts-litecell15/l1_if.c
+++ b/src/osmo-bts-litecell15/l1_if.c
@@ -410,7 +410,7 @@
memcpy(l1p->u.phDataReq.msgUnitParam.u8Buffer,
lchan->tch.dtx.facch, msgb_l2len(msg));
else if (trx->bts->dtxd && lchan->tch.dtx.dl_amr_fsm &&
- lchan->tch.dtx.dl_amr_fsm->state == ST_FACCH) {
+ lchan->tch.dtx.dl_amr_fsm->state == ST_ONSET_F) {
if (sapi == GsmL1_Sapi_FacchF) {
sapi = GsmL1_Sapi_TchF;
}
diff --git a/src/osmo-bts-litecell15/tch.c b/src/osmo-bts-litecell15/tch.c
index 70764f5..4337d68 100644
--- a/src/osmo-bts-litecell15/tch.c
+++ b/src/osmo-bts-litecell15/tch.c
@@ -328,8 +328,7 @@
return -EAGAIN;
case ST_FACCH_V:
case ST_FACCH:
- /* FIXME: if this possible at all? */
- return 0;
+ return -EBADMSG;
default:
LOGP(DRTP, LOGL_ERROR, "Unhandled DTX DL AMR FSM state "
"%d\n", lchan->tch.dtx.dl_amr_fsm->state);
diff --git a/src/osmo-bts-sysmo/l1_if.c b/src/osmo-bts-sysmo/l1_if.c
index f7585ce..51bde8b 100644
--- a/src/osmo-bts-sysmo/l1_if.c
+++ b/src/osmo-bts-sysmo/l1_if.c
@@ -405,7 +405,7 @@
memcpy(l1p->u.phDataReq.msgUnitParam.u8Buffer,
lchan->tch.dtx.facch, msgb_l2len(msg));
else if (trx->bts->dtxd && lchan->tch.dtx.dl_amr_fsm &&
- lchan->tch.dtx.dl_amr_fsm->state == ST_FACCH) {
+ lchan->tch.dtx.dl_amr_fsm->state == ST_ONSET_F) {
if (sapi == GsmL1_Sapi_FacchF) {
sapi = GsmL1_Sapi_TchF;
}
diff --git a/src/osmo-bts-sysmo/tch.c b/src/osmo-bts-sysmo/tch.c
index fbb42b2..db5ca78 100644
--- a/src/osmo-bts-sysmo/tch.c
+++ b/src/osmo-bts-sysmo/tch.c
@@ -426,8 +426,7 @@
return -EAGAIN;
case ST_FACCH_V:
case ST_FACCH:
- /* FIXME: if this possible at all? */
- return 0;
+ return -EBADMSG;
default:
LOGP(DRTP, LOGL_ERROR, "Unhandled DTX DL AMR FSM state "
"%d\n", lchan->tch.dtx.dl_amr_fsm->state);
--
To view, visit https://gerrit.osmocom.org/1175
To unsubscribe, visit https://gerrit.osmocom.org/settings
Gerrit-MessageType: merged
Gerrit-Change-Id: I0e9033c5f169da46aed9a0d1295faff489778dcf
Gerrit-PatchSet: 4
Gerrit-Project: osmo-bts
Gerrit-Branch: master
Gerrit-Owner: Max <msuraev at sysmocom.de>
Gerrit-Reviewer: Harald Welte <laforge at gnumonks.org>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: Max <msuraev at sysmocom.de>