[PATCH] osmo-bts[master]: DTX DL AMR: rewrite FSM recursion

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/.

Max gerrit-no-reply at lists.osmocom.org
Thu Nov 10 18:34:42 UTC 2016


Review at  https://gerrit.osmocom.org/1225

DTX DL AMR: rewrite FSM recursion

Add explicit state for recursion (sending the different payload data in
response to the RTS request for same FN) and corresponding
transition. Remove ST_FACCH_V as with new explicit recursion handling it
becomes unreacheable. This makes it easier to maintain
preemption (interruption of current procedure due to FACCH or
Inhibition). This also reduces the number of possible transitions out of
each state thus reducing graph's cyclomatic complexity.

Change-Id: If39b68083d23a4a35f468a5d75f54eb733ebfd14
---
M include/osmo-bts/dtx_dl_amr_fsm.h
M include/osmo-bts/msg_utils.h
M src/common/dtx_dl_amr_fsm.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
8 files changed, 270 insertions(+), 137 deletions(-)


  git pull ssh://gerrit.osmocom.org:29418/osmo-bts refs/changes/25/1225/1

diff --git a/include/osmo-bts/dtx_dl_amr_fsm.h b/include/osmo-bts/dtx_dl_amr_fsm.h
index 8b19595..4fb2f25 100644
--- a/include/osmo-bts/dtx_dl_amr_fsm.h
+++ b/include/osmo-bts/dtx_dl_amr_fsm.h
@@ -14,10 +14,13 @@
 	ST_SID_F2,
 	ST_F1_INH,
 	ST_U_INH,
+	ST_F1_INH_REC,
+	ST_U_INH_REC,
 	ST_SID_U,
 	ST_ONSET_V,
 	ST_ONSET_F,
-	ST_FACCH_V,
+	ST_ONSET_V_REC,
+	ST_ONSET_F_REC,
 	ST_FACCH,
 };
 
diff --git a/include/osmo-bts/msg_utils.h b/include/osmo-bts/msg_utils.h
index 4f9868c..7321045 100644
--- a/include/osmo-bts/msg_utils.h
+++ b/include/osmo-bts/msg_utils.h
@@ -29,6 +29,8 @@
 void lchan_set_marker(bool t, struct gsm_lchan *lchan);
 bool dtx_dl_amr_enabled(const struct gsm_lchan *lchan);
 void dtx_dispatch(struct gsm_lchan *lchan, enum dtx_dl_amr_fsm_events e);
+bool dtx_recursion(const struct gsm_lchan *lchan);
+void dtx_int_signal(struct gsm_lchan *lchan);
 void dtx_cache_payload(struct gsm_lchan *lchan, const uint8_t *l1_payload,
 		       size_t length, uint32_t fn, int update);
 int dtx_dl_amr_fsm_step(struct gsm_lchan *lchan, const uint8_t *rtp_pl,
diff --git a/src/common/dtx_dl_amr_fsm.c b/src/common/dtx_dl_amr_fsm.c
index 5075957..d903b0c 100644
--- a/src/common/dtx_dl_amr_fsm.c
+++ b/src/common/dtx_dl_amr_fsm.c
@@ -96,11 +96,8 @@
 void dtx_fsm_f1_inh(struct osmo_fsm_inst *fi, uint32_t event, void *data)
 {
 	switch (event) {
-	case E_VOICE:
-		osmo_fsm_inst_state_chg(fi, ST_ONSET_V, 0, 0);
-		break;
-	case E_FACCH:
-		osmo_fsm_inst_state_chg(fi, ST_ONSET_F, 0, 0);
+	case E_COMPL:
+		osmo_fsm_inst_state_chg(fi, ST_F1_INH_REC, 0, 0);
 		break;
 	default:
 		LOGP(DL1P, LOGL_ERROR, "Unexpected event %d\n", event);
@@ -112,11 +109,34 @@
 void dtx_fsm_u_inh(struct osmo_fsm_inst *fi, uint32_t event, void *data)
 {
 	switch (event) {
-	case E_VOICE:
+	case E_COMPL:
+		osmo_fsm_inst_state_chg(fi, ST_U_INH_REC, 0, 0);
+		break;
+	default:
+		LOGP(DL1P, LOGL_ERROR, "Unexpected event %d\n", event);
+		OSMO_ASSERT(0);
+		break;
+	}
+}
+
+void dtx_fsm_f1_inh_rec(struct osmo_fsm_inst *fi, uint32_t event, void *data)
+{
+	switch (event) {
+	case E_COMPL:
 		osmo_fsm_inst_state_chg(fi, ST_VOICE, 0, 0);
 		break;
-	case E_FACCH:
-		osmo_fsm_inst_state_chg(fi, ST_FACCH, 0, 0);
+	default:
+		LOGP(DL1P, LOGL_ERROR, "Unexpected event %d\n", event);
+		OSMO_ASSERT(0);
+		break;
+	}
+}
+
+void dtx_fsm_u_inh_rec(struct osmo_fsm_inst *fi, uint32_t event, void *data)
+{
+	switch (event) {
+	case E_COMPL:
+		osmo_fsm_inst_state_chg(fi, ST_VOICE, 0, 0);
 		break;
 	default:
 		LOGP(DL1P, LOGL_ERROR, "Unexpected event %d\n", event);
@@ -155,13 +175,8 @@
 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);
-		break;
-	case E_FACCH:
-		osmo_fsm_inst_state_chg(fi, ST_FACCH_V, 0, 0);
+	case E_COMPL:
+		osmo_fsm_inst_state_chg(fi, ST_ONSET_V_REC, 0, 0);
 		break;
 	default:
 		LOGP(DL1P, LOGL_ERROR, "Unexpected event %d\n", event);
@@ -173,14 +188,8 @@
 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:
-		osmo_fsm_inst_state_chg(fi, ST_VOICE, 0, 0);
-		break;
-	case E_FACCH:
-		osmo_fsm_inst_state_chg(fi, ST_FACCH, 0, 0);
+	case E_COMPL:
+		osmo_fsm_inst_state_chg(fi, ST_ONSET_F_REC, 0, 0);
 		break;
 	default:
 		LOGP(DL1P, LOGL_ERROR, "Unexpected event %d\n", event);
@@ -189,17 +198,24 @@
 	}
 }
 
-void dtx_fsm_facch_v(struct osmo_fsm_inst *fi, uint32_t event, void *data)
+void dtx_fsm_onset_v_rec(struct osmo_fsm_inst *fi, uint32_t event, void *data)
 {
 	switch (event) {
-	case E_FACCH:
-		break;
-	case E_VOICE:
+	case E_COMPL:
 		osmo_fsm_inst_state_chg(fi, ST_VOICE, 0, 0);
 		break;
-	case E_SID_U:
+	default:
+		LOGP(DL1P, LOGL_ERROR, "Unexpected event %d\n", event);
+		OSMO_ASSERT(0);
 		break;
-	case E_SID_F:
+	}
+}
+
+void dtx_fsm_onset_f_rec(struct osmo_fsm_inst *fi, uint32_t event, void *data)
+{
+	switch (event) {
+	case E_COMPL:
+		osmo_fsm_inst_state_chg(fi, ST_FACCH, 0, 0);
 		break;
 	default:
 		LOGP(DL1P, LOGL_ERROR, "Unexpected event %d\n", event);
@@ -211,13 +227,14 @@
 void dtx_fsm_facch(struct osmo_fsm_inst *fi, uint32_t event, void *data)
 {
 	switch (event) {
+	case E_SID_U:
+	case E_SID_F:
 	case E_FACCH:
 		break;
 	case E_VOICE:
 		osmo_fsm_inst_state_chg(fi, ST_VOICE, 0, 0);
 		break;
-	case E_SID_U:
-	case E_SID_F:
+	case E_COMPL:
 		osmo_fsm_inst_state_chg(fi, ST_SID_F1, 0, 0);
 		break;
 	default:
@@ -244,28 +261,42 @@
 		.action = dtx_fsm_sid_f1,
 	},
 	/* SID-FIRST P2 (only for AMR HR):
-	   actual start of silence period in case of 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) | 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,
 	},
-	/* SID-FIRST Inhibited:
-	   incoming SPEECH or FACCH (only for AMR HR) */
+	/* SID-FIRST Inhibited: incoming SPEECH (only for AMR HR) */
 	[ST_F1_INH]= {
-		.in_event_mask = X(E_VOICE) | X(E_FACCH),
-		.out_state_mask = X(ST_VOICE) | X(ST_FACCH_V),
+		.in_event_mask = X(E_COMPL),
+		.out_state_mask = X(ST_F1_INH_REC),
 		.name = "SID-FIRST (Inh)",
 		.action = dtx_fsm_f1_inh,
 	},
-	/* SID-UPDATE Inhibited:
-	   incoming SPEECH or FACCH (only for AMR HR) */
+	/* SID-UPDATE Inhibited: incoming SPEECH (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),
+		.in_event_mask = X(E_COMPL),
+		.out_state_mask = X(ST_U_INH_REC),
 		.name = "SID-UPDATE (Inh)",
 		.action = dtx_fsm_u_inh,
+	},
+	/* SID-FIRST Inhibition recursion in progress:
+	   Inhibit itself was already sent, now have to send the voice that caused it */
+	[ST_F1_INH_REC]= {
+		.in_event_mask = X(E_COMPL),
+		.out_state_mask = X(ST_VOICE),
+		.name = "SID-FIRST (Inh, Rec)",
+		.action = dtx_fsm_f1_inh_rec,
+	},
+	/* SID-UPDATE Inhibition recursion in progress:
+	   Inhibit itself was already sent, now have to send the voice that caused it */
+	[ST_U_INH_REC]= {
+		.in_event_mask = X(E_COMPL),
+		.out_state_mask = X(ST_VOICE),
+		.name = "SID-UPDATE (Inh, Rec)",
+		.action = dtx_fsm_u_inh_rec,
 	},
 	/* Silence period with periodic comfort noise data updates */
 	[ST_SID_U]= {
@@ -276,31 +307,39 @@
 	},
 	/* ONSET - end of silent period due to incoming SPEECH frame */
 	[ST_ONSET_V]= {
-		.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),
+		.in_event_mask = X(E_COMPL),
+		.out_state_mask = X(ST_ONSET_V_REC),
 		.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),
+		.in_event_mask = X(E_COMPL),
+		.out_state_mask = X(ST_ONSET_F_REC),
 		.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 */
-	[ST_FACCH_V]= {
-		.in_event_mask = X(E_FACCH) | X(E_VOICE) | X(E_SID_U) | X(E_SID_F),
-		.out_state_mask = X(ST_FACCH_V) | X(ST_VOICE) | X(ST_SID_F1),
-		.name = "FACCH (SPEECH)",
-		.action = dtx_fsm_facch_v,
+	/* ONSET recursion in progress:
+	   ONSET itself was already sent, now have to send the voice that caused it */
+	[ST_ONSET_V_REC]= {
+		.in_event_mask = X(E_COMPL),
+		.out_state_mask = X(ST_VOICE),
+		.name = "ONSET (SPEECH, Rec)",
+		.action = dtx_fsm_onset_v_rec,
+	},
+	/* ONSET recursion in progress:
+	   ONSET itself was already sent, now have to send the data that caused it */
+	[ST_ONSET_F_REC]= {
+		.in_event_mask = X(E_COMPL),
+		.out_state_mask = X(ST_FACCH),
+		.name = "ONSET (FACCH, Rec)",
+		.action = dtx_fsm_onset_f_rec,
 	},
 	/* FACCH sending state: no SPEECH was observed before so once we're done
 	   FSM should get back to silent period via SID-FIRST */
 	[ST_FACCH]= {
-		.in_event_mask = X(E_FACCH) | X(E_VOICE) | X(E_SID_U) | X(E_SID_F),
-		.out_state_mask = X(ST_FACCH_V) | X(ST_VOICE) | X(ST_SID_F1),
+		.in_event_mask = X(E_FACCH) | X(E_VOICE) | X(E_COMPL) | X(E_SID_U) | X(E_SID_F),
+		.out_state_mask = X(ST_VOICE) | X(ST_SID_F1),
 		.name = "FACCH",
 		.action = dtx_fsm_facch,
 	},
@@ -310,7 +349,7 @@
 	{ E_VOICE,	"Voice" },
 	{ E_ONSET,	"ONSET" },
 	{ E_FACCH,	"FACCH" },
-	{ E_COMPL,	"Complete P1 -> P2" },
+	{ E_COMPL,	"Complete" },
 	{ E_INHIB,	"Inhibit" },
 	{ E_SID_F,	"SID-FIRST" },
 	{ E_SID_U,	"SID-UPDATE" },
diff --git a/src/common/msg_utils.c b/src/common/msg_utils.c
index b876443..c5081b4 100644
--- a/src/common/msg_utils.c
+++ b/src/common/msg_utils.c
@@ -22,6 +22,7 @@
 #include <osmo-bts/logging.h>
 #include <osmo-bts/oml.h>
 #include <osmo-bts/amr.h>
+#include <osmo-bts/rsl.h>
 
 #include <osmocom/gsm/protocol/ipaccess.h>
 #include <osmocom/gsm/protocol/gsm_12_21.h>
@@ -120,8 +121,15 @@
 	lchan->tch.dtx.len = copy_len + amr;
 	/* SID FIRST is special because it's both sent and cached: */
 	if (update == 0) {
-		lchan->tch.dtx.fn = fn;
 		lchan->tch.dtx.is_update = false; /* Mark SID FIRST explicitly */
+		/* for non-AMR case - always update FN for incoming SID FIRST */
+		if (!amr ||
+		    (dtx_dl_amr_enabled(lchan) &&
+		     lchan->tch.dtx.dl_amr_fsm->state != ST_SID_U))
+			lchan->tch.dtx.fn = fn;
+		/* for AMR case - do not update FN if SID FIRST arrives in a
+		   middle of silence: this should not be happening according to
+		   the spec */
 	}
 
 	memcpy(lchan->tch.dtx.cache + amr, l1_payload, copy_len);
@@ -148,12 +156,16 @@
 	int8_t sti, cmi;
 	int rc;
 
-	if (rtp_pl == NULL) { /* SID-FIRST P1 -> P2 */
+	if (lchan->type == GSM_LCHAN_TCH_H && /* SID-FIRST P1 -> P2 completion */
+	    lchan->tch.dtx.dl_amr_fsm->state == ST_SID_F2 && !rtp_pl) {
 		*len = 3;
 		memcpy(l1_payload, lchan->tch.dtx.cache, 2);
-		dtx_dispatch(lchan, E_COMPL);
+		dtx_dispatch(lchan, E_SID_U);
 		return 0;
 	}
+
+	if (!rtp_pl_len)
+		return -EBADMSG;
 
 	rc = osmo_amr_rtp_dec(rtp_pl, rtp_pl_len, &cmr, &cmi, &ft, &bfi, &sti);
 	if (rc < 0) {
@@ -199,10 +211,15 @@
 	}
 
 	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)
+		if (lchan->tch.dtx.dl_amr_fsm->state == ST_VOICE) {
+			/* SID FIRST/UPDATE scheduling logic relies on SID FIRST
+			   being sent first hence we have to force caching of SID
+			   as FIRST regardless of actually decoded type */
+			dtx_cache_payload(lchan, rtp_pl, rtp_pl_len, fn, false);
 			return osmo_fsm_inst_dispatch(lchan->tch.dtx.dl_amr_fsm,
 						      E_SID_F, (void *)lchan);
+		} else
+			dtx_cache_payload(lchan, rtp_pl, rtp_pl_len, fn, sti);
 		return osmo_fsm_inst_dispatch(lchan->tch.dtx.dl_amr_fsm,
 					      sti ? E_SID_U : E_SID_F,
 					      (void *)lchan);
@@ -220,11 +237,14 @@
 	return 0;
 }
 
+/* STI is located in payload byte 6, cache contains 2 byte prefix (CMR/CMI)
+ * STI set = SID UPDATE, STI unset = SID FIRST
+ */
 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;
@@ -241,17 +261,27 @@
 	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 ||
-	    lchan->tch.dtx.dl_amr_fsm->state == ST_ONSET_F) {
+	if (lchan->tch.dtx.dl_amr_fsm->state == ST_FACCH) {
 		/* force STI bit to 0 so cache is treated as SID FIRST */
 		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);
-		/* this FN was already used for ONSET message so we just prepare
-		   things for next one */
+		/* check that this FN has not been used for FACCH message
+		   already: we rely here on the order of RTS arrival from L1 - we
+		   expect that PH-DATA.req ALWAYS comes before PH-TCH.req for the
+		   same FN */
+		if (lchan->tch.dtx.fn != LCHAN_FN_DUMMY) {
+			/* FACCH interruption is over */
+			dtx_dispatch(lchan, E_COMPL);
+			return false;
+		} else
+			lchan->tch.dtx.fn = fn;
+		/* this FN was already used for FACCH or ONSET message so we just
+		   prepare things for next one */
 		return true;
 	}
+
+	if (lchan->tch.dtx.dl_amr_fsm->state == ST_VOICE)
+		return true;
 
 	/* according to 3GPP TS 26.093 A.5.1.1:
 	   (*26) to avoid float math, add 1 FN tolerance (-120) */
@@ -297,6 +327,11 @@
 	return false;
 }
 
+/*! \brief Check if DTX DL AMR is enabled for a given lchan (it have proper type,
+ *         FSM is allocated etc.)
+ *  \param[in] lchan Logical channel on which we check scheduling
+ *  \returns true if DTX DL AMR is enabled, false otherwise
+ */
 bool dtx_dl_amr_enabled(const struct gsm_lchan *lchan)
 {
 	if (lchan->ts->trx->bts->dtxd &&
@@ -306,6 +341,31 @@
 	return false;
 }
 
+/*! \brief Check if DTX DL AMR FSM state is recursive: requires secondary
+ *         response to a single RTS request from L1.
+ *  \param[in] lchan Logical channel on which we check scheduling
+ *  \returns true if DTX DL AMR FSM state is recursive, false otherwise
+ */
+bool dtx_recursion(const struct gsm_lchan *lchan)
+{
+	if (!dtx_dl_amr_enabled(lchan))
+		return false;
+
+	if (lchan->tch.dtx.dl_amr_fsm->state == ST_U_INH ||
+	    lchan->tch.dtx.dl_amr_fsm->state == ST_F1_INH ||
+	    lchan->tch.dtx.dl_amr_fsm->state == ST_ONSET_F ||
+	    lchan->tch.dtx.dl_amr_fsm->state == ST_ONSET_V ||
+	    lchan->tch.dtx.dl_amr_fsm->state == ST_ONSET_F_REC ||
+	    lchan->tch.dtx.dl_amr_fsm->state == ST_ONSET_V_REC)
+		return true;
+
+	return false;
+}
+
+/*! \brief Send signal to FSM: with proper check if DIX is enabled for this lchan
+ *  \param[in] lchan Logical channel on which we check scheduling
+ *  \param[in] e DTX DL AMR FSM Event
+ */
 void dtx_dispatch(struct gsm_lchan *lchan, enum dtx_dl_amr_fsm_events e)
 {
 	if (dtx_dl_amr_enabled(lchan))
@@ -313,7 +373,23 @@
 				       (void *)lchan);
 }
 
-/* repeat last SID if possible, returns SID length + 1 or 0 */
+/*! \brief Send internal signal to FSM: check that DTX is enabled for this chan,
+ *         check that current FSM and lchan states are permitting such signal.
+ *         Note: this should be the only way to dispatch E_COMPL to FSM from
+ *               BTS code.
+ *  \param[in] lchan Logical channel on which we check scheduling
+ */
+void dtx_int_signal(struct gsm_lchan *lchan)
+{
+	if (!dtx_dl_amr_enabled(lchan))
+		return;
+
+	if ((lchan->type == GSM_LCHAN_TCH_H &&
+	     lchan->tch.dtx.dl_amr_fsm->state == ST_SID_F1) ||
+	    dtx_recursion(lchan))
+		dtx_dispatch(lchan, E_COMPL);
+}
+
 /*! \brief Repeat last SID if possible in case of DTX
  *  \param[in] lchan Logical channel on which we check scheduling
  *  \param[in] dst Buffer to copy last SID into
@@ -334,13 +410,26 @@
 			return 0;
 
 	if (lchan->tch.dtx.len) {
+		if (dtx_dl_amr_enabled(lchan)) {
+			if ((lchan->type == GSM_LCHAN_TCH_H &&
+			     lchan->tch.dtx.dl_amr_fsm->state == ST_SID_F2) ||
+			    (lchan->type == GSM_LCHAN_TCH_F &&
+			     lchan->tch.dtx.dl_amr_fsm->state == ST_SID_F1)) {
+				/* advance FSM in case we've just sent SID FIRST
+				   to restore silence after FACCH interruption */
+				osmo_fsm_inst_dispatch(lchan->tch.dtx.dl_amr_fsm,
+						       E_SID_U, (void *)lchan);
+				dtx_sti_unset(lchan);
+			} else if (lchan->tch.dtx.dl_amr_fsm->state ==
+				   ST_SID_U) {
+				/* enforce SID UPDATE for next repetition: it
+				   might have been altered by FACCH handling */
+				dtx_sti_set(lchan);
+			lchan->tch.dtx.is_update = true;
+			}
+		}
 		memcpy(dst, lchan->tch.dtx.cache, lchan->tch.dtx.len);
 		lchan->tch.dtx.fn = fn;
-		/* enforce SID UPDATE for next repetition - it might have
-		   been altered by FACCH handling */
-		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 9d57c2f..ccf8aa2 100644
--- a/src/osmo-bts-litecell15/l1_if.c
+++ b/src/osmo-bts-litecell15/l1_if.c
@@ -53,6 +53,7 @@
 #include <osmo-bts/cbch.h>
 #include <osmo-bts/bts_model.h>
 #include <osmo-bts/l1sap.h>
+#include <osmo-bts/msg_utils.h>
 #include <osmo-bts/dtx_dl_amr_fsm.h>
 
 #include <nrw/litecell15/litecell15.h>
@@ -339,7 +340,6 @@
 	uint32_t u32Fn;
 	uint8_t u8Tn, subCh, u8BlockNbr = 0, sapi = 0;
 	uint8_t chan_nr, link_id;
-	bool rec = false;
 	int len;
 
 	if (!msg) {
@@ -355,6 +355,7 @@
 	u32Fn = l1sap->u.data.fn;
 	u8Tn = L1SAP_CHAN2TS(chan_nr);
 	subCh = 0x1f;
+	lchan = get_lchan_by_chan_nr(trx, chan_nr);
 	if (L1SAP_IS_LINK_SACCH(link_id)) {
 		sapi = GsmL1_Sapi_Sacch;
 		if (!L1SAP_IS_CHAN_TCHF(chan_nr))
@@ -404,8 +405,7 @@
 	if (len) {
 		/* data request */
 		GsmL1_Prim_t *l1p = msgb_l1prim(l1msg);
-		lchan = get_lchan_by_chan_nr(trx, chan_nr);
-
+		data_req_from_l1sap(l1p, fl1, u8Tn, u32Fn, sapi, subCh, u8BlockNbr, len);
 		if (use_cache)
 			memcpy(l1p->u.phDataReq.msgUnitParam.u8Buffer,
 			       lchan->tch.dtx.facch, msgb_l2len(msg));
@@ -423,20 +423,25 @@
 				memcpy(lchan->tch.dtx.facch, msg->l2h,
 				       msgb_l2len(msg));
 				/* prepare ONSET message */
-				len = 3;
 				l1p->u.phDataReq.msgUnitParam.u8Buffer[0] =
 					GsmL1_TchPlType_Amr_Onset;
 				/* ignored CMR/CMI pair */
 				l1p->u.phDataReq.msgUnitParam.u8Buffer[1] = 0;
 				l1p->u.phDataReq.msgUnitParam.u8Buffer[2] = 0;
-				/* ONSET is ready, recursive call is necessary */
-				rec = true;
+				/* update length */
+				data_req_from_l1sap(l1p, fl1, u8Tn, u32Fn, sapi,
+						    subCh, u8BlockNbr, 3);
+				/* update FN so it can be checked by TCH silence
+				   resume handler */
+				lchan->tch.dtx.fn = LCHAN_FN_DUMMY;
 			}
+		} else if (dtx_dl_amr_enabled(lchan) &&
+			   lchan->tch.dtx.dl_amr_fsm->state == ST_FACCH) {
+			/* update FN so it can be checked by TCH silence
+			   resume handler */
+			lchan->tch.dtx.fn = LCHAN_FN_DUMMY;
 		}
-
-		data_req_from_l1sap(l1p, fl1, u8Tn, u32Fn, sapi, subCh, u8BlockNbr, len);
-
-		if (!rec && !use_cache) {
+		else {
 			OSMO_ASSERT(msgb_l2len(msg) <= sizeof(l1p->u.phDataReq.msgUnitParam.u8Buffer));
 			memcpy(l1p->u.phDataReq.msgUnitParam.u8Buffer, msg->l2h,
 			       msgb_l2len(msg));
@@ -455,9 +460,10 @@
 	if (osmo_wqueue_enqueue(&fl1->write_q[MQ_L1_WRITE], l1msg) != 0) {
 		LOGP(DL1P, LOGL_ERROR, "MQ_L1_WRITE queue full. Dropping msg.\n");
 		msgb_free(l1msg);
-	}
+	} else
+		dtx_int_signal(lchan);
 
-	if (rec)
+	if (dtx_recursion(lchan))
 		ph_data_req(trx, msg, l1sap, true);
 	return 0;
 }
@@ -536,8 +542,9 @@
 	}
 	/* send message to DSP's queue */
 	osmo_wqueue_enqueue(&fl1->write_q[MQ_L1_WRITE], nmsg);
+	dtx_int_signal(lchan);
 
-	if (rc > 0 && trx->bts->dtxd) /* DTX: send voice after ONSET was sent */
+	if (dtx_recursion(lchan)) /* DTX: send voice after ONSET was sent */
 		return ph_tch_req(trx, l1sap->oph.msg, l1sap, true, false);
 
 	return 0;
diff --git a/src/osmo-bts-litecell15/tch.c b/src/osmo-bts-litecell15/tch.c
index 7495073..de3c7e3 100644
--- a/src/osmo-bts-litecell15/tch.c
+++ b/src/osmo-bts-litecell15/tch.c
@@ -305,9 +305,6 @@
 			*payload_type = GsmL1_TchPlType_Amr;
 			rtppayload_to_l1_amr(l1_payload + 2, rtp_pl, rtp_pl_len,
 					     ft);
-			/* force STI bit to 0 to make sure resume after FACCH
-			   works properly */
-			l1_payload[6 + 2] &= ~16;
 			return 0;
 		case ST_SID_F2:
 			*payload_type = GsmL1_TchPlType_Amr;
@@ -326,7 +323,6 @@
 			return 1;
 		case ST_SID_U:
 			return -EAGAIN;
-		case ST_FACCH_V:
 		case ST_FACCH:
 			return -EBADMSG;
 		default:
@@ -496,19 +492,17 @@
 	switch (lchan->tch_mode) {
 	case GSM48_CMODE_SPEECH_AMR:
 		if (lchan->type == GSM_LCHAN_TCH_H &&
-		    lchan->tch.dtx.dl_amr_fsm->state == ST_SID_F1 &&
 		    dtx_dl_amr_enabled(lchan)) {
+			/* we have to explicitly handle sending SID FIRST P2 for
+			   AMR HR in here */
 			*payload_type = GsmL1_TchPlType_Amr_SidFirstP2;
 			rc = dtx_dl_amr_fsm_step(lchan, NULL, 0, fn, l1_payload,
 						 false, &(msu_param->u8Size),
 						 NULL);
-			if (rc < 0) {
-				msgb_free(msg);
-				return NULL;
-			}
-			return msg;
-		} else
-			*payload_type = GsmL1_TchPlType_Amr;
+			if (rc == 0)
+				return msg;
+		}
+		*payload_type = GsmL1_TchPlType_Amr;
 		break;
 	case GSM48_CMODE_SPEECH_V1:
 		if (lchan->type == GSM_LCHAN_TCH_F)
@@ -524,13 +518,12 @@
 		return NULL;
 	}
 
-	if (dtx_dl_amr_enabled(lchan)) {
-		rc = repeat_last_sid(lchan, l1_payload, fn);
-		if (!rc) {
-			msgb_free(msg);
-			return NULL;
-		}
-		msu_param->u8Size = rc;
+	rc = repeat_last_sid(lchan, l1_payload, fn);
+	if (!rc) {
+		msgb_free(msg);
+		return NULL;
 	}
+	msu_param->u8Size = rc;
+
 	return msg;
 }
diff --git a/src/osmo-bts-sysmo/l1_if.c b/src/osmo-bts-sysmo/l1_if.c
index d14eac4..5b9df80 100644
--- a/src/osmo-bts-sysmo/l1_if.c
+++ b/src/osmo-bts-sysmo/l1_if.c
@@ -49,6 +49,7 @@
 #include <osmo-bts/cbch.h>
 #include <osmo-bts/bts_model.h>
 #include <osmo-bts/l1sap.h>
+#include <osmo-bts/msg_utils.h>
 #include <osmo-bts/dtx_dl_amr_fsm.h>
 
 #include <sysmocom/femtobts/superfemto.h>
@@ -334,7 +335,6 @@
 	uint32_t u32Fn;
 	uint8_t u8Tn, subCh, u8BlockNbr = 0, sapi = 0;
 	uint8_t chan_nr, link_id;
-	bool rec = false;
 	int len;
 
 	if (!msg) {
@@ -350,6 +350,7 @@
 	u32Fn = l1sap->u.data.fn;
 	u8Tn = L1SAP_CHAN2TS(chan_nr);
 	subCh = 0x1f;
+	lchan = get_lchan_by_chan_nr(trx, chan_nr);
 	if (L1SAP_IS_LINK_SACCH(link_id)) {
 		sapi = GsmL1_Sapi_Sacch;
 		if (!L1SAP_IS_CHAN_TCHF(chan_nr))
@@ -399,8 +400,7 @@
 	if (len) {
 		/* data request */
 		GsmL1_Prim_t *l1p = msgb_l1prim(l1msg);
-		lchan = get_lchan_by_chan_nr(trx, chan_nr);
-
+		data_req_from_l1sap(l1p, fl1, u8Tn, u32Fn, sapi, subCh, u8BlockNbr, len);
 		if (use_cache)
 			memcpy(l1p->u.phDataReq.msgUnitParam.u8Buffer,
 			       lchan->tch.dtx.facch, msgb_l2len(msg));
@@ -418,20 +418,25 @@
 				memcpy(lchan->tch.dtx.facch, msg->l2h,
 				       msgb_l2len(msg));
 				/* prepare ONSET message */
-				len = 3;
 				l1p->u.phDataReq.msgUnitParam.u8Buffer[0] =
 					GsmL1_TchPlType_Amr_Onset;
 				/* ignored CMR/CMI pair */
 				l1p->u.phDataReq.msgUnitParam.u8Buffer[1] = 0;
 				l1p->u.phDataReq.msgUnitParam.u8Buffer[2] = 0;
-				/* ONSET is ready, recursive call is necessary */
-				rec = true;
+				/* update length */
+				data_req_from_l1sap(l1p, fl1, u8Tn, u32Fn, sapi,
+						    subCh, u8BlockNbr, 3);
+				/* update FN so it can be checked by TCH silence
+				   resume handler */
+				lchan->tch.dtx.fn = LCHAN_FN_DUMMY;
 			}
+		} else if (dtx_dl_amr_enabled(lchan) &&
+			   lchan->tch.dtx.dl_amr_fsm->state == ST_FACCH) {
+			/* update FN so it can be checked by TCH silence
+			   resume handler */
+			lchan->tch.dtx.fn = LCHAN_FN_DUMMY;
 		}
-
-		data_req_from_l1sap(l1p, fl1, u8Tn, u32Fn, sapi, subCh, u8BlockNbr, len);
-
-		if (!rec && !use_cache) {
+		else {
 			OSMO_ASSERT(msgb_l2len(msg) <= sizeof(l1p->u.phDataReq.msgUnitParam.u8Buffer));
 			memcpy(l1p->u.phDataReq.msgUnitParam.u8Buffer, msg->l2h,
 			       msgb_l2len(msg));
@@ -450,9 +455,10 @@
 	if (osmo_wqueue_enqueue(&fl1->write_q[MQ_L1_WRITE], l1msg) != 0) {
 		LOGP(DL1P, LOGL_ERROR, "MQ_L1_WRITE queue full. Dropping msg.\n");
 		msgb_free(l1msg);
-	}
+	} else
+		dtx_int_signal(lchan);
 
-	if (rec)
+	if (dtx_recursion(lchan))
 		ph_data_req(trx, msg, l1sap, true);
 	return 0;
 }
@@ -531,8 +537,9 @@
 	}
 	/* send message to DSP's queue */
 	osmo_wqueue_enqueue(&fl1->write_q[MQ_L1_WRITE], nmsg);
+	dtx_int_signal(lchan);
 
-	if (rc > 0 && trx->bts->dtxd) /* DTX: send voice after ONSET was sent */
+	if (dtx_recursion(lchan)) /* DTX: send voice after ONSET was sent */
 		return ph_tch_req(trx, l1sap->oph.msg, l1sap, true, false);
 
 	return 0;
diff --git a/src/osmo-bts-sysmo/tch.c b/src/osmo-bts-sysmo/tch.c
index addb2ff..16c2cf3 100644
--- a/src/osmo-bts-sysmo/tch.c
+++ b/src/osmo-bts-sysmo/tch.c
@@ -403,9 +403,6 @@
 			*payload_type = GsmL1_TchPlType_Amr;
 			rtppayload_to_l1_amr(l1_payload + 2, rtp_pl, rtp_pl_len,
 					     ft);
-			/* force STI bit to 0 to make sure resume after FACCH
-			   works properly */
-			l1_payload[6 + 2] &= ~16;
 			return 0;
 		case ST_SID_F2:
 			*payload_type = GsmL1_TchPlType_Amr;
@@ -424,7 +421,6 @@
 			return 1;
 		case ST_SID_U:
 			return -EAGAIN;
-		case ST_FACCH_V:
 		case ST_FACCH:
 			return -EBADMSG;
 		default:
@@ -598,19 +594,17 @@
 	switch (lchan->tch_mode) {
 	case GSM48_CMODE_SPEECH_AMR:
 		if (lchan->type == GSM_LCHAN_TCH_H &&
-		    lchan->tch.dtx.dl_amr_fsm->state == ST_SID_F1 &&
 		    dtx_dl_amr_enabled(lchan)) {
+			/* we have to explicitly handle sending SID FIRST P2 for
+			   AMR HR in here */
 			*payload_type = GsmL1_TchPlType_Amr_SidFirstP2;
 			rc = dtx_dl_amr_fsm_step(lchan, NULL, 0, fn, l1_payload,
 						 false, &(msu_param->u8Size),
 						 NULL);
-			if (rc < 0) {
-				msgb_free(msg);
-				return NULL;
-			}
-			return msg;
-		} else
-			*payload_type = GsmL1_TchPlType_Amr;
+			if (rc == 0)
+				return msg;
+		}
+		*payload_type = GsmL1_TchPlType_Amr;
 		break;
 	case GSM48_CMODE_SPEECH_V1:
 		if (lchan->type == GSM_LCHAN_TCH_F)
@@ -626,13 +620,12 @@
 		return NULL;
 	}
 
-	if (dtx_dl_amr_enabled(lchan)) {
-		rc = repeat_last_sid(lchan, l1_payload, fn);
-		if (!rc) {
-			msgb_free(msg);
-			return NULL;
-		}
-		msu_param->u8Size = rc;
+	rc = repeat_last_sid(lchan, l1_payload, fn);
+	if (!rc) {
+		msgb_free(msg);
+		return NULL;
 	}
+	msu_param->u8Size = rc;
+
 	return msg;
 }

-- 
To view, visit https://gerrit.osmocom.org/1225
To unsubscribe, visit https://gerrit.osmocom.org/settings

Gerrit-MessageType: newchange
Gerrit-Change-Id: If39b68083d23a4a35f468a5d75f54eb733ebfd14
Gerrit-PatchSet: 1
Gerrit-Project: osmo-bts
Gerrit-Branch: master
Gerrit-Owner: Max <msuraev at sysmocom.de>



More information about the gerrit-log mailing list