fixeria has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-bts/+/33069 )
Change subject: trx TCH DL: transmit invalid speech frames instead of dummy FACCH
......................................................................
Patch Set 2:
(1 comment)
File src/osmo-bts-trx/sched_lchan_tchf.c:
https://gerrit.osmocom.org/c/osmo-bts/+/33069/comment/320f8fa7_2f92e7c3
PS1, Line 500: what is the correct BTS Tx behavior for frame
: * gaps in TCH/AFS
> Please read the comment I just added under that ticket. […]
I just suggested a possible solution seeing a FIXME comment in your patch. It's not a high priority for me either, and unfortunately I don't have access to my nanoBTS units anymore to confirm that it behaves as I described. This can be easily checked by establishing a silent call from osmo-msc and looking at the Um interface captures.
--
To view, visit https://gerrit.osmocom.org/c/osmo-bts/+/33069
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings
Gerrit-Project: osmo-bts
Gerrit-Branch: master
Gerrit-Change-Id: I78106802a0aa4af39859c75d29fe0e77037899fe
Gerrit-Change-Number: 33069
Gerrit-PatchSet: 2
Gerrit-Owner: falconia <falcon(a)freecalypso.org>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: fixeria <vyanitskiy(a)sysmocom.de>
Gerrit-Reviewer: pespin <pespin(a)sysmocom.de>
Gerrit-Comment-Date: Mon, 29 May 2023 19:15:19 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: falconia <falcon(a)freecalypso.org>
Comment-In-Reply-To: fixeria <vyanitskiy(a)sysmocom.de>
Gerrit-MessageType: comment
falconia has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-bts/+/33069 )
Change subject: trx TCH DL: transmit invalid speech frames instead of dummy FACCH
......................................................................
Patch Set 2:
(1 comment)
Patchset:
PS2:
> Why is Gerrit showing "Merge Conflict" on my latest patchset 2? The commit I pushed to refs/for/mast […]
It looks like it just resolved itself - the "Merge Conflict" heading disappeared, replaced with the expected "Active".
--
To view, visit https://gerrit.osmocom.org/c/osmo-bts/+/33069
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings
Gerrit-Project: osmo-bts
Gerrit-Branch: master
Gerrit-Change-Id: I78106802a0aa4af39859c75d29fe0e77037899fe
Gerrit-Change-Number: 33069
Gerrit-PatchSet: 2
Gerrit-Owner: falconia <falcon(a)freecalypso.org>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: fixeria <vyanitskiy(a)sysmocom.de>
Gerrit-Reviewer: pespin <pespin(a)sysmocom.de>
Gerrit-Comment-Date: Mon, 29 May 2023 19:09:36 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: falconia <falcon(a)freecalypso.org>
Gerrit-MessageType: comment
falconia has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-bts/+/33069 )
Change subject: trx TCH DL: transmit invalid speech frames instead of dummy FACCH
......................................................................
Patch Set 2:
(1 comment)
Patchset:
PS2:
Why is Gerrit showing "Merge Conflict" on my latest patchset 2? The commit I pushed to refs/for/master for PS2 applies directly on top of current master branch tip, how can it be a merge conflict? Would it be possible to get a Gerrit admin to look at this issue?
--
To view, visit https://gerrit.osmocom.org/c/osmo-bts/+/33069
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings
Gerrit-Project: osmo-bts
Gerrit-Branch: master
Gerrit-Change-Id: I78106802a0aa4af39859c75d29fe0e77037899fe
Gerrit-Change-Number: 33069
Gerrit-PatchSet: 2
Gerrit-Owner: falconia <falcon(a)freecalypso.org>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: fixeria <vyanitskiy(a)sysmocom.de>
Gerrit-Reviewer: pespin <pespin(a)sysmocom.de>
Gerrit-Comment-Date: Mon, 29 May 2023 19:08:04 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Gerrit-MessageType: comment
Attention is currently required from: falconia.
fixeria has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-bts/+/33069 )
Change subject: trx TCH DL: transmit invalid speech frames instead of dummy FACCH
......................................................................
Patch Set 2: Code-Review+1
(1 comment)
File src/osmo-bts-trx/sched_lchan_tchf.c:
https://gerrit.osmocom.org/c/osmo-bts/+/33069/comment/a0385618_8ed5f398
PS1, Line 505: gsm0503_tch_fr_encode
> I will look into implementing your suggestion in the next revision of my patch.
Done
--
To view, visit https://gerrit.osmocom.org/c/osmo-bts/+/33069
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings
Gerrit-Project: osmo-bts
Gerrit-Branch: master
Gerrit-Change-Id: I78106802a0aa4af39859c75d29fe0e77037899fe
Gerrit-Change-Number: 33069
Gerrit-PatchSet: 2
Gerrit-Owner: falconia <falcon(a)freecalypso.org>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: fixeria <vyanitskiy(a)sysmocom.de>
Gerrit-Reviewer: pespin <pespin(a)sysmocom.de>
Gerrit-Attention: falconia <falcon(a)freecalypso.org>
Gerrit-Comment-Date: Mon, 29 May 2023 19:03:31 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: Yes
Comment-In-Reply-To: falconia <falcon(a)freecalypso.org>
Comment-In-Reply-To: fixeria <vyanitskiy(a)sysmocom.de>
Gerrit-MessageType: comment
Attention is currently required from: fixeria.
Hello Jenkins Builder, pespin,
I'd like you to reexamine a change. Please visit
https://gerrit.osmocom.org/c/osmo-bts/+/33069
to look at the new patch set (#2).
Change subject: trx TCH DL: transmit invalid speech frames instead of dummy FACCH
......................................................................
trx TCH DL: transmit invalid speech frames instead of dummy FACCH
In speech TCH operation, there will always be times when a speech frame
needs to be transmitted on the downlink, but there is no frame available
to transmit (gap in the incoming RTP stream), or the logic of DTXd says
that no frame shall be transmitted at this FN, but we are not doing
physical DTXd. Previous osmo-bts-trx code sent dummy FACCH during such
conditions, but this approach is bad for TCH/HS where we would like to
transmit good speech frames or speech frame gaps one 20 ms frame at a
time, rather than take out pairs of frames for dummy FACCH/H. Other
BTS models (sysmoBTS PHY) send out invalid speech frames with inverted
CRC3 under the conditions in question - do the same in osmo-bts-trx.
The present change modifies osmo-bts-trx TCH DL frame gap behavior
only for TCH/FS, TCH/HS and TCH/EFS; for all other channel modes,
including AMR speech, the previous behavior of sending dummy FACCH
is left unchanged.
Depends: Iade3310e16b906efb6892d28f474a0d15204e861 (libosmocore)
Change-Id: I78106802a0aa4af39859c75d29fe0e77037899fe
---
M src/osmo-bts-trx/sched_lchan_tchf.c
M src/osmo-bts-trx/sched_lchan_tchh.c
2 files changed, 62 insertions(+), 2 deletions(-)
git pull ssh://gerrit.osmocom.org:29418/osmo-bts refs/changes/69/33069/2
--
To view, visit https://gerrit.osmocom.org/c/osmo-bts/+/33069
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings
Gerrit-Project: osmo-bts
Gerrit-Branch: master
Gerrit-Change-Id: I78106802a0aa4af39859c75d29fe0e77037899fe
Gerrit-Change-Number: 33069
Gerrit-PatchSet: 2
Gerrit-Owner: falconia <falcon(a)freecalypso.org>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: pespin <pespin(a)sysmocom.de>
Gerrit-CC: fixeria <vyanitskiy(a)sysmocom.de>
Gerrit-Attention: fixeria <vyanitskiy(a)sysmocom.de>
Gerrit-MessageType: newpatchset
Attention is currently required from: fixeria.
falconia has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-bts/+/33069 )
Change subject: trx TCH DL: transmit invalid speech frames instead of dummy FACCH
......................................................................
Patch Set 1:
(2 comments)
File src/osmo-bts-trx/sched_lchan_tchf.c:
https://gerrit.osmocom.org/c/osmo-bts/+/33069/comment/fa2297b7_925e210c
PS1, Line 500: what is the correct BTS Tx behavior for frame
: * gaps in TCH/AFS
> I guess we could indicate a bad frame via AMR's in-band signalling? Just an idea. […]
Please read the comment I just added under that ticket. Basically the short version is that I consider myself qualified only to work on FR/HR/EFR, i.e., make code improvements for these 3 codecs and review other developers' changes where they affect these non-AMR codecs. When it comes to AMR, I do not feel so qualified, hence I have to defer to other developers for whom AMR is a higher priority than it is for me. If you (fixeria) or any other developer has a solution in mind for AMR (doing something better than transmitting dummy FACCH, especially on TCH/H), perhaps you can implement it as a follow-on patch after my fix for FR/HR/EFR?
https://gerrit.osmocom.org/c/osmo-bts/+/33069/comment/600dc842_02da4934
PS1, Line 505: gsm0503_tch_fr_encode
> I suggest using a switch here: […]
I will look into implementing your suggestion in the next revision of my patch.
--
To view, visit https://gerrit.osmocom.org/c/osmo-bts/+/33069
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings
Gerrit-Project: osmo-bts
Gerrit-Branch: master
Gerrit-Change-Id: I78106802a0aa4af39859c75d29fe0e77037899fe
Gerrit-Change-Number: 33069
Gerrit-PatchSet: 1
Gerrit-Owner: falconia <falcon(a)freecalypso.org>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: pespin <pespin(a)sysmocom.de>
Gerrit-CC: fixeria <vyanitskiy(a)sysmocom.de>
Gerrit-Attention: fixeria <vyanitskiy(a)sysmocom.de>
Gerrit-Comment-Date: Mon, 29 May 2023 18:39:39 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: fixeria <vyanitskiy(a)sysmocom.de>
Gerrit-MessageType: comment
fixeria has uploaded this change for review. ( https://gerrit.osmocom.org/c/osmo-bts/+/33076 )
Change subject: osmo-bts-trx: visualize rx_tch[fh]_fn() functions
......................................................................
osmo-bts-trx: visualize rx_tch[fh]_fn() functions
Change-Id: I373dbbc3d427858f76d07ff85633e07fe2600770
Related: OS#1572
---
A doc/trx_sched_tch.txt
M src/osmo-bts-trx/sched_lchan_tchf.c
M src/osmo-bts-trx/sched_lchan_tchh.c
3 files changed, 112 insertions(+), 2 deletions(-)
git pull ssh://gerrit.osmocom.org:29418/osmo-bts refs/changes/76/33076/1
diff --git a/doc/trx_sched_tch.txt b/doc/trx_sched_tch.txt
new file mode 100644
index 0000000..f8c79ff
--- /dev/null
+++ b/doc/trx_sched_tch.txt
@@ -0,0 +1,98 @@
+== rx_tchf_fn(): TCH/FS, TCH/EFS, TCH/AFS, TCH/F2.4, and FACCH/F
+
+ 00 01 02 03 04 05 06 07 08 09 10 11 12 13 14 15 16 17 18 19 20 21 22 23
++---+---+---+---+---+---+---+---+---+---+---+---+---+---+---+---|---+---+---+---+---+---+---+---+ << 4
+| | | | | | | | | | | | | | | | | | | | | a | b | c | d | Rx bid={0,1,2,3}, decode
++---+---+---+---+---+---+---+---+---+---+---+---+---+---+---+---|---+---+---+---+---+---+---+---+ << 4
+| | | | | | | | | | | | | | | | | a | b | c | d | e | f | g | h | Rx bid={0,1,2,3}, decode
++---+---+---+---+---+---+---+---+---+---+---+---+---+---+---+---|---+---+---+---+---+---+---+---+ << 4
+ |
+ |<~~~~~~~~~~~~~~~~~~~~~~~~~~~~~>| frame A
+ | |<~~~~~~~~~~~~~~~~~~~~~~~~~~~~~>| frame B
+ @ decoding from here
+
+
+== rx_tchf_fn(): TCH/F14.4, TCH/F9.6, TCH/F4.8
+
+ 00 01 02 03 04 05 06 07 08 09 10 11 12 13 14 15 16 17 18 19 20 21 22 23
+|---+---+---+---+---+---+---+---+---+---+---+---+---+---+---+---+---+---+---+---+---+---+---+---+ << 4
+| | | | | | | | | | | | | | | | | | | | | a | b | c | d | Rx bid={0,1,2,3}, decode
+|---+---+---+---+---+---+---+---+---+---+---+---+---+---+---+---+---+---+---+---+---+---+---+---+ << 4
+| | | | | | | | | | | | | | | | | a | b | c | d | e | f | g | h | Rx bid={0,1,2,3}, decode
+|---+---+---+---+---+---+---+---+---+---+---+---+---+---+---+---+---+---+---+---+---+---+---+---+ << 4
+| | | | | | | | | | | | | a | b | c | d | e | f | g | h | i | j | k | l | Rx bid={0,1,2,3}, decode
+|---+---+---+---+---+---+---+---+---+---+---+---+---+---+---+---+---+---+---+---+---+---+---+---+ << 4
+| | | | | | | | | a | b | c | d | e | f | g | h | i | j | k | l | m | n | o | p | Rx bid={0,1,2,3}, decode
+|---+---+---+---+---+---+---+---+---+---+---+---+---+---+---+---+---+---+---+---+---+---+---+---+ << 4
+| | | | | a | b | c | d | e | f | g | h | i | j | k | l | m | n | o | p | q | r | s | t | Rx bid={0,1,2,3}, decode
+|---+---+---+---+---+---+---+---+---+---+---+---+---+---+---+---+---+---+---+---+---+---+---+---+ << 4
+| a | b | c | d | e | f | g | h | i | j | k | l | m | n | o | p | q | r | s | t | u | v | w | x | Rx bid={0,1,2,3}, decode
+|---+---+---+---+---+---+---+---+---+---+---+---+---+---+---+---+---+---+---+---+---+---+---+---+ << 4
+|
+|<~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~>| frame A
+| |<~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~>| frame B
+@ decoding from here
+
+
+== rx_tchh_fn(): TCH/HS, TCH/AHS
+
+ 00 01 02 03 04 05 06 07 08 09 10 11 12 13 14 15 16 17 18 19 20 21 22 23
++---+---+---+---+---+---+---+---+---+---+---+---+---+---+---+---|---+---+---+---+---+---+---+---+ << 2
+| | | | | | | | | | | | | | | | | | | | | a | b | | | Rx bid={0,1}, decode
++---+---+---+---+---+---+---+---+---+---+---+---+---+---+---+---|---+---+---+---+---+---+---+---+ << 2
+| | | | | | | | | | | | | | | | | | | a | b | c | d | | | Rx bid={0,1}, decode
++---+---+---+---+---+---+---+---+---+---+---+---+---+---+---+---|---+---+---+---+---+---+---+---+ << 2
+| | | | | | | | | | | | | | | | | a | b | c | d | e | f | | | Rx bid={0,1}, decode
++---+---+---+---+---+---+---+---+---+---+---+---+---+---+---+---|---+---+---+---+---+---+---+---+ << 2
+ |
+ |<~~~~~~~~~~~~~>| frame A
+ | |<~~~~~~~~~~~~~>| frame B
+ @ decoding from here
+
+
+== rx_tchh_fn(): FACCH/H
+
+ 00 01 02 03 04 05 06 07 08 09 10 11 12 13 14 15 16 17 18 19 20 21 22 23
++---+---+---+---+---+---+---+---+---+---+---+---+---+---+---+---|---+---+---+---+---+---+---+---+ << 2
+| | | | | | | | | | | | | | | | | | | | | a | b | | | Rx bid={0,1}, decode
++---+---+---+---+---+---+---+---+---+---+---+---+---+---+---+---|---+---+---+---+---+---+---+---+ << 2
+| | | | | | | | | | | | | | | | | | | a | b | c | d | | | Rx bid={0,1}
++---+---+---+---+---+---+---+---+---+---+---+---+---+---+---+---|---+---+---+---+---+---+---+---+ << 2
+| | | | | | | | | | | | | | | | | a | b | c | d | e | f | | | Rx bid={0,1}, decode
++---+---+---+---+---+---+---+---+---+---+---+---+---+---+---+---|---+---+---+---+---+---+---+---+ << 2
+ |
+ |<~~~~~~~~~~~~~~~~~~~~~>| frame A
+ | |<~~~~~~~~~~~~~~~~~~~~~>| frame B
+ @ decoding from here
+
+
+== rx_tchh_fn(): TCH/H4.8, TCH/H2.4
+
+ 00 01 02 03 04 05 06 07 08 09 10 11 12 13 14 15 16 17 18 19 20 21 22 23
+|---+---+---+---+---+---+---+---+---+---+---+---+---+---+---+---+---+---+---+---+---+---+---+---+ << 2
+| | | | | | | | | | | | | | | | | | | | | a | b | | | Rx bid={0,1}, decode
+|---+---+---+---+---+---+---+---+---+---+---+---+---+---+---+---+---+---+---+---+---+---+---+---+ << 2
+| | | | | | | | | | | | | | | | | | | a | b | c | d | | | Rx bid={0,1}
+|---+---+---+---+---+---+---+---+---+---+---+---+---+---+---+---+---+---+---+---+---+---+---+---+ << 2
+| | | | | | | | | | | | | | | | | a | b | c | d | e | f | | | Rx bid={0,1}, decode
+|---+---+---+---+---+---+---+---+---+---+---+---+---+---+---+---+---+---+---+---+---+---+---+---+ << 2
+| | | | | | | | | | | | | | | a | b | c | d | e | f | g | h | | | Rx bid={0,1}
+|---+---+---+---+---+---+---+---+---+---+---+---+---+---+---+---+---+---+---+---+---+---+---+---+ << 2
+| | | | | | | | | | | | | a | b | c | d | e | f | g | h | i | j | | | Rx bid={0,1}, decode
+|---+---+---+---+---+---+---+---+---+---+---+---+---+---+---+---+---+---+---+---+---+---+---+---+ << 2
+| | | | | | | | | | | a | b | c | d | e | f | g | h | i | j | k | l | | | Rx bid={0,1}
+|---+---+---+---+---+---+---+---+---+---+---+---+---+---+---+---+---+---+---+---+---+---+---+---+ << 2
+| | | | | | | | | a | b | c | d | e | f | g | h | i | j | k | l | m | n | | | Rx bid={0,1}, decode
+|---+---+---+---+---+---+---+---+---+---+---+---+---+---+---+---+---+---+---+---+---+---+---+---+ << 2
+| | | | | | | a | b | c | d | e | f | g | h | i | j | k | l | m | n | o | p | | | Rx bid={0,1}
+|---+---+---+---+---+---+---+---+---+---+---+---+---+---+---+---+---+---+---+---+---+---+---+---+ << 2
+| | | | | a | b | c | d | e | f | g | h | i | j | k | l | m | n | o | p | q | r | | | Rx bid={0,1}, decode
+|---+---+---+---+---+---+---+---+---+---+---+---+---+---+---+---+---+---+---+---+---+---+---+---+ << 2
+| | | a | b | c | d | e | f | g | h | i | j | k | l | m | n | o | p | q | r | s | t | | | Rx bid={0,1}
+|---+---+---+---+---+---+---+---+---+---+---+---+---+---+---+---+---+---+---+---+---+---+---+---+ << 2
+| a | b | c | d | e | f | g | h | i | j | k | l | m | n | o | p | q | r | s | t | u | v | | | Rx bid={0,1}, decode
+|---+---+---+---+---+---+---+---+---+---+---+---+---+---+---+---+---+---+---+---+---+---+---+---+ << 2
+|
+|<~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~>| frame A
+| |<~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~>| frame B
+@ decoding from here
diff --git a/src/osmo-bts-trx/sched_lchan_tchf.c b/src/osmo-bts-trx/sched_lchan_tchf.c
index 596912b..fa06736 100644
--- a/src/osmo-bts-trx/sched_lchan_tchf.c
+++ b/src/osmo-bts-trx/sched_lchan_tchf.c
@@ -70,7 +70,8 @@
extern const uint8_t sched_tchh_dl_amr_cmi_map[26];
-/*! \brief a single TCH/F burst was received by the PHY, process it */
+/* Process a single Uplink TCH/F burst received by the PHY.
+ * This function is visualized in file 'doc/trx_sched_tch.txt'. */
int rx_tchf_fn(struct l1sched_ts *l1ts, const struct trx_ul_burst_ind *bi)
{
struct l1sched_chan_state *chan_state = &l1ts->chan_state[bi->chan];
diff --git a/src/osmo-bts-trx/sched_lchan_tchh.c b/src/osmo-bts-trx/sched_lchan_tchh.c
index 11d8b33..d7a19a3 100644
--- a/src/osmo-bts-trx/sched_lchan_tchh.c
+++ b/src/osmo-bts-trx/sched_lchan_tchh.c
@@ -127,7 +127,8 @@
[18] = 1, /* TCH/H(1): B2(18 ... 11) */
};
-/*! \brief a single TCH/H burst was received by the PHY, process it */
+/* Process a single Uplink TCH/H burst received by the PHY.
+ * This function is visualized in file 'doc/trx_sched_tch.txt'. */
int rx_tchh_fn(struct l1sched_ts *l1ts, const struct trx_ul_burst_ind *bi)
{
struct l1sched_chan_state *chan_state = &l1ts->chan_state[bi->chan];
--
To view, visit https://gerrit.osmocom.org/c/osmo-bts/+/33076
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings
Gerrit-Project: osmo-bts
Gerrit-Branch: master
Gerrit-Change-Id: I373dbbc3d427858f76d07ff85633e07fe2600770
Gerrit-Change-Number: 33076
Gerrit-PatchSet: 1
Gerrit-Owner: fixeria <vyanitskiy(a)sysmocom.de>
Gerrit-MessageType: newchange