fixeria has submitted this change. (
https://gerrit.osmocom.org/c/osmocom-bb/+/33706 )
Change subject: trxcon/l1sched: fix handling of UL FACCH on TCH/A[FH]S
......................................................................
trxcon/l1sched: fix handling of UL FACCH on TCH/A[FH]S
In ad8f7794 I changed both tx_tch[fh]_fn() to use a switch statement
and introduced a regression by removing special treatment of FACCH:
@@ -238,10 +237,16 @@ int tx_tchf_fn(struct l1sched_lchan_state *lchan,
- if (msgb_l2len(lchan->prim) == GSM_MACBLOCK_LEN) {
- /* Encode payload */
- rc = gsm0503_tch_fr_encode(buffer, msgb_l2(lchan->prim),
GSM_MACBLOCK_LEN, 1);
- } else if (lchan->tch_mode == GSM48_CMODE_SPEECH_AMR) {
@@ -459,10 +458,15 @@ int tx_tchh_fn(struct l1sched_lchan_state *lchan,
- if (msgb_l2len(lchan->prim) == GSM_MACBLOCK_LEN) {
- rc = gsm0503_tch_hr_encode(buffer, msgb_l2(lchan->prim),
GSM_MACBLOCK_LEN);
- lchan->ul_facch_blocks = 6;
- } else if (lchan->tch_mode == GSM48_CMODE_SPEECH_AMR) {
Now if the channel mode is GSM48_CMODE_SPEECH_AMR, UL FACCH/[FH] frames
will be fed to osmo_amr_rtp_dec(), which is definitely wrong. Fix this
by doing all AMR specific checks in a separate function, which is
called only for speech frames.
Change-Id: Ie217bbb389b5abb95d241781ffe3f5c7b1c188c0
Fixes: ad8f7794 "trxcon/l1sched: remove redundant TCH/[FH] prim length checks"
Related: OS#4396
---
M src/host/trxcon/include/osmocom/bb/l1sched/l1sched.h
M src/host/trxcon/src/sched_lchan_common.c
M src/host/trxcon/src/sched_lchan_tchf.c
M src/host/trxcon/src/sched_lchan_tchh.c
4 files changed, 98 insertions(+), 90 deletions(-)
Approvals:
Jenkins Builder: Verified
laforge: Looks good to me, but someone else must approve
pespin: Looks good to me, but someone else must approve
fixeria: Looks good to me, approved
diff --git a/src/host/trxcon/include/osmocom/bb/l1sched/l1sched.h
b/src/host/trxcon/include/osmocom/bb/l1sched/l1sched.h
index d3b079c..715b9a0 100644
--- a/src/host/trxcon/include/osmocom/bb/l1sched/l1sched.h
+++ b/src/host/trxcon/include/osmocom/bb/l1sched/l1sched.h
@@ -392,6 +392,7 @@
const char *l1sched_burst_mask2str(const uint8_t *mask, int bits);
size_t l1sched_bad_frame_ind(uint8_t *l2, struct l1sched_lchan_state *lchan);
+bool l1sched_lchan_amr_prim_is_valid(struct l1sched_lchan_state *lchan, bool
is_cmr);
/* Interleaved TCH/H block TDMA frame mapping */
bool l1sched_tchh_block_map_fn(enum l1sched_lchan_type chan,
diff --git a/src/host/trxcon/src/sched_lchan_common.c
b/src/host/trxcon/src/sched_lchan_common.c
index cddc9e5..26d8f22 100644
--- a/src/host/trxcon/src/sched_lchan_common.c
+++ b/src/host/trxcon/src/sched_lchan_common.c
@@ -141,3 +141,48 @@
return 0;
}
}
+
+bool l1sched_lchan_amr_prim_is_valid(struct l1sched_lchan_state *lchan, bool
is_cmr)
+{
+ enum osmo_amr_type ft_codec;
+ enum osmo_amr_quality bfi;
+ uint8_t cmr_codec;
+ int ft, cmr, len;
+ int8_t sti, cmi;
+
+ len = osmo_amr_rtp_dec(msgb_l2(lchan->prim), msgb_l2len(lchan->prim),
+ &cmr_codec, &cmi, &ft_codec, &bfi, &sti);
+ if (len < 0) {
+ LOGP_LCHAND(lchan, LOGL_ERROR, "Cannot send invalid AMR payload
(%u): %s\n",
+ msgb_l2len(lchan->prim),
msgb_hexdump_l2(lchan->prim));
+ return false;
+ }
+ ft = -1;
+ cmr = -1;
+ for (unsigned int i = 0; i < lchan->amr.codecs; i++) {
+ if (lchan->amr.codec[i] == ft_codec)
+ ft = i;
+ if (lchan->amr.codec[i] == cmr_codec)
+ cmr = i;
+ }
+ if (ft < 0) {
+ LOGP_LCHAND(lchan, LOGL_ERROR,
+ "Codec (FT = %d) of RTP frame not in list\n",
ft_codec);
+ return false;
+ }
+ if (is_cmr && lchan->amr.ul_ft != ft) {
+ LOGP_LCHAND(lchan, LOGL_ERROR,
+ "Codec (FT = %d) of RTP cannot be changed now, but
in next frame\n",
+ ft_codec);
+ return false;
+ }
+ lchan->amr.ul_ft = ft;
+ if (cmr < 0) {
+ LOGP_LCHAND(lchan, LOGL_ERROR,
+ "Codec (CMR = %d) of RTP frame not in list\n",
cmr_codec);
+ } else {
+ lchan->amr.ul_cmr = cmr;
+ }
+
+ return true;
+}
diff --git a/src/host/trxcon/src/sched_lchan_tchf.c
b/src/host/trxcon/src/sched_lchan_tchf.c
index a4aa80c..9e8256f 100644
--- a/src/host/trxcon/src/sched_lchan_tchf.c
+++ b/src/host/trxcon/src/sched_lchan_tchf.c
@@ -244,54 +244,20 @@
break;
case GSM48_CMODE_SPEECH_AMR:
{
- int len;
- uint8_t cmr_codec;
- int ft, cmr, i;
- enum osmo_amr_type ft_codec;
- enum osmo_amr_quality bfi;
- int8_t sti, cmi;
- bool amr_fn_is_cmr;
- /* the first FN 0,8,17 defines that CMI is included in frame,
- * the first FN 4,13,21 defines that CMR is included in frame.
- */
- amr_fn_is_cmr = !sched_tchf_ul_amr_cmi_map[br->fn % 26];
+ bool amr_fn_is_cmr = !sched_tchf_ul_amr_cmi_map[br->fn % 26];
+ const uint8_t *data = msgb_l2(lchan->prim);
+ size_t data_len = msgb_l2len(lchan->prim);
- len = osmo_amr_rtp_dec(msgb_l2(lchan->prim),
msgb_l2len(lchan->prim),
- &cmr_codec, &cmi, &ft_codec, &bfi, &sti);
- if (len < 0) {
- LOGP_LCHAND(lchan, LOGL_ERROR, "Cannot send invalid AMR
payload (%u): %s\n",
- msgb_l2len(lchan->prim),
msgb_hexdump_l2(lchan->prim));
- goto free_bad_msg;
+ if (data_len != GSM_MACBLOCK_LEN) { /* TCH/AFS: speech */
+ if (!l1sched_lchan_amr_prim_is_valid(lchan,
amr_fn_is_cmr))
+ goto free_bad_msg;
+ /* pull the AMR header - sizeof(struct amr_hdr) */
+ data_len -= 2;
+ data += 2;
}
- ft = -1;
- cmr = -1;
- for (i = 0; i < lchan->amr.codecs; i++) {
- if (lchan->amr.codec[i] == ft_codec)
- ft = i;
- if (lchan->amr.codec[i] == cmr_codec)
- cmr = i;
- }
- if (ft < 0) {
- LOGP_LCHAND(lchan, LOGL_ERROR,
- "Codec (FT = %d) of RTP frame not in
list\n", ft_codec);
- goto free_bad_msg;
- }
- if (amr_fn_is_cmr && lchan->amr.ul_ft != ft) {
- LOGP_LCHAND(lchan, LOGL_ERROR,
- "Codec (FT = %d) of RTP cannot be changed
now, but in next frame\n",
- ft_codec);
- goto free_bad_msg;
- }
- lchan->amr.ul_ft = ft;
- if (cmr < 0) {
- LOGP_LCHAND(lchan, LOGL_ERROR,
- "Codec (CMR = %d) of RTP frame not in
list\n", cmr_codec);
- } else {
- lchan->amr.ul_cmr = cmr;
- }
+
rc = gsm0503_tch_afs_encode(bursts_p,
- msgb_l2(lchan->prim) + 2,
- msgb_l2len(lchan->prim) - 2,
+ data, data_len,
amr_fn_is_cmr,
lchan->amr.codec,
lchan->amr.codecs,
diff --git a/src/host/trxcon/src/sched_lchan_tchh.c
b/src/host/trxcon/src/sched_lchan_tchh.c
index 30d623b..6973f91 100644
--- a/src/host/trxcon/src/sched_lchan_tchh.c
+++ b/src/host/trxcon/src/sched_lchan_tchh.c
@@ -457,54 +457,20 @@
break;
case GSM48_CMODE_SPEECH_AMR:
{
- int len;
- uint8_t cmr_codec;
- int ft, cmr, i;
- enum osmo_amr_type ft_codec;
- enum osmo_amr_quality bfi;
- int8_t sti, cmi;
- bool amr_fn_is_cmr;
- /* the first FN 0,8,17 defines that CMI is included in frame,
- * the first FN 4,13,21 defines that CMR is included in frame.
- */
- amr_fn_is_cmr = !sched_tchh_ul_amr_cmi_map[br->fn % 26];
+ bool amr_fn_is_cmr = !sched_tchh_ul_amr_cmi_map[br->fn % 26];
+ const uint8_t *data = msgb_l2(lchan->prim);
+ size_t data_len = msgb_l2len(lchan->prim);
- len = osmo_amr_rtp_dec(msgb_l2(lchan->prim),
msgb_l2len(lchan->prim),
- &cmr_codec, &cmi, &ft_codec, &bfi, &sti);
- if (len < 0) {
- LOGP_LCHAND(lchan, LOGL_ERROR, "Cannot send invalid AMR
payload (%u): %s\n",
- msgb_l2len(lchan->prim),
msgb_hexdump_l2(lchan->prim));
- goto free_bad_msg;
+ if (data_len != GSM_MACBLOCK_LEN) { /* TCH/AHS: speech */
+ if (!l1sched_lchan_amr_prim_is_valid(lchan,
amr_fn_is_cmr))
+ goto free_bad_msg;
+ /* pull the AMR header - sizeof(struct amr_hdr) */
+ data_len -= 2;
+ data += 2;
}
- ft = -1;
- cmr = -1;
- for (i = 0; i < lchan->amr.codecs; i++) {
- if (lchan->amr.codec[i] == ft_codec)
- ft = i;
- if (lchan->amr.codec[i] == cmr_codec)
- cmr = i;
- }
- if (ft < 0) {
- LOGP_LCHAND(lchan, LOGL_ERROR,
- "Codec (FT = %d) of RTP frame not in
list\n", ft_codec);
- goto free_bad_msg;
- }
- if (amr_fn_is_cmr && lchan->amr.ul_ft != ft) {
- LOGP_LCHAND(lchan, LOGL_ERROR,
- "Codec (FT = %d) of RTP cannot be changed
now, but in next frame\n",
- ft_codec);
- goto free_bad_msg;
- }
- lchan->amr.ul_ft = ft;
- if (cmr < 0) {
- LOGP_LCHAND(lchan, LOGL_ERROR,
- "Codec (CMR = %d) of RTP frame not in
list\n", cmr_codec);
- } else {
- lchan->amr.ul_cmr = cmr;
- }
+
rc = gsm0503_tch_ahs_encode(bursts_p,
- msgb_l2(lchan->prim) + 2,
- msgb_l2len(lchan->prim) - 2,
+ data, data_len,
amr_fn_is_cmr,
lchan->amr.codec,
lchan->amr.codecs,
--
To view, visit https://gerrit.osmocom.org/c/osmocom-bb/+/33706
To unsubscribe, or for help writing mail filters, visit
https://gerrit.osmocom.org/settings
Gerrit-Project: osmocom-bb
Gerrit-Branch: master
Gerrit-Change-Id: Ie217bbb389b5abb95d241781ffe3f5c7b1c188c0
Gerrit-Change-Number: 33706
Gerrit-PatchSet: 1
Gerrit-Owner: fixeria <vyanits...@sysmocom.de>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: fixeria <vyanits...@sysmocom.de>
Gerrit-Reviewer: laforge <lafo...@osmocom.org>
Gerrit-Reviewer: pespin <pes...@sysmocom.de>
Gerrit-MessageType: merged
