Attention is currently required from: falconia, pespin.

fixeria has posted comments on this change by falconia. ( https://gerrit.osmocom.org/c/osmo-bts/+/38553?usp=email )

Change subject: CSD: implement half-rate modes correctly ......................................................................

Patch Set 2: Code-Review+1

(5 comments)

Commit Message:

https://gerrit.osmocom.org/c/osmo-bts/+/38553/comment/f55d153e_392cfca0?usp=... : PS2, Line 26: by emitting two RTP packets : directly back-to-back for UL, and pulling two RTP packets at a time : from the Rx jitter buffer at the needed times for DL. I understand your idea now. Just curious, how would you do that if osmo-bts were an E1 BTS (not Abis-over-IP)? I guess the only option would be to buffer V.110 frames, since you cannot do the same trick for a TDM interface.

File src/common/l1sap.c:

https://gerrit.osmocom.org/c/osmo-bts/+/38553/comment/7e6234ce_4292240f?usp=... : PS2, Line 1493: /* TDMA frame number of burst 'a' % 26 is the table index.

This bit of code is not newly written with this patch - I simply moved it from osmo-bts-trx to the c […]

I agree with Mychaela here. It's a well known constant that is used quite often here and there.

https://gerrit.osmocom.org/c/osmo-bts/+/38553/comment/5aa500ad_532f6eb5?usp=... : PS2, Line 1539: 2 `ARRAY_SIZE(input_msg)` here and below, please.

https://gerrit.osmocom.org/c/osmo-bts/+/38553/comment/d0770a5a_b08f40ee?usp=... : PS2, Line 1556: else I would put a comment that it's an IDLE frame (all bits set to 1).

https://gerrit.osmocom.org/c/osmo-bts/+/38553/comment/8879c982_8934bcf2?usp=... : PS2, Line 2098: switch (lchan->tch_mode) { As a further improvement, I suggest sharing `csd_v110_lchan_desc[]`, so that it can be used here (`desc->num_blocks * desc->num_bits`). I have a WIP patch moving that lookup table to libosmocore: https://gerrit.osmocom.org/c/libosmocore/+/35914.