[PATCH] osmo-bts[master]: LC15: Fix missing fill frame and GSM 05.08 mandatory frame

2018-04-19 Thread Stefan Sperling
Hello Vadim Yanitskiy, Jenkins Builder,

I'd like you to reexamine a change.  Please visit

https://gerrit.osmocom.org/5753

to look at the new patch set (#8).

LC15: Fix missing fill frame and GSM 05.08 mandatory frame

It was discovered that the LC15 BTS does not send L2 fill frame
in case there is nothing to transmit. This leads to bad RXQUAL
reported by MS during signaling in TCH channel.

BTS needs to send L2 fill frame in case there is nothing to
transmit as indicated in GSM TS 05.08, section 8.3.

Related: OS#1950
Change-Id: I40e9bf9438c0b400e4d29eb39ffae37207e34db6
---
M src/common/msg_utils.c
M src/osmo-bts-litecell15/l1_if.c
2 files changed, 126 insertions(+), 15 deletions(-)


  git pull ssh://gerrit.osmocom.org:29418/osmo-bts refs/changes/53/5753/8

diff --git a/src/common/msg_utils.c b/src/common/msg_utils.c
index f936c98..fdd403d 100644
--- a/src/common/msg_utils.c
+++ b/src/common/msg_utils.c
@@ -379,15 +379,18 @@
static const uint8_t f[] = { 52, 53, 54, 55, 56, 57, 58, 59 },
h0[] = { 0, 2, 4, 6, 52, 54, 56, 58 },
h1[] = { 14, 16, 18, 20, 66, 68, 70, 72 };
-   if (lchan->tch_mode == GSM48_CMODE_SPEECH_V1) {
+
+   switch (lchan->tch_mode) {
+   case GSM48_CMODE_SPEECH_V1:
+   case GSM48_CMODE_SPEECH_AMR:
if (lchan->type == GSM_LCHAN_TCH_F)
return fn_chk(f, fn, ARRAY_SIZE(f));
-   else
-   return fn_chk(lchan->nr ? h1 : h0, fn,
- lchan->nr ? ARRAY_SIZE(h1) :
- ARRAY_SIZE(h0));
+
+   return fn_chk(lchan->nr ? h1 : h0, fn,
+   lchan->nr ? ARRAY_SIZE(h1) : ARRAY_SIZE(h0));
+   default:
+   return false;
}
-   return false;
 }
 
 /*! \brief Check if DTX DL AMR is enabled for a given lchan (it have proper 
type,
@@ -465,16 +468,70 @@
  */
 uint8_t repeat_last_sid(struct gsm_lchan *lchan, uint8_t *dst, uint32_t fn)
 {
-   /* FIXME: add EFR support */
-   if (lchan->tch_mode == GSM48_CMODE_SPEECH_EFR)
+   uint8_t payload_len;
+
+   static const uint8_t amr_sid_first_zero[] = {
+   0x00, 0x00, 0x00, 0x44, 0x00, 0x00, 0x00, 0x00, 0x00
+   };
+
+   switch (lchan->tch_mode) {
+   case GSM48_CMODE_SPEECH_EFR:
+   /* FIXME: add EFR support */
return 0;
 
-   if (lchan->tch_mode != GSM48_CMODE_SPEECH_AMR) {
-   if (dtx_sched_optional(lchan, fn))
-   return 0;
-   } else
+   case GSM48_CMODE_SPEECH_AMR:
if (dtx_amr_sid_optional(lchan, fn))
return 0;
+
+   if (lchan->rsl_cmode != RSL_CMOD_SPD_SPEECH)
+   break;
+
+   if (lchan->tch.dtx.len)
+   break;
+
+   /* Send zero SID FIRST frame */
+   payload_len = sizeof(amr_sid_first_zero);
+   memcpy(dst, amr_sid_first_zero, payload_len);
+
+   LOGP(DL1C, LOGL_DEBUG, "%s %s %s: Sending zero speech frame "
+   "for tch_mode=%s\n", gsm_fn_as_gsmtime_str(fn),
+   gsm_ts_name(lchan->ts), 
get_value_string(gsm_chan_t_names, lchan->type),
+   get_value_string(gsm48_chan_mode_names, 
lchan->tch_mode));
+
+   return payload_len + 1;
+
+   default:
+   if (dtx_sched_optional(lchan, fn))
+   return 0;
+
+   if (!lchan->ts->trx->bts->dtxd)
+   break;
+
+   /* Determine the payload length */
+   switch (lchan->type) {
+   case GSM_LCHAN_TCH_F:
+   payload_len = GSM_FR_BYTES;
+   break;
+   case GSM_LCHAN_TCH_H:
+   payload_len = GSM_HR_BYTES;
+   break;
+   default:
+   return 0;
+   }
+
+   /**
+* Need to send zeroed TCH frame on mandatory fn,
+* defined in GSM TS 05.08, section 8.3
+*/
+   memset(dst, 0x00, payload_len);
+
+   LOGP(DL1C, LOGL_DEBUG, "%s %s %s: Sending zero speech frame "
+   "for tch_mode=%s\n", gsm_fn_as_gsmtime_str(fn),
+   gsm_ts_name(lchan->ts), 
get_value_string(gsm_chan_t_names, lchan->type),
+   get_value_string(gsm48_chan_mode_names, 
lchan->tch_mode));
+
+   return payload_len + 1;
+   }
 
if (lchan->tch.dtx.len) {
if (dtx_dl_amr_enabled(lchan)) {
@@ -502,6 +559,12 @@
}
memcpy(dst, lchan->tch.dtx.cache, lchan->tch.dtx.len);
lchan->tch.dtx.fn = fn;
+
+   LOGP(DL1C, LOGL_DEBUG, "%s %s %s: Sending SID buffer "
+   "for tch_mode=%s\n", 

osmo-bts[master]: LC15: Fix missing fill frame and GSM 05.08 mandatory frame

2018-04-09 Thread Pau Espin Pedrol

Patch Set 7:

(2 comments)

https://gerrit.osmocom.org/#/c/5753/7/src/common/msg_utils.c
File src/common/msg_utils.c:

Line 489:   if (!!lchan->tch.dtx.len)
imho we can simplify the !!.


https://gerrit.osmocom.org/#/c/5753/7/src/osmo-bts-litecell15/l1_if.c
File src/osmo-bts-litecell15/l1_if.c:

Line 365:   LOGP(DL1C, LOGL_DEBUG, "%s tn=%u: Sending fill frame on in none 
DTX mode "
This text is really confusing.


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I40e9bf9438c0b400e4d29eb39ffae37207e34db6
Gerrit-PatchSet: 7
Gerrit-Project: osmo-bts
Gerrit-Branch: master
Gerrit-Owner: Minh-Quang Nguyen 
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: Max 
Gerrit-Reviewer: Minh-Quang Nguyen 
Gerrit-Reviewer: Pau Espin Pedrol 
Gerrit-Reviewer: Stefan Sperling 
Gerrit-Reviewer: Vadim Yanitskiy 
Gerrit-HasComments: Yes


[PATCH] osmo-bts[master]: LC15: Fix missing fill frame and GSM 05.08 mandatory frame

2018-04-09 Thread Stefan Sperling
Hello Vadim Yanitskiy, Jenkins Builder,

I'd like you to reexamine a change.  Please visit

https://gerrit.osmocom.org/5753

to look at the new patch set (#7).

LC15: Fix missing fill frame and GSM 05.08 mandatory frame

It was discovered that the LC15 BTS does not send L2 fill frame
in case there is nothing to transmit. This leads to bad RXQUAL
reported by MS during signaling in TCH channel.

BTS needs to send L2 fill frame in case there is nothing to
transmit as indicated in GSM TS 05.08, section 8.3.

Related: OS#1950
Change-Id: I40e9bf9438c0b400e4d29eb39ffae37207e34db6
---
M src/common/msg_utils.c
M src/osmo-bts-litecell15/l1_if.c
2 files changed, 126 insertions(+), 15 deletions(-)


  git pull ssh://gerrit.osmocom.org:29418/osmo-bts refs/changes/53/5753/7

diff --git a/src/common/msg_utils.c b/src/common/msg_utils.c
index f936c98..5fffd71 100644
--- a/src/common/msg_utils.c
+++ b/src/common/msg_utils.c
@@ -379,15 +379,18 @@
static const uint8_t f[] = { 52, 53, 54, 55, 56, 57, 58, 59 },
h0[] = { 0, 2, 4, 6, 52, 54, 56, 58 },
h1[] = { 14, 16, 18, 20, 66, 68, 70, 72 };
-   if (lchan->tch_mode == GSM48_CMODE_SPEECH_V1) {
+
+   switch (lchan->tch_mode) {
+   case GSM48_CMODE_SPEECH_V1:
+   case GSM48_CMODE_SPEECH_AMR:
if (lchan->type == GSM_LCHAN_TCH_F)
return fn_chk(f, fn, ARRAY_SIZE(f));
-   else
-   return fn_chk(lchan->nr ? h1 : h0, fn,
- lchan->nr ? ARRAY_SIZE(h1) :
- ARRAY_SIZE(h0));
+
+   return fn_chk(lchan->nr ? h1 : h0, fn,
+   lchan->nr ? ARRAY_SIZE(h1) : ARRAY_SIZE(h0));
+   default:
+   return false;
}
-   return false;
 }
 
 /*! \brief Check if DTX DL AMR is enabled for a given lchan (it have proper 
type,
@@ -465,16 +468,70 @@
  */
 uint8_t repeat_last_sid(struct gsm_lchan *lchan, uint8_t *dst, uint32_t fn)
 {
-   /* FIXME: add EFR support */
-   if (lchan->tch_mode == GSM48_CMODE_SPEECH_EFR)
+   uint8_t payload_len;
+
+   static const uint8_t amr_sid_first_zero[] = {
+   0x00, 0x00, 0x00, 0x44, 0x00, 0x00, 0x00, 0x00, 0x00
+   };
+
+   switch (lchan->tch_mode) {
+   case GSM48_CMODE_SPEECH_EFR:
+   /* FIXME: add EFR support */
return 0;
 
-   if (lchan->tch_mode != GSM48_CMODE_SPEECH_AMR) {
-   if (dtx_sched_optional(lchan, fn))
-   return 0;
-   } else
+   case GSM48_CMODE_SPEECH_AMR:
if (dtx_amr_sid_optional(lchan, fn))
return 0;
+
+   if (lchan->rsl_cmode != RSL_CMOD_SPD_SPEECH)
+   break;
+
+   if (!!lchan->tch.dtx.len)
+   break;
+
+   /* Send zero SID FIRST frame */
+   payload_len = sizeof(amr_sid_first_zero);
+   memcpy(dst, amr_sid_first_zero, payload_len);
+
+   LOGP(DL1C, LOGL_DEBUG, "%s %s %s: Sending zero speech frame "
+   "for tch_mode=%s\n", gsm_fn_as_gsmtime_str(fn),
+   gsm_ts_name(lchan->ts), 
get_value_string(gsm_chan_t_names, lchan->type),
+   get_value_string(gsm48_chan_mode_names, 
lchan->tch_mode));
+
+   return payload_len + 1;
+
+   default:
+   if (dtx_sched_optional(lchan, fn))
+   return 0;
+
+   if (!lchan->ts->trx->bts->dtxd)
+   break;
+
+   /* Determine the payload length */
+   switch (lchan->type) {
+   case GSM_LCHAN_TCH_F:
+   payload_len = GSM_FR_BYTES;
+   break;
+   case GSM_LCHAN_TCH_H:
+   payload_len = GSM_HR_BYTES;
+   break;
+   default:
+   return 0;
+   }
+
+   /**
+* Need to send zeroed TCH frame on mandatory fn,
+* defined in GSM TS 05.08, section 8.3
+*/
+   memset(dst, 0x00, payload_len);
+
+   LOGP(DL1C, LOGL_DEBUG, "%s %s %s: Sending zero speech frame "
+   "for tch_mode=%s\n", gsm_fn_as_gsmtime_str(fn),
+   gsm_ts_name(lchan->ts), 
get_value_string(gsm_chan_t_names, lchan->type),
+   get_value_string(gsm48_chan_mode_names, 
lchan->tch_mode));
+
+   return payload_len + 1;
+   }
 
if (lchan->tch.dtx.len) {
if (dtx_dl_amr_enabled(lchan)) {
@@ -502,6 +559,12 @@
}
memcpy(dst, lchan->tch.dtx.cache, lchan->tch.dtx.len);
lchan->tch.dtx.fn = fn;
+
+   LOGP(DL1C, LOGL_DEBUG, "%s %s %s: Sending SID buffer "
+   "for tch_mode=%s\n", 

osmo-bts[master]: LC15: Fix missing fill frame and GSM 05.08 mandatory frame

2018-04-09 Thread Pau Espin Pedrol

Patch Set 6:

(1 comment)

https://gerrit.osmocom.org/#/c/5753/6/src/common/msg_utils.c
File src/common/msg_utils.c:

Line 489:   if (!!lchan->tch.dtx.len)
> I am still unsure about this use of '!!'.
This is basically done to transform an integer into a boolean, ie: var>=0 -> 
var=1, but I agree in this place it doesn't make much sense and we can simply 
do:
if (lchan->tch.dtx.len!=0)

or 

if (lchan->tch.dtx.len)


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I40e9bf9438c0b400e4d29eb39ffae37207e34db6
Gerrit-PatchSet: 6
Gerrit-Project: osmo-bts
Gerrit-Branch: master
Gerrit-Owner: Minh-Quang Nguyen 
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: Max 
Gerrit-Reviewer: Minh-Quang Nguyen 
Gerrit-Reviewer: Pau Espin Pedrol 
Gerrit-Reviewer: Stefan Sperling 
Gerrit-Reviewer: Vadim Yanitskiy 
Gerrit-HasComments: Yes


osmo-bts[master]: LC15: Fix missing fill frame and GSM 05.08 mandatory frame

2018-04-09 Thread Stefan Sperling

Patch Set 6:

(1 comment)

https://gerrit.osmocom.org/#/c/5753/6/src/common/msg_utils.c
File src/common/msg_utils.c:

Line 489:   if (!!lchan->tch.dtx.len)
> I am still unsure about this use of '!!'.
Oops, I meant to say: "why not write it as 'dtx.len != 0'"


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I40e9bf9438c0b400e4d29eb39ffae37207e34db6
Gerrit-PatchSet: 6
Gerrit-Project: osmo-bts
Gerrit-Branch: master
Gerrit-Owner: Minh-Quang Nguyen 
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: Max 
Gerrit-Reviewer: Minh-Quang Nguyen 
Gerrit-Reviewer: Pau Espin Pedrol 
Gerrit-Reviewer: Stefan Sperling 
Gerrit-Reviewer: Vadim Yanitskiy 
Gerrit-HasComments: Yes


osmo-bts[master]: LC15: Fix missing fill frame and GSM 05.08 mandatory frame

2018-04-09 Thread Stefan Sperling

Patch Set 6:

(1 comment)

https://gerrit.osmocom.org/#/c/5753/6/src/common/msg_utils.c
File src/common/msg_utils.c:

Line 489:   if (!!lchan->tch.dtx.len)
I am still unsure about this use of '!!'.

It looks much like an accidental typo where '!' was intended.

Or is it really a check for a zero-length frame? If so, why not write it as 
dtx.len == 0?


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I40e9bf9438c0b400e4d29eb39ffae37207e34db6
Gerrit-PatchSet: 6
Gerrit-Project: osmo-bts
Gerrit-Branch: master
Gerrit-Owner: Minh-Quang Nguyen 
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: Max 
Gerrit-Reviewer: Minh-Quang Nguyen 
Gerrit-Reviewer: Pau Espin Pedrol 
Gerrit-Reviewer: Stefan Sperling 
Gerrit-Reviewer: Vadim Yanitskiy 
Gerrit-HasComments: Yes


osmo-bts[master]: LC15: Fix missing fill frame and GSM 05.08 mandatory frame

2018-04-09 Thread Stefan Sperling

Patch Set 6:

I have applied the purely cosmetic suggestions from my comment above.

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I40e9bf9438c0b400e4d29eb39ffae37207e34db6
Gerrit-PatchSet: 6
Gerrit-Project: osmo-bts
Gerrit-Branch: master
Gerrit-Owner: Minh-Quang Nguyen 
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: Max 
Gerrit-Reviewer: Minh-Quang Nguyen 
Gerrit-Reviewer: Pau Espin Pedrol 
Gerrit-Reviewer: Stefan Sperling 
Gerrit-Reviewer: Vadim Yanitskiy 
Gerrit-HasComments: No


[PATCH] osmo-bts[master]: LC15: Fix missing fill frame and GSM 05.08 mandatory frame

2018-04-09 Thread Stefan Sperling
Hello Vadim Yanitskiy, Jenkins Builder,

I'd like you to reexamine a change.  Please visit

https://gerrit.osmocom.org/5753

to look at the new patch set (#6).

LC15: Fix missing fill frame and GSM 05.08 mandatory frame

It was discovered that the LC15 BTS does not send L2 fill frame
in case there is nothing to transmit. This leads to bad RXQUAL
reported by MS during signaling in TCH channel.

BTS needs to send L2 fill frame in case there is nothing to
transmit as indicated in GSM TS 05.08, section 8.3.

Related: OS#1950
Change-Id: I40e9bf9438c0b400e4d29eb39ffae37207e34db6
---
M src/common/msg_utils.c
M src/osmo-bts-litecell15/l1_if.c
2 files changed, 127 insertions(+), 15 deletions(-)


  git pull ssh://gerrit.osmocom.org:29418/osmo-bts refs/changes/53/5753/6

diff --git a/src/common/msg_utils.c b/src/common/msg_utils.c
index f936c98..bab533b 100644
--- a/src/common/msg_utils.c
+++ b/src/common/msg_utils.c
@@ -379,15 +379,18 @@
static const uint8_t f[] = { 52, 53, 54, 55, 56, 57, 58, 59 },
h0[] = { 0, 2, 4, 6, 52, 54, 56, 58 },
h1[] = { 14, 16, 18, 20, 66, 68, 70, 72 };
-   if (lchan->tch_mode == GSM48_CMODE_SPEECH_V1) {
+
+   switch (lchan->tch_mode) {
+   case GSM48_CMODE_SPEECH_V1:
+   case GSM48_CMODE_SPEECH_AMR:
if (lchan->type == GSM_LCHAN_TCH_F)
return fn_chk(f, fn, ARRAY_SIZE(f));
-   else
-   return fn_chk(lchan->nr ? h1 : h0, fn,
- lchan->nr ? ARRAY_SIZE(h1) :
- ARRAY_SIZE(h0));
+
+   return fn_chk(lchan->nr ? h1 : h0, fn,
+   lchan->nr ? ARRAY_SIZE(h1) : ARRAY_SIZE(h0));
+   default:
+   return false;
}
-   return false;
 }
 
 /*! \brief Check if DTX DL AMR is enabled for a given lchan (it have proper 
type,
@@ -465,16 +468,71 @@
  */
 uint8_t repeat_last_sid(struct gsm_lchan *lchan, uint8_t *dst, uint32_t fn)
 {
-   /* FIXME: add EFR support */
-   if (lchan->tch_mode == GSM48_CMODE_SPEECH_EFR)
+   uint8_t payload_len;
+
+   static const uint8_t amr_sid_first_zero[] = {
+   0x00, 0x00, 0x00, 0x44, 0x00, 0x00, 0x00, 0x00, 0x00
+   };
+
+   switch (lchan->tch_mode) {
+   case GSM48_CMODE_SPEECH_EFR:
+   /* FIXME: add EFR support */
return 0;
 
-   if (lchan->tch_mode != GSM48_CMODE_SPEECH_AMR) {
-   if (dtx_sched_optional(lchan, fn))
-   return 0;
-   } else
+   case GSM48_CMODE_SPEECH_AMR:
if (dtx_amr_sid_optional(lchan, fn))
return 0;
+
+   if (lchan->rsl_cmode != RSL_CMOD_SPD_SPEECH)
+   break;
+
+   if (!!lchan->tch.dtx.len)
+   break;
+
+   /* Send zero SID FIRST frame */
+   payload_len = sizeof(amr_sid_first_zero);
+   memcpy(dst, amr_sid_first_zero, payload_len);
+
+   /* Debug print */
+   LOGP(DL1C, LOGL_DEBUG, "%s %s %s: Sending zero speech frame "
+   "for tch_mode=%s\n", gsm_fn_as_gsmtime_str(fn),
+   gsm_ts_name(lchan->ts), 
get_value_string(gsm_chan_t_names, lchan->type),
+   get_value_string(gsm48_chan_mode_names, 
lchan->tch_mode));
+
+   return payload_len + 1;
+
+   default:
+   if (dtx_sched_optional(lchan, fn))
+   return 0;
+
+   if (!lchan->ts->trx->bts->dtxd)
+   break;
+
+   /* Determine the payload length */
+   switch (lchan->type) {
+   case GSM_LCHAN_TCH_F:
+   payload_len = GSM_FR_BYTES;
+   break;
+   case GSM_LCHAN_TCH_H:
+   payload_len = GSM_HR_BYTES;
+   break;
+   default:
+   return 0;
+   }
+
+   /**
+* Need to send zeroed TCH frame on mandatory fn,
+* defined in GSM TS 05.08, section 8.3
+*/
+   memset(dst, 0x00, payload_len);
+
+   LOGP(DL1C, LOGL_DEBUG, "%s %s %s: Sending zero speech frame "
+   "for tch_mode=%s\n", gsm_fn_as_gsmtime_str(fn),
+   gsm_ts_name(lchan->ts), 
get_value_string(gsm_chan_t_names, lchan->type),
+   get_value_string(gsm48_chan_mode_names, 
lchan->tch_mode));
+
+   return payload_len + 1;
+   }
 
if (lchan->tch.dtx.len) {
if (dtx_dl_amr_enabled(lchan)) {
@@ -502,6 +560,12 @@
}
memcpy(dst, lchan->tch.dtx.cache, lchan->tch.dtx.len);
lchan->tch.dtx.fn = fn;
+
+   LOGP(DL1C, LOGL_DEBUG, "%s %s %s: Sending SID buffer "
+   

osmo-bts[master]: LC15: Fix missing fill frame and GSM 05.08 mandatory frame

2018-03-12 Thread Stefan Sperling

Patch Set 5:

(8 comments)

I'm not familiar with these parts of the spec. So my comments are only cosmetic 
in nature.

https://gerrit.osmocom.org/#/c/5753/5/src/common/msg_utils.c
File src/common/msg_utils.c:

Line 388:   else
Add curly braces to this else block? It spans more than one line due to 
indentation.

I wonder though if we should just remove the 'else' line and then indent the 
line below it one column to the left.


Line 489:   if (!!lchan->tch.dtx.len)
Is !! intentional? Might be clearer to write this as something like if (len) or 
if (len == 0) or if (len != 0)?


Line 529:   /* Debug print */
Comment adds no information and could be removed.


Line 565:   /* Debug print */
Comment adds no information and could be removed.


https://gerrit.osmocom.org/#/c/5753/5/src/osmo-bts-litecell15/l1_if.c
File src/osmo-bts-litecell15/l1_if.c:

Line 365:   /* Debug print */
This comment could be deleted, it doesn't add additional information.


Line 504:   if (lchan->rsl_cmode == RSL_CMOD_SPD_SIGN)
This if-block should use curly braces because it spans more than one line due 
to the comment


Line 508:   if (lchan->ts->trx->bts->dtxd)
likewise


Line 511:   else
This else block should use curly braces, too.


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I40e9bf9438c0b400e4d29eb39ffae37207e34db6
Gerrit-PatchSet: 5
Gerrit-Project: osmo-bts
Gerrit-Branch: master
Gerrit-Owner: Minh-Quang Nguyen 
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: Max 
Gerrit-Reviewer: Minh-Quang Nguyen 
Gerrit-Reviewer: Pau Espin Pedrol 
Gerrit-Reviewer: Stefan Sperling 
Gerrit-Reviewer: Vadim Yanitskiy 
Gerrit-HasComments: Yes


osmo-bts[master]: LC15: Fix missing fill frame and GSM 05.08 mandatory frame

2018-03-07 Thread Vadim Yanitskiy

Patch Set 5: Verified-1

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I40e9bf9438c0b400e4d29eb39ffae37207e34db6
Gerrit-PatchSet: 5
Gerrit-Project: osmo-bts
Gerrit-Branch: master
Gerrit-Owner: Minh-Quang Nguyen 
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: Max 
Gerrit-Reviewer: Minh-Quang Nguyen 
Gerrit-Reviewer: Pau Espin Pedrol 
Gerrit-Reviewer: Vadim Yanitskiy 
Gerrit-HasComments: No


[PATCH] osmo-bts[master]: LC15: Fix missing fill frame and GSM 05.08 mandatory frame

2018-03-07 Thread Vadim Yanitskiy
Hello Jenkins Builder,

I'd like you to reexamine a change.  Please visit

https://gerrit.osmocom.org/5753

to look at the new patch set (#5).

LC15: Fix missing fill frame and GSM 05.08 mandatory frame

It was discovered that the LC15 BTS does not send L2 fill frame
in case there is nothing to transmit. This leads to bad RXQUAL
reported by MS during signaling in TCH channel.

BTS needs to send L2 fill frame in case there is nothing to
transmit as indicated in GSM TS 05.08, section 8.3.

Related: OS#1950
Change-Id: I40e9bf9438c0b400e4d29eb39ffae37207e34db6
---
M src/common/msg_utils.c
M src/osmo-bts-litecell15/l1_if.c
2 files changed, 127 insertions(+), 13 deletions(-)


  git pull ssh://gerrit.osmocom.org:29418/osmo-bts refs/changes/53/5753/5

diff --git a/src/common/msg_utils.c b/src/common/msg_utils.c
index f936c98..55874cf 100644
--- a/src/common/msg_utils.c
+++ b/src/common/msg_utils.c
@@ -379,15 +379,18 @@
static const uint8_t f[] = { 52, 53, 54, 55, 56, 57, 58, 59 },
h0[] = { 0, 2, 4, 6, 52, 54, 56, 58 },
h1[] = { 14, 16, 18, 20, 66, 68, 70, 72 };
-   if (lchan->tch_mode == GSM48_CMODE_SPEECH_V1) {
+
+   switch (lchan->tch_mode) {
+   case GSM48_CMODE_SPEECH_V1:
+   case GSM48_CMODE_SPEECH_AMR:
if (lchan->type == GSM_LCHAN_TCH_F)
return fn_chk(f, fn, ARRAY_SIZE(f));
else
return fn_chk(lchan->nr ? h1 : h0, fn,
- lchan->nr ? ARRAY_SIZE(h1) :
- ARRAY_SIZE(h0));
+   lchan->nr ? ARRAY_SIZE(h1) : ARRAY_SIZE(h0));
+   default:
+   return false;
}
-   return false;
 }
 
 /*! \brief Check if DTX DL AMR is enabled for a given lchan (it have proper 
type,
@@ -465,16 +468,72 @@
  */
 uint8_t repeat_last_sid(struct gsm_lchan *lchan, uint8_t *dst, uint32_t fn)
 {
-   /* FIXME: add EFR support */
-   if (lchan->tch_mode == GSM48_CMODE_SPEECH_EFR)
+   uint8_t payload_len;
+
+   static const uint8_t amr_sid_first_zero[] = {
+   0x00, 0x00, 0x00, 0x44, 0x00, 0x00, 0x00, 0x00, 0x00
+   };
+
+   switch (lchan->tch_mode) {
+   case GSM48_CMODE_SPEECH_EFR:
+   /* FIXME: add EFR support */
return 0;
 
-   if (lchan->tch_mode != GSM48_CMODE_SPEECH_AMR) {
-   if (dtx_sched_optional(lchan, fn))
-   return 0;
-   } else
+   case GSM48_CMODE_SPEECH_AMR:
if (dtx_amr_sid_optional(lchan, fn))
return 0;
+
+   if (lchan->rsl_cmode != RSL_CMOD_SPD_SPEECH)
+   break;
+
+   if (!!lchan->tch.dtx.len)
+   break;
+
+   /* Send zero SID FIRST frame */
+   payload_len = sizeof(amr_sid_first_zero);
+   memcpy(dst, amr_sid_first_zero, payload_len);
+
+   /* Debug print */
+   LOGP(DL1C, LOGL_DEBUG, "%s %s %s: Sending zero speech frame "
+   "for tch_mode=%s\n", gsm_fn_as_gsmtime_str(fn),
+   gsm_ts_name(lchan->ts), 
get_value_string(gsm_chan_t_names, lchan->type),
+   get_value_string(gsm48_chan_mode_names, 
lchan->tch_mode));
+
+   return payload_len + 1;
+
+   default:
+   if (dtx_sched_optional(lchan, fn))
+   return 0;
+
+   if (!lchan->ts->trx->bts->dtxd)
+   break;
+
+   /* Determine the payload length */
+   switch (lchan->type) {
+   case GSM_LCHAN_TCH_F:
+   payload_len = GSM_FR_BYTES;
+   break;
+   case GSM_LCHAN_TCH_H:
+   payload_len = GSM_HR_BYTES;
+   break;
+   default:
+   return 0;
+   }
+
+   /**
+* Need to send zeroed TCH frame on mandatory fn,
+* defined in GSM TS 05.08, section 8.3
+*/
+   memset(dst, 0x00, payload_len);
+
+   /* Debug print */
+   LOGP(DL1C, LOGL_DEBUG, "%s %s %s: Sending zero speech frame "
+   "for tch_mode=%s\n", gsm_fn_as_gsmtime_str(fn),
+   gsm_ts_name(lchan->ts), 
get_value_string(gsm_chan_t_names, lchan->type),
+   get_value_string(gsm48_chan_mode_names, 
lchan->tch_mode));
+
+   return payload_len + 1;
+   }
 
if (lchan->tch.dtx.len) {
if (dtx_dl_amr_enabled(lchan)) {
@@ -502,6 +561,13 @@
}
memcpy(dst, lchan->tch.dtx.cache, lchan->tch.dtx.len);
lchan->tch.dtx.fn = fn;
+
+   /* Debug print */
+   LOGP(DL1C, LOGL_DEBUG, "%s %s %s: Sending SID buffer "
+ 

osmo-bts[master]: LC15: Fix missing fill frame and GSM 05.08 mandatory frame

2018-03-07 Thread Vadim Yanitskiy

Patch Set 4: Verified-1

I've refactored the source code to fit the common Osmocom
code style, but have no opportunity to verify and test.

URGENT: please check everything before submitting!

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I40e9bf9438c0b400e4d29eb39ffae37207e34db6
Gerrit-PatchSet: 4
Gerrit-Project: osmo-bts
Gerrit-Branch: master
Gerrit-Owner: Minh-Quang Nguyen 
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: Max 
Gerrit-Reviewer: Minh-Quang Nguyen 
Gerrit-Reviewer: Pau Espin Pedrol 
Gerrit-Reviewer: Vadim Yanitskiy 
Gerrit-HasComments: No


[PATCH] osmo-bts[master]: LC15: Fix missing fill frame and GSM 05.08 mandatory frame

2018-03-07 Thread Vadim Yanitskiy
Hello Jenkins Builder,

I'd like you to reexamine a change.  Please visit

https://gerrit.osmocom.org/5753

to look at the new patch set (#4).

LC15: Fix missing fill frame and GSM 05.08 mandatory frame

It was discovered that the LC15 BTS does not send L2 fill frame
in case there is nothing to transmit. This leads to bad RXQUAL
reported by MS during signaling in TCH channel.

BTS needs to send L2 fill frame in case there is nothing to
transmit as indicated in GSM TS 05.08, section 8.3.

Related: OS#1950
Change-Id: I40e9bf9438c0b400e4d29eb39ffae37207e34db6
---
M src/common/msg_utils.c
M src/osmo-bts-litecell15/l1_if.c
2 files changed, 127 insertions(+), 12 deletions(-)


  git pull ssh://gerrit.osmocom.org:29418/osmo-bts refs/changes/53/5753/4

diff --git a/src/common/msg_utils.c b/src/common/msg_utils.c
index f936c98..cf3f463 100644
--- a/src/common/msg_utils.c
+++ b/src/common/msg_utils.c
@@ -379,14 +379,18 @@
static const uint8_t f[] = { 52, 53, 54, 55, 56, 57, 58, 59 },
h0[] = { 0, 2, 4, 6, 52, 54, 56, 58 },
h1[] = { 14, 16, 18, 20, 66, 68, 70, 72 };
-   if (lchan->tch_mode == GSM48_CMODE_SPEECH_V1) {
+
+   switch (lchan->tch_mode) {
+   case GSM48_CMODE_SPEECH_V1:
+   case GSM48_CMODE_SPEECH_AMR:
if (lchan->type == GSM_LCHAN_TCH_F)
return fn_chk(f, fn, ARRAY_SIZE(f));
else
return fn_chk(lchan->nr ? h1 : h0, fn,
- lchan->nr ? ARRAY_SIZE(h1) :
- ARRAY_SIZE(h0));
+   lchan->nr ? ARRAY_SIZE(h1) : ARRAY_SIZE(h0));
+   break;
}
+
return false;
 }
 
@@ -465,16 +469,72 @@
  */
 uint8_t repeat_last_sid(struct gsm_lchan *lchan, uint8_t *dst, uint32_t fn)
 {
-   /* FIXME: add EFR support */
-   if (lchan->tch_mode == GSM48_CMODE_SPEECH_EFR)
+   uint8_t payload_len;
+
+   static const uint8_t amr_sid_first_zero[] = {
+   0x00, 0x00, 0x00, 0x44, 0x00, 0x00, 0x00, 0x00, 0x00
+   };
+
+   switch (lchan->tch_mode) {
+   case GSM48_CMODE_SPEECH_EFR:
+   /* FIXME: add EFR support */
return 0;
 
-   if (lchan->tch_mode != GSM48_CMODE_SPEECH_AMR) {
-   if (dtx_sched_optional(lchan, fn))
-   return 0;
-   } else
+   case GSM48_CMODE_SPEECH_AMR:
if (dtx_amr_sid_optional(lchan, fn))
return 0;
+
+   if (lchan->rsl_cmode != RSL_CMOD_SPD_SPEECH)
+   break;
+
+   if (!!lchan->tch.dtx.len)
+   break;
+
+   /* Send zero SID FIRST frame */
+   payload_len = sizeof(amr_sid_first_zero);
+   memcpy(dst, amr_sid_first_zero, payload_len);
+
+   /* Debug print */
+   LOGP(DL1C, LOGL_DEBUG, "%s %s %s: Sending zero speech frame "
+   "for tch_mode=%s\n", gsm_fn_as_gsmtime_str(fn),
+   gsm_ts_name(lchan->ts), 
get_value_string(gsm_chan_t_names, lchan->type),
+   get_value_string(gsm48_chan_mode_names, 
lchan->tch_mode));
+
+   return payload_len + 1;
+
+   default:
+   if (dtx_sched_optional(lchan, fn))
+   return 0;
+
+   if (!lchan->ts->trx->bts->dtxd)
+   break;
+
+   /* Determine the payload length */
+   switch (lchan->type) {
+   case GSM_LCHAN_TCH_F:
+   payload_len = GSM_FR_BYTES;
+   break;
+   case GSM_LCHAN_TCH_H:
+   payload_len = GSM_HR_BYTES;
+   break;
+   default:
+   return 0;
+   }
+
+   /**
+* Need to send zeroed TCH frame on mandatory fn,
+* defined in GSM TS 05.08, section 8.3
+*/
+   memset(dst, 0x00, payload_len);
+
+   /* Debug print */
+   LOGP(DL1C, LOGL_DEBUG, "%s %s %s: Sending zero speech frame "
+   "for tch_mode=%s\n", gsm_fn_as_gsmtime_str(fn),
+   gsm_ts_name(lchan->ts), 
get_value_string(gsm_chan_t_names, lchan->type),
+   get_value_string(gsm48_chan_mode_names, 
lchan->tch_mode));
+
+   return payload_len + 1;
+   }
 
if (lchan->tch.dtx.len) {
if (dtx_dl_amr_enabled(lchan)) {
@@ -502,6 +562,13 @@
}
memcpy(dst, lchan->tch.dtx.cache, lchan->tch.dtx.len);
lchan->tch.dtx.fn = fn;
+
+   /* Debug print */
+   LOGP(DL1C, LOGL_DEBUG, "%s %s %s: Sending SID buffer "
+   "for tch_mode=%s\n", gsm_fn_as_gsmtime_str(fn),
+   

osmo-bts[master]: LC15: Fix missing fill frame and GSM 05.08 mandatory frame

2018-02-13 Thread Minh-Quang Nguyen

Patch Set 1:

> (1 comment)

Thanks for your comments. I will refactor the codes to make them cleaner at 
some point.

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I40e9bf9438c0b400e4d29eb39ffae37207e34db6
Gerrit-PatchSet: 1
Gerrit-Project: osmo-bts
Gerrit-Branch: master
Gerrit-Owner: Minh-Quang Nguyen 
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: Max 
Gerrit-Reviewer: Minh-Quang Nguyen 
Gerrit-Reviewer: Pau Espin Pedrol 
Gerrit-Reviewer: Vadim Yanitskiy 
Gerrit-HasComments: No


osmo-bts[master]: LC15: Fix missing fill frame and GSM 05.08 mandatory frame

2018-02-13 Thread Vadim Yanitskiy

Patch Set 3:

(1 comment)

https://gerrit.osmocom.org/#/c/5753/3//COMMIT_MSG
Commit Message:

Line 13:  Related issue: https://osmocom.org/issues/1950
Please move close to the change-id and change to:

Related: OS#1950


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I40e9bf9438c0b400e4d29eb39ffae37207e34db6
Gerrit-PatchSet: 3
Gerrit-Project: osmo-bts
Gerrit-Branch: master
Gerrit-Owner: Minh-Quang Nguyen 
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: Max 
Gerrit-Reviewer: Minh-Quang Nguyen 
Gerrit-Reviewer: Pau Espin Pedrol 
Gerrit-Reviewer: Vadim Yanitskiy 
Gerrit-HasComments: Yes


osmo-bts[master]: LC15: Fix missing fill frame and GSM 05.08 mandatory frame

2018-02-13 Thread Vadim Yanitskiy

Patch Set 3: Code-Review-1

And finally, regarding to the commit message: we usually follow
a shorter line length in description, so I would be happy if you
limit it as other contributers do. But this is cosmetics...

-1 due to some code warnings...

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I40e9bf9438c0b400e4d29eb39ffae37207e34db6
Gerrit-PatchSet: 3
Gerrit-Project: osmo-bts
Gerrit-Branch: master
Gerrit-Owner: Minh-Quang Nguyen 
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: Max 
Gerrit-Reviewer: Minh-Quang Nguyen 
Gerrit-Reviewer: Pau Espin Pedrol 
Gerrit-Reviewer: Vadim Yanitskiy 
Gerrit-HasComments: No


osmo-bts[master]: LC15: Fix missing fill frame and GSM 05.08 mandatory frame

2018-02-13 Thread Vadim Yanitskiy

Patch Set 2:

(6 comments)

https://gerrit.osmocom.org/#/c/5753/2/src/common/msg_utils.c
File src/common/msg_utils.c:

Line 484:   if (lchan->tch_mode != GSM48_CMODE_SPEECH_AMR) {
Moreover, I would recommend to use a switch here:

switch (lchan->tch_mode) ...


Line 487:   else {
Here you can avoid the usage of else block, and thus reduce
the code nesting, because the function will return if DTX is
optional for particular combination of lchan and fn...


Line 502:   LOGP(DL1C, LOGL_DEBUG, "%s Have to send 
%s %s zero speech frame, Fn=%d, Fn mod 104=%d, dump=%s\n",
This code block is already shifted away from the line
beginning, so I would use a single tab for such alignment.


Line 518:   LOGP(DL1C, LOGL_DEBUG, "%s Have to send 
%s %s zero speech frame, Fn=%d, Fn mod 104=%d, dump=%s\n",
Same here, single tab for arguments would be better.


Line 534: 
Why do we have this line here?

It's not related to the change anyhow, so please remove.


Line 560:   LOGP(DL1C, LOGL_DEBUG, "%s Have to send %s SID buffer "
Please also unify the alignment here...


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I40e9bf9438c0b400e4d29eb39ffae37207e34db6
Gerrit-PatchSet: 2
Gerrit-Project: osmo-bts
Gerrit-Branch: master
Gerrit-Owner: Minh-Quang Nguyen 
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: Max 
Gerrit-Reviewer: Minh-Quang Nguyen 
Gerrit-Reviewer: Pau Espin Pedrol 
Gerrit-Reviewer: Vadim Yanitskiy 
Gerrit-HasComments: Yes


[PATCH] osmo-bts[master]: LC15: Fix missing fill frame and GSM 05.08 mandatory frame

2018-02-13 Thread Minh-Quang Nguyen
Hello Vadim Yanitskiy, Jenkins Builder,

I'd like you to reexamine a change.  Please visit

https://gerrit.osmocom.org/5753

to look at the new patch set (#2).

LC15: Fix missing fill frame and GSM 05.08 mandatory frame

Problem:
 We have noticed that the LC15 BTS does not send L2 fill frame in case there is 
nothing to transmit.
 This leads to bad RXQUAL repored by MS during signaling in TCH channel.

 Related issue: https://osmocom.org/issues/1950

Fixes:
 BTS needs to send L2 fill frame in case there is nothing to transmit as 
indicated
 in GSM 05.08, section 8.3.

 "On any TCH this subset of TDMA frames is always used for transmission during 
DTX. For speech, when
 no signalling or speech is to be transmitted these TDMA frames are occupied by 
the SID (Silence
 Descriptor) speech frame, see TS GSM 06.12 and TSM GSM 06.31 for detailed 
specification of the SID
 frame and its transmission requirements. In other cases when no information is 
required to be transmitted,
 e.g. on data channels, the L2 fill frame (see GSM 04.06 section 5.4.2.3) shall 
be transmitted as a FACCH
 in the TDMA frame subset always to be transmitted.
 On the SDCCH and on the half rate speech traffic channel in signalling only 
mode DTX is not allowed. In
 these cases and during signalling on the TCH when DTX is not used, the same L2 
fill frame shall be
 transmitted in case there is nothing else to transmit."

Change-Id: I40e9bf9438c0b400e4d29eb39ffae37207e34db6
---
M src/common/msg_utils.c
M src/osmo-bts-litecell15/l1_if.c
2 files changed, 119 insertions(+), 6 deletions(-)


  git pull ssh://gerrit.osmocom.org:29418/osmo-bts refs/changes/53/5753/2

diff --git a/src/common/msg_utils.c b/src/common/msg_utils.c
index f936c98..370dd09 100644
--- a/src/common/msg_utils.c
+++ b/src/common/msg_utils.c
@@ -36,6 +36,17 @@
 
 #define STI_BIT_MASK 16
 
+static const uint8_t gsm_speech_zero[GSM_FR_BYTES] = {
+   0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+   0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+   0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+   0x00, 0x00, 0x00
+};
+
+static const uint8_t amr_sid_first_zero[9] = {
+   0x00, 0x00, 0x00, 0x44, 0x00, 0x00, 0x00, 0x00, 0x00
+};
+
 static int check_fom(struct abis_om_hdr *omh, size_t len)
 {
if (omh->length != len) {
@@ -379,7 +390,7 @@
static const uint8_t f[] = { 52, 53, 54, 55, 56, 57, 58, 59 },
h0[] = { 0, 2, 4, 6, 52, 54, 56, 58 },
h1[] = { 14, 16, 18, 20, 66, 68, 70, 72 };
-   if (lchan->tch_mode == GSM48_CMODE_SPEECH_V1) {
+   if ((lchan->tch_mode == GSM48_CMODE_SPEECH_V1) || (lchan->tch_mode == 
GSM48_CMODE_SPEECH_AMR)){
if (lchan->type == GSM_LCHAN_TCH_F)
return fn_chk(f, fn, ARRAY_SIZE(f));
else
@@ -465,6 +476,7 @@
  */
 uint8_t repeat_last_sid(struct gsm_lchan *lchan, uint8_t *dst, uint32_t fn)
 {
+   uint8_t pl_len;
/* FIXME: add EFR support */
if (lchan->tch_mode == GSM48_CMODE_SPEECH_EFR)
return 0;
@@ -472,11 +484,54 @@
if (lchan->tch_mode != GSM48_CMODE_SPEECH_AMR) {
if (dtx_sched_optional(lchan, fn))
return 0;
-   } else
+   else {
+   if (lchan->ts->trx->bts->dtxd) {
+   /* Need to send zeroed TCH frame on mandatory 
Fn defined in GSM 05.08, section 8.3 */
+   switch (lchan->type) {
+   case GSM_LCHAN_TCH_F:
+   pl_len = GSM_FR_BYTES;
+   break;
+   case GSM_LCHAN_TCH_H:
+   pl_len = GSM_HR_BYTES;
+   break;
+   default:
+   return 0;
+   }
+   memcpy(dst, gsm_speech_zero, pl_len);
+
+   LOGP(DL1C, LOGL_DEBUG, "%s Have to send %s %s 
zero speech frame, Fn=%d, Fn mod 104=%d, dump=%s\n",
+   
gsm_lchan_name(lchan),
+   
get_value_string(gsm_chan_t_names, lchan->type),
+   
get_value_string(gsm48_chan_mode_names, lchan->tch_mode),
+   fn,
+   fn%104,
+   
osmo_hexdump(dst, pl_len));
+   return pl_len + 1;
+   }
+   }
+   } else {
+   

osmo-bts[master]: LC15: Fix missing fill frame and GSM 05.08 mandatory frame

2018-02-13 Thread Minh-Quang Nguyen

Patch Set 1:

> > ping. Minh-Quang do you plan to submit a new version soon? seems
 > > like a few minutes job but it's been waiting for over a month.
 > 
 > Unfortunately, my local branch of this topic has been lost... is
 > there anyway to pull this topic to my local BTS repository?

Just ignore my question. I have just found out there is a Download button at 
Gerrit GUI to pull this topic to my machine.

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I40e9bf9438c0b400e4d29eb39ffae37207e34db6
Gerrit-PatchSet: 1
Gerrit-Project: osmo-bts
Gerrit-Branch: master
Gerrit-Owner: Minh-Quang Nguyen 
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: Max 
Gerrit-Reviewer: Minh-Quang Nguyen 
Gerrit-Reviewer: Pau Espin Pedrol 
Gerrit-Reviewer: Vadim Yanitskiy 
Gerrit-HasComments: No


osmo-bts[master]: LC15: Fix missing fill frame and GSM 05.08 mandatory frame

2018-02-13 Thread Minh-Quang Nguyen

Patch Set 1:

> ping. Minh-Quang do you plan to submit a new version soon? seems
 > like a few minutes job but it's been waiting for over a month.

Unfortunately, my local branch of this topic has been lost... is there anyway 
to pull this topic to my local BTS repository?

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I40e9bf9438c0b400e4d29eb39ffae37207e34db6
Gerrit-PatchSet: 1
Gerrit-Project: osmo-bts
Gerrit-Branch: master
Gerrit-Owner: Minh-Quang Nguyen 
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: Max 
Gerrit-Reviewer: Minh-Quang Nguyen 
Gerrit-Reviewer: Pau Espin Pedrol 
Gerrit-Reviewer: Vadim Yanitskiy 
Gerrit-HasComments: No


osmo-bts[master]: LC15: Fix missing fill frame and GSM 05.08 mandatory frame

2018-02-13 Thread Minh-Quang Nguyen

Patch Set 1:

> ping. Minh-Quang do you plan to submit a new version soon? seems
 > like a few minutes job but it's been waiting for over a month.

Hi Paul, 

I am sorry for the delay. I think that we can simply replace the static const 
by const declaration in msg_util.h.

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I40e9bf9438c0b400e4d29eb39ffae37207e34db6
Gerrit-PatchSet: 1
Gerrit-Project: osmo-bts
Gerrit-Branch: master
Gerrit-Owner: Minh-Quang Nguyen 
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: Max 
Gerrit-Reviewer: Minh-Quang Nguyen 
Gerrit-Reviewer: Pau Espin Pedrol 
Gerrit-Reviewer: Vadim Yanitskiy 
Gerrit-HasComments: No


osmo-bts[master]: LC15: Fix missing fill frame and GSM 05.08 mandatory frame

2018-02-13 Thread Max

Patch Set 1:

Do we need the same fix for sysmoBTS as well?

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I40e9bf9438c0b400e4d29eb39ffae37207e34db6
Gerrit-PatchSet: 1
Gerrit-Project: osmo-bts
Gerrit-Branch: master
Gerrit-Owner: Minh-Quang Nguyen 
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: Max 
Gerrit-Reviewer: Pau Espin Pedrol 
Gerrit-Reviewer: Vadim Yanitskiy 
Gerrit-HasComments: No


osmo-bts[master]: LC15: Fix missing fill frame and GSM 05.08 mandatory frame

2018-02-13 Thread Pau Espin Pedrol

Patch Set 1:

ping. Minh-Quang do you plan to submit a new version soon? seems like a few 
minutes job but it's been waiting for over a month.

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I40e9bf9438c0b400e4d29eb39ffae37207e34db6
Gerrit-PatchSet: 1
Gerrit-Project: osmo-bts
Gerrit-Branch: master
Gerrit-Owner: Minh-Quang Nguyen 
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: Pau Espin Pedrol 
Gerrit-Reviewer: Vadim Yanitskiy 
Gerrit-HasComments: No


osmo-bts[master]: LC15: Fix missing fill frame and GSM 05.08 mandatory frame

2018-01-11 Thread Vadim Yanitskiy

Patch Set 1: Code-Review-1

(1 comment)

https://gerrit.osmocom.org/#/c/5753/1/include/osmo-bts/msg_utils.h
File include/osmo-bts/msg_utils.h:

PS1, Line 35: static const uint8_t gsm_speech_zero[GSM_FR_BYTES] = {
:   0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 
0x00,
:   0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 
0x00,
:   0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 
0x00,
:   0x00, 0x00, 0x00
: };
: 
: static const uint8_t amr_sid_first_zero[9] = {
:   0x00, 0x00, 0x00, 0x44, 0x00, 0x00, 0x00, 0x00, 0x00
: };
Do we really need static struct definitions within the header?

I had only a quick look, but it seems they only used by msg_utils.c...
If I am correct, they shouldn't be here.


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I40e9bf9438c0b400e4d29eb39ffae37207e34db6
Gerrit-PatchSet: 1
Gerrit-Project: osmo-bts
Gerrit-Branch: master
Gerrit-Owner: Minh-Quang Nguyen 
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: Vadim Yanitskiy 
Gerrit-HasComments: Yes


[PATCH] osmo-bts[master]: LC15: Fix missing fill frame and GSM 05.08 mandatory frame

2018-01-11 Thread Minh-Quang Nguyen

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

LC15: Fix missing fill frame and GSM 05.08 mandatory frame

Problem:
 We have noticed that the LC15 BTS does not send L2 fill frame in case there is 
nothing to transmit.
 This leads to bad RXQUAL repored by MS during signaling in TCH channel.

 Related issue: https://osmocom.org/issues/1950

Fixes:
 BTS needs to send L2 fill frame in case there is nothing to transmit as 
indicated
 in GSM 05.08, section 8.3.

 "On any TCH this subset of TDMA frames is always used for transmission during 
DTX. For speech, when
 no signalling or speech is to be transmitted these TDMA frames are occupied by 
the SID (Silence
 Descriptor) speech frame, see TS GSM 06.12 and TSM GSM 06.31 for detailed 
specification of the SID
 frame and its transmission requirements. In other cases when no information is 
required to be transmitted,
 e.g. on data channels, the L2 fill frame (see GSM 04.06 section 5.4.2.3) shall 
be transmitted as a FACCH
 in the TDMA frame subset always to be transmitted.
 On the SDCCH and on the half rate speech traffic channel in signalling only 
mode DTX is not allowed. In
 these cases and during signalling on the TCH when DTX is not used, the same L2 
fill frame shall be
 transmitted in case there is nothing else to transmit."

Change-Id: I40e9bf9438c0b400e4d29eb39ffae37207e34db6
---
M include/osmo-bts/msg_utils.h
M src/common/msg_utils.c
M src/osmo-bts-litecell15/l1_if.c
3 files changed, 119 insertions(+), 6 deletions(-)


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

diff --git a/include/osmo-bts/msg_utils.h b/include/osmo-bts/msg_utils.h
index 7ddbe88..24534ac 100644
--- a/include/osmo-bts/msg_utils.h
+++ b/include/osmo-bts/msg_utils.h
@@ -32,6 +32,17 @@
OML_MSG_TYPE_OSMO,
 };
 
+static const uint8_t gsm_speech_zero[GSM_FR_BYTES] = {
+   0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+   0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+   0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+   0x00, 0x00, 0x00
+};
+
+static const uint8_t amr_sid_first_zero[9] = {
+   0x00, 0x00, 0x00, 0x44, 0x00, 0x00, 0x00, 0x00, 0x00
+};
+
 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);
diff --git a/src/common/msg_utils.c b/src/common/msg_utils.c
index f936c98..41fe990 100644
--- a/src/common/msg_utils.c
+++ b/src/common/msg_utils.c
@@ -379,7 +379,7 @@
static const uint8_t f[] = { 52, 53, 54, 55, 56, 57, 58, 59 },
h0[] = { 0, 2, 4, 6, 52, 54, 56, 58 },
h1[] = { 14, 16, 18, 20, 66, 68, 70, 72 };
-   if (lchan->tch_mode == GSM48_CMODE_SPEECH_V1) {
+   if ((lchan->tch_mode == GSM48_CMODE_SPEECH_V1) || (lchan->tch_mode == 
GSM48_CMODE_SPEECH_AMR)){
if (lchan->type == GSM_LCHAN_TCH_F)
return fn_chk(f, fn, ARRAY_SIZE(f));
else
@@ -465,6 +465,7 @@
  */
 uint8_t repeat_last_sid(struct gsm_lchan *lchan, uint8_t *dst, uint32_t fn)
 {
+   uint8_t pl_len;
/* FIXME: add EFR support */
if (lchan->tch_mode == GSM48_CMODE_SPEECH_EFR)
return 0;
@@ -472,11 +473,54 @@
if (lchan->tch_mode != GSM48_CMODE_SPEECH_AMR) {
if (dtx_sched_optional(lchan, fn))
return 0;
-   } else
+   else {
+   if (lchan->ts->trx->bts->dtxd) {
+   /* Need to send zeroed TCH frame on mandatory 
Fn defined in GSM 05.08, section 8.3 */
+   switch (lchan->type) {
+   case GSM_LCHAN_TCH_F:
+   pl_len = GSM_FR_BYTES;
+   break;
+   case GSM_LCHAN_TCH_H:
+   pl_len = GSM_HR_BYTES;
+   break;
+   default:
+   return 0;
+   }
+   memcpy(dst, gsm_speech_zero, pl_len);
+
+   LOGP(DL1C, LOGL_DEBUG, "%s Have to send %s %s 
zero speech frame, Fn=%d, Fn mod 104=%d, dump=%s\n",
+   
gsm_lchan_name(lchan),
+   
get_value_string(gsm_chan_t_names, lchan->type),
+   
get_value_string(gsm48_chan_mode_names, lchan->tch_mode),
+   fn,
+   fn%104,
+