[M] Change in libosmocore[master]: gsm: Improve the TCH/H2.4 coding routines

2023-07-07 Thread tnt
tnt has submitted this change. ( 
https://gerrit.osmocom.org/c/libosmocore/+/33641 )

Change subject: gsm: Improve the TCH/H2.4 coding routines
..

gsm: Improve the TCH/H2.4 coding routines

The spec isn't super clear, but basically the conv coding is done
in two blocks of 72 bits.

The way it's currently implemented isn't "wrong" in the sense it will
produce correct output given no bit errors, but it's better to do the
decoding in two blocks because this then it makes use of the fact we
know the state of the encoder after the 72 bits, which improves the
corrective ability of the code.

Signed-off-by: Sylvain Munaut 
Change-Id: Id2551ffe2a0ebfd0a6df0e1d288a6f0af7e1eda7
---
M src/coding/gsm0503_coding.c
M src/gsm/libosmogsm.map
M tests/conv/conv_gsm0503_test.ok
M utils/conv_codes_gsm.py
4 files changed, 58 insertions(+), 14 deletions(-)

Approvals:
  fixeria: Looks good to me, but someone else must approve
  pespin: Looks good to me, but someone else must approve
  tnt: Looks good to me, approved; Verified




diff --git a/src/coding/gsm0503_coding.c b/src/coding/gsm0503_coding.c
index 286981e..1249ce9 100644
--- a/src/coding/gsm0503_coding.c
+++ b/src/coding/gsm0503_coding.c
@@ -3480,15 +3480,10 @@
 int gsm0503_tch_hr24_encode(ubit_t *bursts, const ubit_t *data)
 {
ubit_t iB[22 * 114], cB[4 * 114];
-   ubit_t conv[2 * 72 + 8];

-   /* 3.7.2 Block code: d1(72) + pad(4) + d2(72) + pad(4) */
-   memset([0], 0, sizeof(conv));
-   memcpy([0], [0], 72);
-   memcpy([76], [72], 72);
-
-   /* 3.7.3 Convolutional encoder: as specified for the TCH/F4.8 in 
subclause 3.4.3 */
-   osmo_conv_encode(_tch_f48, [0], [0]);
+   /* 3.7.{1-3} Block code and Convolutional encoder */
+   osmo_conv_encode(_tch_h24, [ 0], [  0]);
+   osmo_conv_encode(_tch_h24, [72], [228]);

/* 3.7.4 Interleaving: as specified for the TCH/F9.6 in subclause 3.3.4 
*/
memset([0], 0, sizeof(iB));
@@ -3511,8 +3506,8 @@
 int gsm0503_tch_hr24_decode(ubit_t *data, const sbit_t *bursts,
int *n_errors, int *n_bits_total)
 {
+   int n_errors_l[2], n_bits_total_l[2];
sbit_t iB[22 * 114], cB[4 * 114];
-   ubit_t conv[120 + 32];

/* 3.7.5 Mapping on a burst: as specified for TCH/FS in subclause 3.1.4 
*/
for (unsigned int i = 0; i < 22; i++) {
@@ -3523,12 +3518,15 @@
/* 3.7.4 Interleaving: as specified for the TCH/F9.6 in subclause 3.3.4 
*/
gsm0503_tch_f96_deinterleave([0], [0]);

-   /* 3.7.3 Convolutional encoder: as specified for the TCH/F4.8 in 
subclause 3.4.3 */
-   osmo_conv_decode_ber(_tch_f48, [0], [0], n_errors, 
n_bits_total);
+   /* 3.7.{1-3} Block code and Convolutional encoder */
+   osmo_conv_decode_ber(_tch_h24, [  0], [ 0], 
_errors_l[0], _bits_total_l[0]);
+   osmo_conv_decode_ber(_tch_h24, [228], [72], 
_errors_l[1], _bits_total_l[1]);

-   /* 3.7.2 Block code: d1(72) + pad(4) + d2(72) + pad(4) */
-   memcpy([0], [0], 72);
-   memcpy([72], [76], 72);
+   if (n_errors)
+   *n_errors = n_errors_l[0] + n_errors_l[1];
+
+   if (n_bits_total)
+   *n_bits_total = n_bits_total_l[0] + n_bits_total_l[1];

return 2 * 72;
 }
diff --git a/src/gsm/libosmogsm.map b/src/gsm/libosmogsm.map
index 609ee7e..0dfffd3 100644
--- a/src/gsm/libosmogsm.map
+++ b/src/gsm/libosmogsm.map
@@ -123,6 +123,7 @@
 gsm0503_cs3_np;
 gsm0503_tch_fr;
 gsm0503_tch_f24;
+gsm0503_tch_h24;
 gsm0503_tch_f48;
 gsm0503_tch_f96;
 gsm0503_tch_f144;
diff --git a/tests/conv/conv_gsm0503_test.ok b/tests/conv/conv_gsm0503_test.ok
index 6fd3793..dba52bd 100644
--- a/tests/conv/conv_gsm0503_test.ok
+++ b/tests/conv/conv_gsm0503_test.ok
@@ -14,6 +14,14 @@
 [..] Encoding / Decoding cycle : OK
 [..] Encoding / Decoding cycle : OK

+[+] Testing: gsm0503_tch_h24
+[.] Input length  : ret =  72  exp =  72 -> OK
+[.] Output length : ret = 228  exp = 228 -> OK
+[.] Random vector checks:
+[..] Encoding / Decoding cycle : OK
+[..] Encoding / Decoding cycle : OK
+[..] Encoding / Decoding cycle : OK
+
 [+] Testing: gsm0503_tch_f48
 [.] Input length  : ret = 148  exp = 148 -> OK
 [.] Output length : ret = 456  exp = 456 -> OK
diff --git a/utils/conv_codes_gsm.py b/utils/conv_codes_gsm.py
index 62b5a0b..24a8ee2 100644
--- a/utils/conv_codes_gsm.py
+++ b/utils/conv_codes_gsm.py
@@ -66,6 +66,24 @@
]
),

+   # TCH/H2.4 definition
+   ConvolutionalCode(
+   72,
+   [
+   (G1, 1),
+   (G2, 1),
+   (G3, 1),
+   ],
+   name = "tch_h24",
+   description = [
+   "TCH/H2.4 convolutional code:",
+   "72 bits blocks, rate 1/3, k = 5",
+   "G1 = 1 + D + D3 + D4",
+   "G2 = 1 + D2 + D4",
+   "G3 = 1 + D + D2 + 

[M] Change in libosmocore[master]: gsm: Improve the TCH/H2.4 coding routines

2023-07-07 Thread tnt
tnt has posted comments on this change. ( 
https://gerrit.osmocom.org/c/libosmocore/+/33641 )

Change subject: gsm: Improve the TCH/H2.4 coding routines
..


Patch Set 2: Verified+1 Code-Review+2


--
To view, visit https://gerrit.osmocom.org/c/libosmocore/+/33641
To unsubscribe, or for help writing mail filters, visit 
https://gerrit.osmocom.org/settings

Gerrit-Project: libosmocore
Gerrit-Branch: master
Gerrit-Change-Id: Id2551ffe2a0ebfd0a6df0e1d288a6f0af7e1eda7
Gerrit-Change-Number: 33641
Gerrit-PatchSet: 2
Gerrit-Owner: tnt 
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: fixeria 
Gerrit-Reviewer: pespin 
Gerrit-Reviewer: tnt 
Gerrit-Comment-Date: Fri, 07 Jul 2023 18:04:47 +
Gerrit-HasComments: No
Gerrit-Has-Labels: Yes
Gerrit-MessageType: comment


[M] Change in libosmocore[master]: gsm: Improve the TCH/H2.4 coding routines

2023-07-07 Thread tnt
Attention is currently required from: tnt.

tnt has removed a vote from this change. ( 
https://gerrit.osmocom.org/c/libosmocore/+/33641 )


Change subject: gsm: Improve the TCH/H2.4 coding routines
..


Removed Verified-1 by Jenkins Builder (102)
--
To view, visit https://gerrit.osmocom.org/c/libosmocore/+/33641
To unsubscribe, or for help writing mail filters, visit 
https://gerrit.osmocom.org/settings

Gerrit-Project: libosmocore
Gerrit-Branch: master
Gerrit-Change-Id: Id2551ffe2a0ebfd0a6df0e1d288a6f0af7e1eda7
Gerrit-Change-Number: 33641
Gerrit-PatchSet: 2
Gerrit-Owner: tnt 
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: fixeria 
Gerrit-Reviewer: pespin 
Gerrit-Attention: tnt 
Gerrit-MessageType: deleteVote


[M] Change in libosmocore[master]: gsm: Improve the TCH/H2.4 coding routines

2023-07-07 Thread pespin
Attention is currently required from: tnt.

pespin has posted comments on this change. ( 
https://gerrit.osmocom.org/c/libosmocore/+/33641 )

Change subject: gsm: Improve the TCH/H2.4 coding routines
..


Patch Set 2: Code-Review+1


--
To view, visit https://gerrit.osmocom.org/c/libosmocore/+/33641
To unsubscribe, or for help writing mail filters, visit 
https://gerrit.osmocom.org/settings

Gerrit-Project: libosmocore
Gerrit-Branch: master
Gerrit-Change-Id: Id2551ffe2a0ebfd0a6df0e1d288a6f0af7e1eda7
Gerrit-Change-Number: 33641
Gerrit-PatchSet: 2
Gerrit-Owner: tnt 
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: fixeria 
Gerrit-Reviewer: pespin 
Gerrit-Attention: tnt 
Gerrit-Comment-Date: Fri, 07 Jul 2023 17:11:32 +
Gerrit-HasComments: No
Gerrit-Has-Labels: Yes
Gerrit-MessageType: comment


[M] Change in libosmocore[master]: gsm: Improve the TCH/H2.4 coding routines

2023-07-07 Thread fixeria
Attention is currently required from: tnt.

fixeria has posted comments on this change. ( 
https://gerrit.osmocom.org/c/libosmocore/+/33641 )

Change subject: gsm: Improve the TCH/H2.4 coding routines
..


Patch Set 2: Code-Review+1

(2 comments)

Patchset:

PS2:
> FWIW I don't agree with the linter. […]
We can remove the V-1 vote and re-add V+1 manually, if others find it more 
readable. Personally I don't have a strong preference here and for me it's fine 
either way.

This is also an option, which should not trigger the linter:

```
osmo_conv_encode(_tch_h24,  [0],   [0]);
osmo_conv_encode(_tch_h24, [72], [228]);
```


PS2:
Thanks a lot for the patch!



--
To view, visit https://gerrit.osmocom.org/c/libosmocore/+/33641
To unsubscribe, or for help writing mail filters, visit 
https://gerrit.osmocom.org/settings

Gerrit-Project: libosmocore
Gerrit-Branch: master
Gerrit-Change-Id: Id2551ffe2a0ebfd0a6df0e1d288a6f0af7e1eda7
Gerrit-Change-Number: 33641
Gerrit-PatchSet: 2
Gerrit-Owner: tnt 
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: fixeria 
Gerrit-Attention: tnt 
Gerrit-Comment-Date: Fri, 07 Jul 2023 14:44:43 +
Gerrit-HasComments: Yes
Gerrit-Has-Labels: Yes
Comment-In-Reply-To: tnt 
Gerrit-MessageType: comment


[M] Change in libosmocore[master]: gsm: Improve the TCH/H2.4 coding routines

2023-07-07 Thread tnt
tnt has posted comments on this change. ( 
https://gerrit.osmocom.org/c/libosmocore/+/33641 )

Change subject: gsm: Improve the TCH/H2.4 coding routines
..


Patch Set 2:

(1 comment)

Patchset:

PS2:
FWIW I don't agree with the linter. In this specific case keeping the brackets 
aligned between blocks makes the code more readable IMHO.



--
To view, visit https://gerrit.osmocom.org/c/libosmocore/+/33641
To unsubscribe, or for help writing mail filters, visit 
https://gerrit.osmocom.org/settings

Gerrit-Project: libosmocore
Gerrit-Branch: master
Gerrit-Change-Id: Id2551ffe2a0ebfd0a6df0e1d288a6f0af7e1eda7
Gerrit-Change-Number: 33641
Gerrit-PatchSet: 2
Gerrit-Owner: tnt 
Gerrit-Reviewer: Jenkins Builder
Gerrit-Comment-Date: Fri, 07 Jul 2023 14:12:20 +
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Gerrit-MessageType: comment


[M] Change in libosmocore[master]: gsm: Improve the TCH/H2.4 coding routines

2023-07-07 Thread Jenkins Builder
Attention is currently required from: tnt.

Jenkins Builder has posted comments on this change. ( 
https://gerrit.osmocom.org/c/libosmocore/+/33641 )

Change subject: gsm: Improve the TCH/H2.4 coding routines
..


Patch Set 2:

(2 comments)

File src/coding/gsm0503_coding.c:

Robot Comment from checkpatch (run ID jenkins-gerrit-lint-9162):
https://gerrit.osmocom.org/c/libosmocore/+/33641/comment/5ad2a3ea_4727be8c
PS2, Line 3485: osmo_conv_encode(_tch_h24, [ 0], [  0]);
space prohibited after that open square bracket '['


Robot Comment from checkpatch (run ID jenkins-gerrit-lint-9162):
https://gerrit.osmocom.org/c/libosmocore/+/33641/comment/d4ef67f0_93d2af2f
PS2, Line 3522: osmo_conv_decode_ber(_tch_h24, [  0], [ 
0], _errors_l[0], _bits_total_l[0]);
space prohibited after that open square bracket '['



--
To view, visit https://gerrit.osmocom.org/c/libosmocore/+/33641
To unsubscribe, or for help writing mail filters, visit 
https://gerrit.osmocom.org/settings

Gerrit-Project: libosmocore
Gerrit-Branch: master
Gerrit-Change-Id: Id2551ffe2a0ebfd0a6df0e1d288a6f0af7e1eda7
Gerrit-Change-Number: 33641
Gerrit-PatchSet: 2
Gerrit-Owner: tnt 
Gerrit-Reviewer: Jenkins Builder
Gerrit-Attention: tnt 
Gerrit-Comment-Date: Fri, 07 Jul 2023 14:06:50 +
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Gerrit-MessageType: comment


[M] Change in libosmocore[master]: gsm: Improve the TCH/H2.4 coding routines

2023-07-07 Thread tnt
Attention is currently required from: tnt.

Hello Jenkins Builder,

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

https://gerrit.osmocom.org/c/libosmocore/+/33641

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

Change subject: gsm: Improve the TCH/H2.4 coding routines
..

gsm: Improve the TCH/H2.4 coding routines

The spec isn't super clear, but basically the conv coding is done
in two blocks of 72 bits.

The way it's currently implemented isn't "wrong" in the sense it will
produce correct output given no bit errors, but it's better to do the
decoding in two blocks because this then it makes use of the fact we
know the state of the encoder after the 72 bits, which improves the
corrective ability of the code.

Signed-off-by: Sylvain Munaut 
Change-Id: Id2551ffe2a0ebfd0a6df0e1d288a6f0af7e1eda7
---
M src/coding/gsm0503_coding.c
M src/gsm/libosmogsm.map
M tests/conv/conv_gsm0503_test.ok
M utils/conv_codes_gsm.py
4 files changed, 58 insertions(+), 14 deletions(-)


  git pull ssh://gerrit.osmocom.org:29418/libosmocore refs/changes/41/33641/2
--
To view, visit https://gerrit.osmocom.org/c/libosmocore/+/33641
To unsubscribe, or for help writing mail filters, visit 
https://gerrit.osmocom.org/settings

Gerrit-Project: libosmocore
Gerrit-Branch: master
Gerrit-Change-Id: Id2551ffe2a0ebfd0a6df0e1d288a6f0af7e1eda7
Gerrit-Change-Number: 33641
Gerrit-PatchSet: 2
Gerrit-Owner: tnt 
Gerrit-Reviewer: Jenkins Builder
Gerrit-Attention: tnt 
Gerrit-MessageType: newpatchset


[M] Change in libosmocore[master]: gsm: Improve the TCH/H2.4 coding routines

2023-07-07 Thread Jenkins Builder
Jenkins Builder has posted comments on this change. ( 
https://gerrit.osmocom.org/c/libosmocore/+/33641 )

Change subject: gsm: Improve the TCH/H2.4 coding routines
..


Patch Set 1:

(2 comments)

File src/coding/gsm0503_coding.c:

Robot Comment from checkpatch (run ID jenkins-gerrit-lint-9161):
https://gerrit.osmocom.org/c/libosmocore/+/33641/comment/8f085bb2_f41320e6
PS1, Line 3485: osmo_conv_encode(_tch_h24, [ 0], [  0]);
space prohibited after that open square bracket '['


Robot Comment from checkpatch (run ID jenkins-gerrit-lint-9161):
https://gerrit.osmocom.org/c/libosmocore/+/33641/comment/01498b54_d084eea6
PS1, Line 3522: osmo_conv_decode_ber(_tch_h24, [  0], [ 
0], _errors_l[0], _bits_total_l[0]);
space prohibited after that open square bracket '['



--
To view, visit https://gerrit.osmocom.org/c/libosmocore/+/33641
To unsubscribe, or for help writing mail filters, visit 
https://gerrit.osmocom.org/settings

Gerrit-Project: libosmocore
Gerrit-Branch: master
Gerrit-Change-Id: Id2551ffe2a0ebfd0a6df0e1d288a6f0af7e1eda7
Gerrit-Change-Number: 33641
Gerrit-PatchSet: 1
Gerrit-Owner: tnt 
Gerrit-CC: Jenkins Builder
Gerrit-Comment-Date: Fri, 07 Jul 2023 13:58:51 +
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Gerrit-MessageType: comment


[M] Change in libosmocore[master]: gsm: Improve the TCH/H2.4 coding routines

2023-07-07 Thread tnt
tnt has uploaded this change for review. ( 
https://gerrit.osmocom.org/c/libosmocore/+/33641 )


Change subject: gsm: Improve the TCH/H2.4 coding routines
..

gsm: Improve the TCH/H2.4 coding routines

The spec isn't super clear, but basically the conv coding is done
in two blocks of 72 bits.

The way it's currently implemented isn't "wrong" in the sense it will
produce correct output given no bit errors, but it's better to do the
decoding in two blocks because this then it makes use of the fact we
know the state of the encoder after the 72 bits, which improves the
corrective ability of the code.

Signed-off-by: Sylvain Munaut 
Change-Id: Id2551ffe2a0ebfd0a6df0e1d288a6f0af7e1eda7
---
M src/coding/gsm0503_coding.c
M src/gsm/libosmogsm.map
M utils/conv_codes_gsm.py
3 files changed, 50 insertions(+), 14 deletions(-)



  git pull ssh://gerrit.osmocom.org:29418/libosmocore refs/changes/41/33641/1

diff --git a/src/coding/gsm0503_coding.c b/src/coding/gsm0503_coding.c
index 286981e..1249ce9 100644
--- a/src/coding/gsm0503_coding.c
+++ b/src/coding/gsm0503_coding.c
@@ -3480,15 +3480,10 @@
 int gsm0503_tch_hr24_encode(ubit_t *bursts, const ubit_t *data)
 {
ubit_t iB[22 * 114], cB[4 * 114];
-   ubit_t conv[2 * 72 + 8];

-   /* 3.7.2 Block code: d1(72) + pad(4) + d2(72) + pad(4) */
-   memset([0], 0, sizeof(conv));
-   memcpy([0], [0], 72);
-   memcpy([76], [72], 72);
-
-   /* 3.7.3 Convolutional encoder: as specified for the TCH/F4.8 in 
subclause 3.4.3 */
-   osmo_conv_encode(_tch_f48, [0], [0]);
+   /* 3.7.{1-3} Block code and Convolutional encoder */
+   osmo_conv_encode(_tch_h24, [ 0], [  0]);
+   osmo_conv_encode(_tch_h24, [72], [228]);

/* 3.7.4 Interleaving: as specified for the TCH/F9.6 in subclause 3.3.4 
*/
memset([0], 0, sizeof(iB));
@@ -3511,8 +3506,8 @@
 int gsm0503_tch_hr24_decode(ubit_t *data, const sbit_t *bursts,
int *n_errors, int *n_bits_total)
 {
+   int n_errors_l[2], n_bits_total_l[2];
sbit_t iB[22 * 114], cB[4 * 114];
-   ubit_t conv[120 + 32];

/* 3.7.5 Mapping on a burst: as specified for TCH/FS in subclause 3.1.4 
*/
for (unsigned int i = 0; i < 22; i++) {
@@ -3523,12 +3518,15 @@
/* 3.7.4 Interleaving: as specified for the TCH/F9.6 in subclause 3.3.4 
*/
gsm0503_tch_f96_deinterleave([0], [0]);

-   /* 3.7.3 Convolutional encoder: as specified for the TCH/F4.8 in 
subclause 3.4.3 */
-   osmo_conv_decode_ber(_tch_f48, [0], [0], n_errors, 
n_bits_total);
+   /* 3.7.{1-3} Block code and Convolutional encoder */
+   osmo_conv_decode_ber(_tch_h24, [  0], [ 0], 
_errors_l[0], _bits_total_l[0]);
+   osmo_conv_decode_ber(_tch_h24, [228], [72], 
_errors_l[1], _bits_total_l[1]);

-   /* 3.7.2 Block code: d1(72) + pad(4) + d2(72) + pad(4) */
-   memcpy([0], [0], 72);
-   memcpy([72], [76], 72);
+   if (n_errors)
+   *n_errors = n_errors_l[0] + n_errors_l[1];
+
+   if (n_bits_total)
+   *n_bits_total = n_bits_total_l[0] + n_bits_total_l[1];

return 2 * 72;
 }
diff --git a/src/gsm/libosmogsm.map b/src/gsm/libosmogsm.map
index 609ee7e..0dfffd3 100644
--- a/src/gsm/libosmogsm.map
+++ b/src/gsm/libosmogsm.map
@@ -123,6 +123,7 @@
 gsm0503_cs3_np;
 gsm0503_tch_fr;
 gsm0503_tch_f24;
+gsm0503_tch_h24;
 gsm0503_tch_f48;
 gsm0503_tch_f96;
 gsm0503_tch_f144;
diff --git a/utils/conv_codes_gsm.py b/utils/conv_codes_gsm.py
index 62b5a0b..24a8ee2 100644
--- a/utils/conv_codes_gsm.py
+++ b/utils/conv_codes_gsm.py
@@ -66,6 +66,24 @@
]
),

+   # TCH/H2.4 definition
+   ConvolutionalCode(
+   72,
+   [
+   (G1, 1),
+   (G2, 1),
+   (G3, 1),
+   ],
+   name = "tch_h24",
+   description = [
+   "TCH/H2.4 convolutional code:",
+   "72 bits blocks, rate 1/3, k = 5",
+   "G1 = 1 + D + D3 + D4",
+   "G2 = 1 + D2 + D4",
+   "G3 = 1 + D + D2 + D3 + D4",
+   ]
+   ),
+
# TCH/F4.8 definition
ConvolutionalCode(
148,

--
To view, visit https://gerrit.osmocom.org/c/libosmocore/+/33641
To unsubscribe, or for help writing mail filters, visit 
https://gerrit.osmocom.org/settings

Gerrit-Project: libosmocore
Gerrit-Branch: master
Gerrit-Change-Id: Id2551ffe2a0ebfd0a6df0e1d288a6f0af7e1eda7
Gerrit-Change-Number: 33641
Gerrit-PatchSet: 1
Gerrit-Owner: tnt 
Gerrit-MessageType: newchange