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.