Attention is currently required from: pespin, dexter. falconia 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 3: (3 comments) File src/common/l1sap.c: https://gerrit.osmocom.org/c/osmo-bts/+/32714/comment/3a18ccbd_79bdce91 PS3, Line 1308: static inline bool nonamr_mand_sid_position(struct gsm_lchan *lchan, But this particular function is not limited to FR & EFR, this one already supports TCH/H too, so it is good for all 3 non-AMR codecs. https://gerrit.osmocom.org/c/osmo-bts/+/32714/comment/d82eeab9_44c700c7 PS3, Line 1415: } else Fair enough - I'll fix it in the next revision. https://gerrit.osmocom.org/c/osmo-bts/+/32714/comment/377bd5c7_ecc3fa56 PS3, Line 1441: if (!resp_msg && lchan->tch.dtx_fr_efr.last_rtp_input_was_sid && In the classic GSM architecture this logic was split between two separate components: the first part in the TRAU, the second part in the BTS. The part in the TRAU would cache and repeat the last valid SID per the rules of TS 28.062 section C.3.2.1.1, and the part in the BTS would then throw away most of those SID copies, keeping only those in the right positions per GSM 06.31/06.81 section 5.1.2. If we were to replicate that most-classic approach in OsmoBTS, the code would first resurrect a cached SID copy with l1sap_msgb_alloc(), and then just a few lines later toss it with msgb_free() - it would be very clean logically and exactly replicate what the specs intended, but do we really want the silliness and inefficiency of l1sap_msgb_alloc() immediately followed by msgb_free() for almost every frame position during periods of DTX silence? The code I have right now seems to be the best compromise between cleanness and efficiency - the flow of code from the top to the bottom of the screen-full exactly follows the logical order of conceptually separate transformations and state changes. -- 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: 3 Gerrit-Owner: falconia <falcon(a)freecalypso.org> Gerrit-Reviewer: Jenkins Builder Gerrit-CC: dexter <pmaier(a)sysmocom.de> Gerrit-CC: pespin <pespin(a)sysmocom.de> Gerrit-Attention: pespin <pespin(a)sysmocom.de> Gerrit-Attention: dexter <pmaier(a)sysmocom.de> Gerrit-Comment-Date: Tue, 16 May 2023 08:22:51 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: pespin <pespin(a)sysmocom.de> Gerrit-MessageType: comment