laforge submitted this change.

View Change


Approvals: fixeria: Looks good to me, but someone else must approve pespin: Looks good to me, but someone else must approve Jenkins Builder: Verified laforge: Looks good to me, approved; Verified
bts-{sysmo,oc2g,lc15}: Fix RTP of AMR SID_FIRST_P1

When we receive a SID_FIRST_P1 frame from the PHY (during AMR/HR DTXu),
we must generate a SID frame on the RTP side.

The existing code
* ignored that the Amr_SidFirstP1 PHYIF message actually contains the
RTP payload
* manually generated the same content using osmo_amr_rtp_enc()
* forgot to prefix that with the AMR CMR+TOC
* in the end, sent a completely broken (too short) AMR SID frame over RTP

Let's fix this by simply using the 7-byte RTP payload with CMR+TOC that
the PHY is providing to us.

Change-Id: I90479efcc002d497648a71e73847af54e6208358
Related: OS#5944
---
M src/osmo-bts-lc15/tch.c
M src/osmo-bts-oc2g/tch.c
M src/osmo-bts-sysmo/tch.c
3 files changed, 29 insertions(+), 24 deletions(-)

diff --git a/src/osmo-bts-lc15/tch.c b/src/osmo-bts-lc15/tch.c
index bd84bee..f9a4e21 100644
--- a/src/osmo-bts-lc15/tch.c
+++ b/src/osmo-bts-lc15/tch.c
@@ -360,7 +360,7 @@
{
GsmL1_Prim_t *l1p = msgb_l1prim(l1p_msg);
GsmL1_PhDataInd_t *data_ind = &l1p->u.phDataInd;
- uint8_t *payload, payload_type, payload_len, sid_first[9] = { 0 };
+ uint8_t *payload, payload_type, payload_len;
struct msgb *rmsg = NULL;
struct gsm_lchan *lchan = &trx->ts[L1SAP_CHAN2TS(chan_nr)].lchan[l1sap_chan2ss(chan_nr)];

@@ -450,14 +450,8 @@
rmsg = l1_to_rtppayload_efr(payload, payload_len, lchan);
break;
case GsmL1_TchPlType_Amr:
- rmsg = l1_to_rtppayload_amr(payload, payload_len, lchan);
- break;
case GsmL1_TchPlType_Amr_SidFirstP1:
- memcpy(sid_first, payload, payload_len);
- int len = osmo_amr_rtp_enc(sid_first, 0, AMR_SID, AMR_GOOD);
- if (len < 0)
- return 0;
- rmsg = l1_to_rtppayload_amr(sid_first, len, lchan);
+ rmsg = l1_to_rtppayload_amr(payload, payload_len, lchan);
break;
}

diff --git a/src/osmo-bts-oc2g/tch.c b/src/osmo-bts-oc2g/tch.c
index d469740..bd97959 100644
--- a/src/osmo-bts-oc2g/tch.c
+++ b/src/osmo-bts-oc2g/tch.c
@@ -360,7 +360,7 @@
{
GsmL1_Prim_t *l1p = msgb_l1prim(l1p_msg);
GsmL1_PhDataInd_t *data_ind = &l1p->u.phDataInd;
- uint8_t *payload, payload_type, payload_len, sid_first[9] = { 0 };
+ uint8_t *payload, payload_type, payload_len;
struct msgb *rmsg = NULL;
struct gsm_lchan *lchan = &trx->ts[L1SAP_CHAN2TS(chan_nr)].lchan[l1sap_chan2ss(chan_nr)];

@@ -460,14 +460,8 @@
rmsg = l1_to_rtppayload_efr(payload, payload_len, lchan);
break;
case GsmL1_TchPlType_Amr:
- rmsg = l1_to_rtppayload_amr(payload, payload_len, lchan);
- break;
case GsmL1_TchPlType_Amr_SidFirstP1:
- memcpy(sid_first, payload, payload_len);
- int len = osmo_amr_rtp_enc(sid_first, 0, AMR_SID, AMR_GOOD);
- if (len < 0)
- return 0;
- rmsg = l1_to_rtppayload_amr(sid_first, len, lchan);
+ rmsg = l1_to_rtppayload_amr(payload, payload_len, lchan);
break;
}

diff --git a/src/osmo-bts-sysmo/tch.c b/src/osmo-bts-sysmo/tch.c
index b308a5f..e901d47 100644
--- a/src/osmo-bts-sysmo/tch.c
+++ b/src/osmo-bts-sysmo/tch.c
@@ -505,7 +505,7 @@
{
GsmL1_Prim_t *l1p = msgb_l1prim(l1p_msg);
GsmL1_PhDataInd_t *data_ind = &l1p->u.phDataInd;
- uint8_t *payload, payload_type, payload_len, sid_first[9] = { 0 };
+ uint8_t *payload, payload_type, payload_len;
struct msgb *rmsg = NULL;
struct gsm_lchan *lchan = &trx->ts[L1SAP_CHAN2TS(chan_nr)].lchan[l1sap_chan2ss(chan_nr)];

@@ -602,14 +602,8 @@
break;
#endif
case GsmL1_TchPlType_Amr:
- rmsg = l1_to_rtppayload_amr(payload, payload_len, lchan);
- break;
case GsmL1_TchPlType_Amr_SidFirstP1:
- memcpy(sid_first, payload, payload_len);
- int len = osmo_amr_rtp_enc(sid_first, 0, AMR_SID, AMR_GOOD);
- if (len < 0)
- return 0;
- rmsg = l1_to_rtppayload_amr(sid_first, len, lchan);
+ rmsg = l1_to_rtppayload_amr(payload, payload_len, lchan);
break;
/* FIXME: what about GsmL1_TchPlType_Amr_SidBad? not well documented. */
}

To view, visit change 32088. To unsubscribe, or for help writing mail filters, visit settings.

Gerrit-Project: osmo-bts
Gerrit-Branch: master
Gerrit-Change-Id: I90479efcc002d497648a71e73847af54e6208358
Gerrit-Change-Number: 32088
Gerrit-PatchSet: 4
Gerrit-Owner: laforge <laforge@osmocom.org>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: fixeria <vyanitskiy@sysmocom.de>
Gerrit-Reviewer: laforge <laforge@osmocom.org>
Gerrit-Reviewer: pespin <pespin@sysmocom.de>
Gerrit-MessageType: merged