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.orgHello Jenkins Builder,
I'd like you to reexamine a change. Please visit
https://gerrit.osmocom.org/845
to look at the new patch set (#3).
DTX: fix last SID save/repeat for lc15, sysmo
Previously SID was incorrectly stored (with GSM Fn instead of timestamp)
and, correspondingly, incorrectly retransmitted. Fix this by
* storing timestamp (in ms) alongside with SID instead of Fn
* using generic functions to save and repeat SID frames as necessary
Note: this requires corresponding change in OpenBSC.
Change-Id: Ie545212cce5ed2b3ea3228597f18a473f5e1deb4
Fixes: OS #1800, #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, 81 insertions(+), 136 deletions(-)
git pull ssh://gerrit.osmocom.org:29418/osmo-bts refs/changes/45/845/3
diff --git a/include/osmo-bts/msg_utils.h b/include/osmo-bts/msg_utils.h
index 591d194..f1a95c5 100644
--- a/include/osmo-bts/msg_utils.h
+++ b/include/osmo-bts/msg_utils.h
@@ -22,7 +22,9 @@
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);
+ bool update);
+uint8_t repeat_last_sid(const struct gsm_lchan *lchan, uint8_t *dst);
+bool dtx_amr_sid_optional(const struct gsm_lchan *lchan);
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..e4ca01e 100644
--- a/src/common/msg_utils.c
+++ b/src/common/msg_utils.c
@@ -25,9 +25,10 @@
#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>
-
+#include <time.h>
static int check_fom(struct abis_om_hdr *omh, size_t len)
{
@@ -97,20 +98,70 @@
}
}
+static inline time_t get_msec()
+{
+ struct timespec tspec;
+ clock_gettime(CLOCK_MONOTONIC, &tspec);
+
+ return 1000 * tspec.tv_sec + tspec.tv_nsec / 1000000;
+}
+
/* store the last SID frame in lchan context */
void save_last_sid(struct gsm_lchan *lchan, uint8_t *l1_payload, size_t length,
- uint32_t fn, bool update)
+ bool update)
{
size_t copy_len = OSMO_MIN(length + 1,
ARRAY_SIZE(lchan->tch.last_sid.buf));
+ struct timespec tspec;
+ clock_gettime(CLOCK_MONOTONIC, &tspec);
lchan->tch.last_sid.len = copy_len;
- lchan->tch.last_sid.fn = fn;
+ lchan->tch.last_sid.msec = get_msec();
lchan->tch.last_sid.is_update = update;
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(const struct gsm_lchan *lchan, uint8_t *dst)
+{
+ if (lchan->tch.last_sid.len) {
+ memcpy(dst, lchan->tch.last_sid.buf, lchan->tch.last_sid.len);
+ 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
+ * \returns true if transmission can be omitted, false otherwise
+ */
+bool dtx_amr_sid_optional(const struct gsm_lchan *lchan)
+{
+ time_t delta = get_msec() - lchan->tch.last_sid.msec;
+
+ /* 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 * 7)
+ 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 187f688..190d25f 100644
--- a/src/osmo-bts-litecell15/tch.c
+++ b/src/osmo-bts-litecell15/tch.c
@@ -270,14 +270,8 @@
}
#endif
- if (ft == AMR_SID) {
- /* store the last SID frame in lchan context */
- unsigned int copy_len;
- copy_len = OSMO_MIN(payload_len+1,
- ARRAY_SIZE(lchan->tch.last_sid.buf));
- lchan->tch.last_sid.len = copy_len;
- memcpy(lchan->tch.last_sid.buf, l1_payload, copy_len);
- }
+ if (ft == AMR_SID)
+ save_last_sid(lchan, l1_payload, payload_len, sti);
return payload_len+1;
}
@@ -469,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;
@@ -512,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)) {
+ 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);
+ 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)
@@ -547,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);
+ if (!msu_param->u8Size)
return NULL;
- }
break;
case GSM48_CMODE_SPEECH_EFR:
*payload_type = GsmL1_TchPlType_Efr;
@@ -562,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);
+ 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 39feae1..5b87220 100644
--- a/src/osmo-bts-sysmo/tch.c
+++ b/src/osmo-bts-sysmo/tch.c
@@ -367,14 +367,8 @@
}
#endif
- if (ft == AMR_SID) {
- /* store the last SID frame in lchan context */
- unsigned int copy_len;
- copy_len = OSMO_MIN(payload_len+1,
- ARRAY_SIZE(lchan->tch.last_sid.buf));
- lchan->tch.last_sid.len = copy_len;
- memcpy(lchan->tch.last_sid.buf, l1_payload, copy_len);
- }
+ if (ft == AMR_SID)
+ save_last_sid(lchan, l1_payload, payload_len, sti);
return payload_len+1;
}
@@ -572,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;
@@ -615,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)) {
+ 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);
+ 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)
@@ -650,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);
+ if (!msu_param->u8Size)
return NULL;
- }
break;
case GSM48_CMODE_SPEECH_EFR:
*payload_type = GsmL1_TchPlType_Efr;
@@ -665,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);
+ if (!msu_param->u8Size)
return NULL;
- }
break;
default:
msgb_free(msg);
--
To view, visit https://gerrit.osmocom.org/845
To unsubscribe, visit https://gerrit.osmocom.org/settings
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ie545212cce5ed2b3ea3228597f18a473f5e1deb4
Gerrit-PatchSet: 3
Gerrit-Project: osmo-bts
Gerrit-Branch: master
Gerrit-Owner: Max <msuraev at sysmocom.de>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: Neels Hofmeyr <nhofmeyr at sysmocom.de>