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?