Attention is currently required from: pespin, dexter. fixeria has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-bts/+/32630 )
Change subject: l1sap: Accept RFC5993 and TS 101.318 HR GSM payload ...................................................................... Patch Set 4: Code-Review-1 (5 comments) Patchset: PS4: CR-1 due to TS 101.138 vs TS 101.318. File include/osmo-bts/bts.h: https://gerrit.osmocom.org/c/osmo-bts/+/32630/comment/3650d1a3_413486bc PS4, Line 71: 101.138 TS 101.318 File src/common/bts.c: https://gerrit.osmocom.org/c/osmo-bts/+/32630/comment/4b1d4102_9fab6d15 PS4, Line 208: 101.138 TS 101.318 File src/common/l1sap.c: https://gerrit.osmocom.org/c/osmo-bts/+/32630/comment/11c7b57f_2204b064 PS4, Line 1955: /* Since we already verified the payload in rtppayload_validate_hr(), we may trust that the payload length is > I meant something like this (assuming no weird lengths like len=0 are to be > received here): […] Agreeing with pespin here, his proposed approach looks more readable. Also, given that it's just one optional byte in front of the payload, the code can be simplified further: ``` msg = l1sap_msgb_alloc(rtp_pl_len); if (!msg) return NULL; memcpy(msgb_put(msg, rtp_pl_len), rtp_pl, rtp_pl_len); switch (rtp_pl_len) { case GSM_HR_BYTES_RTP_TS101318: /* Convert from TS 101 318 to RFC 5993 */ if (OSMO_UNLIKELY(!ts101318 && rfc5993)) msgb_push_u8(msg, 0x00); break; case GSM_HR_BYTES_RTP_RFC5993: /* Convert from RFC 5993 TS 101 318 */ if (OSMO_UNLIKELY(!rfc5993 && ts101318)) msgb_pull_u8(msg); break; default: OSMO_ASSERT(0); } ``` File src/osmo-bts-virtual/main.c: https://gerrit.osmocom.org/c/osmo-bts/+/32630/comment/7167ecac_dbd7a54d PS4, Line 76: BTS_INTERNAL_FLAG_SPEECH_H_V1_RTP_RFC5993 I am not 100% sure, but osmo-bts-virtual should in theory support both formats. -- To view, visit https://gerrit.osmocom.org/c/osmo-bts/+/32630 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-bts Gerrit-Branch: master Gerrit-Change-Id: I9419b40c1171876879d41aba4f51c93e8ef5673c Gerrit-Change-Number: 32630 Gerrit-PatchSet: 4 Gerrit-Owner: dexter <pma...@sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: fixeria <vyanits...@sysmocom.de> Gerrit-Reviewer: pespin <pes...@sysmocom.de> Gerrit-Attention: pespin <pes...@sysmocom.de> Gerrit-Attention: dexter <pma...@sysmocom.de> Gerrit-Comment-Date: Fri, 19 May 2023 11:19:13 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Comment-In-Reply-To: pespin <pes...@sysmocom.de> Gerrit-MessageType: comment
