Attention is currently required from: falconia. dexter has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-bts/+/32714 ) Change subject: TCH DL, FR & EFR: reshape SIDs per 06.31/06.81 section 5.1.2 ...................................................................... Patch Set 4: Code-Review+1 (5 comments) Patchset: PS4: This looks very clean to me. The elaborated comments are also very helpful to understand whats going on. File src/common/l1sap.c: https://gerrit.osmocom.org/c/osmo-bts/+/32714/comment/cffdfbcd_ae7c04dd PS3, Line 1308: static inline bool nonamr_mand_sid_position(struct gsm_lchan *lchan, Done https://gerrit.osmocom.org/c/osmo-bts/+/32714/comment/2dc50c4e_60b78772 PS3, Line 1441: if (!resp_msg && lchan->tch.dtx_fr_efr.last_rtp_input_was_sid && maybe just putting the whole code path in a static function would already improve the readability of the code. File src/common/l1sap.c: https://gerrit.osmocom.org/c/osmo-bts/+/32714/comment/bfe2abc9_78f645a3 PS4, Line 1384: /* non-AMR SID handling */ maybe /* FR, HR, EFR SID handling */, or just /* SID handling */ (I think is obvious that this only affects HR/FR and EFR.) https://gerrit.osmocom.org/c/osmo-bts/+/32714/comment/282d3d2f_d818b0d1 PS4, Line 1393: * until OS#5688 is resolved. */ I guess the problem is that you do not know which of the two payload formats you will see here. I have submitted a patch (I25728299b757fbc87dd1b3f5adaec9b8b240c5d1) to make osmo_hr_check_sid() compatible with both payload types. So you can use it here without worrying about in which format the payload is. -- To view, visit https://gerrit.osmocom.org/c/osmo-bts/+/32714 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-bts Gerrit-Branch: master Gerrit-Change-Id: I924ab21952dcf8bb03ba7ccef790474bf66fc9e5 Gerrit-Change-Number: 32714 Gerrit-PatchSet: 4 Gerrit-Owner: falconia <falcon(a)freecalypso.org> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: dexter <pmaier(a)sysmocom.de> Gerrit-Reviewer: pespin <pespin(a)sysmocom.de> Gerrit-Attention: falconia <falcon(a)freecalypso.org> Gerrit-Comment-Date: Wed, 17 May 2023 08:52:05 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Comment-In-Reply-To: falconia <falcon(a)freecalypso.org> Comment-In-Reply-To: pespin <pespin(a)sysmocom.de> Gerrit-MessageType: comment