Attention is currently required from: jolly, laforge, fixeria, dexter. pespin has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-bts/+/31417 ) Change subject: l1sap: Accept RFC5993 and TS 101.318 HR GSM payload ...................................................................... Patch Set 8: (3 comments) File src/common/l1sap.c: https://gerrit.osmocom.org/c/osmo-bts/+/31417/comment/df6fc162_d2a603aa PS8, Line 1244: static bool rtppayload_is_valid(struct gsm_lchan *lchan, struct msgb *resp_msg) if this is expected to be used only for incoming DL packets we forward to lower layers towards the MS, we should really make this function name more descriptive. Because with current name it really looks like it could be used in any direction. https://gerrit.osmocom.org/c/osmo-bts/+/31417/comment/ba4ca3e6_53682eed PS8, Line 1274: /* BTS flags specify that RFC 5993 (and not ETSI TS 101.318) is understood by lower layers */ All this code block below you are writing can iiuc be simplified to: if (resp_msg->len == GSM_HR_BYTES_RTP_RFC5993) return rfc5993; if (resp_msg->len == GSM_HR_BYTES_RTP_TS101318) return ts101318; return false; https://gerrit.osmocom.org/c/osmo-bts/+/31417/comment/89f8f043_35819518 PS8, Line 1952: * formats are supported (either by setting both or none of the flags), no conversion will be carried out. */ we don't care about "both formats being supported". We care about the received format being supported. BTW, what's the point of checking the supported formats in rtppayload_is_valid() if we are anyway converting them here to whatever the supported format is? What am I missing? -- To view, visit https://gerrit.osmocom.org/c/osmo-bts/+/31417 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-bts Gerrit-Branch: master Gerrit-Change-Id: I453562da412fde5b928bd2b588129c58ec8e2a7e Gerrit-Change-Number: 31417 Gerrit-PatchSet: 8 Gerrit-Owner: dexter <pmaier(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: jolly <andreas(a)eversberg.eu> Gerrit-Reviewer: laforge <laforge(a)osmocom.org> Gerrit-Reviewer: pespin <pespin(a)sysmocom.de> Gerrit-CC: fixeria <vyanitskiy(a)sysmocom.de> Gerrit-Attention: jolly <andreas(a)eversberg.eu> Gerrit-Attention: laforge <laforge(a)osmocom.org> Gerrit-Attention: fixeria <vyanitskiy(a)sysmocom.de> Gerrit-Attention: dexter <pmaier(a)sysmocom.de> Gerrit-Comment-Date: Thu, 04 May 2023 16:00:34 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment