[MERGED] osmo-bts[master]: DTX: fix SID repeat scheduling

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
Fri Sep 23 15:00:18 UTC 2016


Max has submitted this change and it was merged.

Change subject: DTX: fix SID repeat scheduling
......................................................................


DTX: fix SID repeat scheduling

Previously SID retransmission was scheduled incorrectly based on GSM
frames instead of voice frames. Fix this by using GSM Fn only as elapsed
time estimation:

* move saved SID retransmission into generic function from lc15 and sysmo
specific code
* split retransmission time check into separate generic function
* compute estimation for elapsed time since last retransmission using
  GSM Fn

Change-Id: Ib054b458a7345d9ba40dba53754ca59ab099c8e8
Fixes: OS#1799
---
M include/osmo-bts/msg_utils.h
M src/common/msg_utils.c
M src/osmo-bts-litecell15/tch.c
M src/osmo-bts-sysmo/tch.c
4 files changed, 66 insertions(+), 116 deletions(-)

Approvals:
  Harald Welte: Looks good to me, approved
  Jenkins Builder: Verified



diff --git a/include/osmo-bts/msg_utils.h b/include/osmo-bts/msg_utils.h
index 591d194..cde7a93 100644
--- a/include/osmo-bts/msg_utils.h
+++ b/include/osmo-bts/msg_utils.h
@@ -23,6 +23,8 @@
 void lchan_set_marker(bool t, struct gsm_lchan *lchan);
 void save_last_sid(struct gsm_lchan *lchan, uint8_t *l1_payload, size_t length,
 		   uint32_t fn, bool update);
+uint8_t repeat_last_sid(struct gsm_lchan *lchan, uint8_t *dst, uint32_t fn);
+bool dtx_amr_sid_optional(const struct gsm_lchan *lchan, uint32_t fn);
 bool dtx_sched_optional(struct gsm_lchan *lchan, uint32_t fn);
 int msg_verify_ipa_structure(struct msgb *msg);
 int msg_verify_oml_structure(struct msgb *msg);
diff --git a/src/common/msg_utils.c b/src/common/msg_utils.c
index f12c653..967b10d 100644
--- a/src/common/msg_utils.c
+++ b/src/common/msg_utils.c
@@ -25,6 +25,7 @@
 #include <osmocom/gsm/protocol/gsm_12_21.h>
 #include <osmocom/gsm/abis_nm.h>
 #include <osmocom/core/msgb.h>
+#include <osmocom/trau/osmo_ortp.h>
 
 #include <arpa/inet.h>
 
@@ -111,6 +112,49 @@
 	memcpy(lchan->tch.last_sid.buf, l1_payload, copy_len);
 }
 
+/* repeat last SID if possible, returns SID length + 1 or 0 */
+/*! \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
+ *  \returns Number of bytes copied + 1 (to accommodate for extra byte with
+ *           payload type) or 0 if there's nothing to copy
+ */
+uint8_t repeat_last_sid(struct gsm_lchan *lchan, uint8_t *dst, uint32_t fn)
+{
+	if (lchan->tch.last_sid.len) {
+		memcpy(dst, lchan->tch.last_sid.buf, lchan->tch.last_sid.len);
+		lchan->tch.last_sid.fn = fn;
+		return lchan->tch.last_sid.len + 1;
+	}
+	LOGP(DL1C, LOGL_NOTICE, "Have to send %s frame on TCH but SID buffer "
+	     "is empty - sent nothing\n",
+	     get_value_string(gsm48_chan_mode_names, lchan->tch_mode));
+	return 0;
+}
+
+/*! \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
+ *  \returns true if transmission can be omitted, false otherwise
+ */
+bool dtx_amr_sid_optional(const struct gsm_lchan *lchan, uint32_t fn)
+{
+	/* Compute approx. time delta based on Fn duration */
+	uint32_t delta = GSM_FN_TO_MS(fn - lchan->tch.last_sid.fn);
+
+	/* according to 3GPP TS 26.093 A.5.1.1: */
+	if (lchan->tch.last_sid.is_update) {
+		/* SID UPDATE should be repeated every 8th RTP frame */
+		if (delta < GSM_RTP_FRAME_DURATION_MS * 8)
+			return true;
+		return false;
+	}
+	/* 3rd frame after SID FIRST should be SID UPDATE */
+	if (delta < GSM_RTP_FRAME_DURATION_MS * 3)
+		return true;
+	return false;
+}
+
 static inline bool fn_chk(const uint8_t *t, uint32_t fn)
 {
 	uint8_t i;
diff --git a/src/osmo-bts-litecell15/tch.c b/src/osmo-bts-litecell15/tch.c
index 5badc4d..4fdf6a6 100644
--- a/src/osmo-bts-litecell15/tch.c
+++ b/src/osmo-bts-litecell15/tch.c
@@ -463,27 +463,6 @@
 	return -EINVAL;
 }
 
-static bool repeat_last_sid(struct gsm_lchan *lchan, struct msgb *msg)
-{
-	GsmL1_Prim_t *l1p;
-	GsmL1_PhDataReq_t *data_req;
-	GsmL1_MsgUnitParam_t *msu_param;
-	uint8_t *l1_payload;
-
-	l1p = msgb_l1prim(msg);
-	data_req = &l1p->u.phDataReq;
-	msu_param = &data_req->msgUnitParam;
-	l1_payload = &msu_param->u8Buffer[1];
-
-	if (lchan->tch.last_sid.len) {
-		memcpy(l1_payload, lchan->tch.last_sid.buf,
-		       lchan->tch.last_sid.len);
-		msu_param->u8Size = lchan->tch.last_sid.len + 1;
-		return true;
-	}
-	return false;
-}
-
 struct msgb *gen_empty_tch_msg(struct gsm_lchan *lchan, uint32_t fn)
 {
 	struct msgb *msg;
@@ -506,30 +485,13 @@
 	switch (lchan->tch_mode) {
 	case GSM48_CMODE_SPEECH_AMR:
 		*payload_type = GsmL1_TchPlType_Amr;
-		/* according to 3GPP TS 26.093 A.5.1.1: */
-		if (lchan->tch.last_sid.is_update) {
-			/* SID UPDATE should be repeated every 8th frame */
-			if (fn - lchan->tch.last_sid.fn < 7) {
-				msgb_free(msg);
-				return NULL;
-			}
-		} else {
-			/* 3rd frame after SID FIRST should be SID UPDATE */
-			if (fn - lchan->tch.last_sid.fn < 3) {
-				msgb_free(msg);
-				return NULL;
-			}
+		if (dtx_amr_sid_optional(lchan, fn)) {
+			msgb_free(msg);
+			return NULL;
 		}
-		if (repeat_last_sid(lchan, msg))
-			return msg;
-		else {
-			LOGP(DL1C, LOGL_NOTICE, "Have to send AMR frame on TCH "
-			     "(FN=%u) but SID buffer is empty - sent NO_DATA\n",
-			     fn);
-			osmo_amr_rtp_enc(l1_payload, 0, AMR_NO_DATA,
-					 AMR_GOOD);
-			return msg;
-		}
+		msu_param->u8Size = repeat_last_sid(lchan, l1_payload, fn);
+		if (!msu_param->u8Size)
+			osmo_amr_rtp_enc(l1_payload, 0, AMR_NO_DATA, AMR_GOOD);
 		break;
 	case GSM48_CMODE_SPEECH_V1:
 		if (lchan->type == GSM_LCHAN_TCH_F)
@@ -541,14 +503,9 @@
 			msgb_free(msg);
 			return NULL;
 		}
-		if (repeat_last_sid(lchan, msg))
-			return msg;
-		else {
-			LOGP(DL1C, LOGL_NOTICE, "Have to send V1 frame on TCH "
-			     "(FN=%u) but SID buffer is empty - sent nothing\n",
-			     fn);
+		msu_param->u8Size = repeat_last_sid(lchan, l1_payload, fn);
+		if (!msu_param->u8Size)
 			return NULL;
-		}
 		break;
 	case GSM48_CMODE_SPEECH_EFR:
 		*payload_type = GsmL1_TchPlType_Efr;
@@ -556,14 +513,9 @@
 			msgb_free(msg);
 			return NULL;
 		}
-		if (repeat_last_sid(lchan, msg))
-			return msg;
-		else {
-			LOGP(DL1C, LOGL_NOTICE, "Have to send EFR frame on TCH "
-			     "(FN=%u) but SID buffer is empty - sent nothing\n",
-			     fn);
+		msu_param->u8Size = repeat_last_sid(lchan, l1_payload, fn);
+		if (!msu_param->u8Size)
 			return NULL;
-		}
 		break;
 	default:
 		msgb_free(msg);
diff --git a/src/osmo-bts-sysmo/tch.c b/src/osmo-bts-sysmo/tch.c
index 6c78ceb..ee72e53 100644
--- a/src/osmo-bts-sysmo/tch.c
+++ b/src/osmo-bts-sysmo/tch.c
@@ -566,27 +566,6 @@
 	return -EINVAL;
 }
 
-static bool repeat_last_sid(struct gsm_lchan *lchan, struct msgb *msg)
-{
-	GsmL1_Prim_t *l1p;
-	GsmL1_PhDataReq_t *data_req;
-	GsmL1_MsgUnitParam_t *msu_param;
-	uint8_t *l1_payload;
-
-	l1p = msgb_l1prim(msg);
-	data_req = &l1p->u.phDataReq;
-	msu_param = &data_req->msgUnitParam;
-	l1_payload = &msu_param->u8Buffer[1];
-
-	if (lchan->tch.last_sid.len) {
-		memcpy(l1_payload, lchan->tch.last_sid.buf,
-		       lchan->tch.last_sid.len);
-		msu_param->u8Size = lchan->tch.last_sid.len + 1;
-		return true;
-	}
-	return false;
-}
-
 struct msgb *gen_empty_tch_msg(struct gsm_lchan *lchan, uint32_t fn)
 {
 	struct msgb *msg;
@@ -609,30 +588,13 @@
 	switch (lchan->tch_mode) {
 	case GSM48_CMODE_SPEECH_AMR:
 		*payload_type = GsmL1_TchPlType_Amr;
-		/* according to 3GPP TS 26.093 A.5.1.1: */
-		if (lchan->tch.last_sid.is_update) {
-			/* SID UPDATE should be repeated every 8th frame */
-			if (fn - lchan->tch.last_sid.fn < 7) {
-				msgb_free(msg);
-				return NULL;
-			}
-		} else {
-			/* 3rd frame after SID FIRST should be SID UPDATE */
-			if (fn - lchan->tch.last_sid.fn < 3) {
-				msgb_free(msg);
-				return NULL;
-			}
+		if (dtx_amr_sid_optional(lchan, fn)) {
+			msgb_free(msg);
+			return NULL;
 		}
-		if (repeat_last_sid(lchan, msg))
-			return msg;
-		else {
-			LOGP(DL1C, LOGL_NOTICE, "Have to send AMR frame on TCH "
-			     "(FN=%u) but SID buffer is empty - sent NO_DATA\n",
-			     fn);
-			osmo_amr_rtp_enc(l1_payload, 0, AMR_NO_DATA,
-					 AMR_GOOD);
-			return msg;
-		}
+		msu_param->u8Size = repeat_last_sid(lchan, l1_payload, fn);
+		if (!msu_param->u8Size)
+			osmo_amr_rtp_enc(l1_payload, 0, AMR_NO_DATA, AMR_GOOD);
 		break;
 	case GSM48_CMODE_SPEECH_V1:
 		if (lchan->type == GSM_LCHAN_TCH_F)
@@ -644,14 +606,9 @@
 			msgb_free(msg);
 			return NULL;
 		}
-		if (repeat_last_sid(lchan, msg))
-			return msg;
-		else {
-			LOGP(DL1C, LOGL_NOTICE, "Have to send V1 frame on TCH "
-			     "(FN=%u) but SID buffer is empty - sent nothing\n",
-			     fn);
+		msu_param->u8Size = repeat_last_sid(lchan, l1_payload, fn);
+		if (!msu_param->u8Size)
 			return NULL;
-		}
 		break;
 	case GSM48_CMODE_SPEECH_EFR:
 		*payload_type = GsmL1_TchPlType_Efr;
@@ -659,14 +616,9 @@
 			msgb_free(msg);
 			return NULL;
 		}
-		if (repeat_last_sid(lchan, msg))
-			return msg;
-		else {
-			LOGP(DL1C, LOGL_NOTICE, "Have to send EFR frame on TCH "
-			     "(FN=%u) but SID buffer is empty - sent nothing\n",
-			     fn);
+		msu_param->u8Size = repeat_last_sid(lchan, l1_payload, fn);
+		if (!msu_param->u8Size)
 			return NULL;
-		}
 		break;
 	default:
 		msgb_free(msg);

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

Gerrit-MessageType: merged
Gerrit-Change-Id: Ib054b458a7345d9ba40dba53754ca59ab099c8e8
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>



More information about the gerrit-log mailing list