pespin has submitted this change. ( https://gerrit.osmocom.org/c/osmo-bts/+/29274 ) Change subject: Clarify RTP AMR header offset in TCH enc/dec ...................................................................... Clarify RTP AMR header offset in TCH enc/dec This helps understand the origin/purpose of those 2 bytes and what's contained in them. Related: SYS#5987 Change-Id: I607fdf6d627242e010fba35be1b9b0ffde820d08 --- M src/osmo-bts-trx/sched_lchan_tchf.c M src/osmo-bts-trx/sched_lchan_tchh.c 2 files changed, 18 insertions(+), 12 deletions(-) Approvals: Jenkins Builder: Verified osmith: Looks good to me, approved pespin: Looks good to me, but someone else must approve diff --git a/src/osmo-bts-trx/sched_lchan_tchf.c b/src/osmo-bts-trx/sched_lchan_tchf.c index 65f397d..bf6703a 100644 --- a/src/osmo-bts-trx/sched_lchan_tchf.c +++ b/src/osmo-bts-trx/sched_lchan_tchf.c @@ -35,6 +35,8 @@ #include <osmocom/coding/gsm0503_coding.h> #include <osmocom/coding/gsm0503_amr_dtx.h> +#include <osmocom/netif/amr.h> + #include <osmo-bts/bts.h> #include <osmo-bts/l1sap.h> #include <osmo-bts/logging.h> @@ -155,11 +157,13 @@ if (chan_state->amr_last_dtx == AFS_ONSET) lchan_set_marker(false, lchan); - /* we store tch_data + 2 header bytes, the amr variable set to - * 2 will allow us to skip the first 2 bytes in case we did - * receive an FACCH frame instead of a voice frame (we do not - * know this before we actually decode the frame) */ - amr = 2; + /* Store AMR payload in tch-data with an offset of 2 bytes, so + * that we can easily prepend/fill the RTP AMR header (struct + * amr_hdr) with osmo_amr_rtp_enc() later on. The amr variable + * is used far below to account for the decoded offset in case + * we receive an FACCH frame instead of a voice frame (we + * do not know this before we actually decode the frame) */ + amr = sizeof(struct amr_hdr); rc = gsm0503_tch_afs_decode_dtx(tch_data + amr, *bursts_p, amr_is_cmr, chan_state->codec, chan_state->codecs, &chan_state->ul_ft, &chan_state->ul_cmr, &n_errors, &n_bits_total, &chan_state->amr_last_dtx); @@ -302,7 +306,7 @@ "not sending BFI\n", rc); return -EINVAL; } - memset(tch_data + 2, 0, rc - 2); + memset(tch_data + sizeof(struct amr_hdr), 0, rc - sizeof(struct amr_hdr)); break; default: LOGL1SB(DL1P, LOGL_ERROR, l1ts, bi, @@ -509,8 +513,8 @@ /* the first FN 4,13,21 defines that CMI is included in frame, * the first FN 0,8,17 defines that CMR is included in frame. */ - gsm0503_tch_afs_encode(*bursts_p, msg->l2h + 2, - msgb_l2len(msg) - 2, !dl_amr_fn_is_cmi(br->fn), + gsm0503_tch_afs_encode(*bursts_p, msg->l2h + sizeof(struct amr_hdr), + msgb_l2len(msg) - sizeof(struct amr_hdr), !dl_amr_fn_is_cmi(br->fn), chan_state->codec, chan_state->codecs, chan_state->dl_ft, chan_state->dl_cmr); diff --git a/src/osmo-bts-trx/sched_lchan_tchh.c b/src/osmo-bts-trx/sched_lchan_tchh.c index a5f6747..17073e0 100644 --- a/src/osmo-bts-trx/sched_lchan_tchh.c +++ b/src/osmo-bts-trx/sched_lchan_tchh.c @@ -35,6 +35,8 @@ #include <osmocom/coding/gsm0503_coding.h> #include <osmocom/coding/gsm0503_amr_dtx.h> +#include <osmocom/netif/amr.h> + #include <osmo-bts/bts.h> #include <osmo-bts/l1sap.h> #include <osmo-bts/logging.h> @@ -196,7 +198,7 @@ fn_is_cmi = sched_tchh_ul_amr_cmi_map[bi->fn % 26]; /* See comment in function rx_tchf_fn() */ - amr = 2; + amr = sizeof(struct amr_hdr); rc = gsm0503_tch_ahs_decode_dtx(tch_data + amr, *bursts_p, !sched_tchh_ul_facch_map[bi->fn % 26], !fn_is_cmi, chan_state->codec, @@ -336,7 +338,7 @@ "not sending BFI\n", rc); return -EINVAL; } - memset(tch_data + 2, 0, rc - 2); + memset(tch_data + sizeof(struct amr_hdr), 0, rc - sizeof(struct amr_hdr)); break; default: LOGL1SB(DL1P, LOGL_ERROR, l1ts, bi, @@ -436,8 +438,8 @@ /* the first FN 4,13,21 or 5,14,22 defines that CMI is included * in frame, the first FN 0,8,17 or 1,9,18 defines that CMR is * included in frame. */ - gsm0503_tch_ahs_encode(*bursts_p, msg->l2h + 2, - msgb_l2len(msg) - 2, !dl_amr_fn_is_cmi(br->fn), + gsm0503_tch_ahs_encode(*bursts_p, msg->l2h + sizeof(struct amr_hdr), + msgb_l2len(msg) - sizeof(struct amr_hdr), !dl_amr_fn_is_cmi(br->fn), chan_state->codec, chan_state->codecs, chan_state->dl_ft, chan_state->dl_cmr); -- To view, visit https://gerrit.osmocom.org/c/osmo-bts/+/29274 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-bts Gerrit-Branch: master Gerrit-Change-Id: I607fdf6d627242e010fba35be1b9b0ffde820d08 Gerrit-Change-Number: 29274 Gerrit-PatchSet: 4 Gerrit-Owner: pespin &lt;pespin(a)sysmocom.de&gt; Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: fixeria &lt;vyanitskiy(a)sysmocom.de&gt; Gerrit-Reviewer: laforge &lt;laforge(a)osmocom.org&gt; Gerrit-Reviewer: neels &lt;nhofmeyr(a)sysmocom.de&gt; Gerrit-Reviewer: osmith &lt;osmith(a)sysmocom.de&gt; Gerrit-Reviewer: pespin &lt;pespin(a)sysmocom.de&gt; Gerrit-MessageType: merged