Attention is currently required from: fixeria, pespin.

falconia 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:

(4 comments)

File include/osmo-bts/csd_v110.h:

https://gerrit.osmocom.org/c/osmo-bts/+/38553/comment/b37caa59_22d35635?usp=email
 :
PS2, Line 11:   uint8_t ra2_ir;
> I can't figure out what this field is about by looking at it, maybe a short 
> comment here helps.
It is the intermediate rate (8 or 16 kbit/s) to be used in the RA2 
packing/unpacking step.


File src/common/l1sap.c:

https://gerrit.osmocom.org/c/osmo-bts/+/38553/comment/2ffbf778_eeccf58d?usp=email
 :
PS2, Line 1493: /* TDMA frame number of burst 'a' % 26 is the table index.
> where does this 26 come from? do we have a define for it? Can we add one?
This bit of code is not newly written with this patch - I simply moved it from 
osmo-bts-trx to the common layer. 26 is the number of TDMA frames per TCH 
multiframe, or the number of TDMA frames after which we get a perfectly 
repeating pattern of which frame numbers we need to trigger on for this or that 
function. The original TCH scheduling code in 
`src/osmo-bts-trx/sched_lchan_tch[fh].c` makes very heavy use of 
directly-written number 26.

I disagree with the obsessive compulsion to add a define for this 26. While I 
wrote "TCH multiframe" above, GSM hackers will most often refer to it as a 
"26-multiframe" in casual speech, and I argue that the meaning will be obvious 
to anyone who knows GSM. I argue we can make a reasonable assumption that 
anyone hacking on osmo-bts code can be expected to know the GSM multiframe 
structure of 05.02, and will know exactly what 26 refers to when they hack on 
or even look at code paths that deal with TCH.


https://gerrit.osmocom.org/c/osmo-bts/+/38553/comment/6e256b5c_cf8bfab4?usp=email
 :
PS2, Line 1523:         if (!sched_tchh_dl_csd_map[fn % 26])
> define for 26 to be used here to.
Again, I don't just have the number 26 randomly floating around in the code. 
Since the actual construct is FN modulo 26, anyone who knows GSM and is 
qualified to touch TCH handling code will immediately know what is going on 
here. Existing code in `src/osmo-bts-trx/sched_lchan_tch[fh].c` is exactly the 
same in this regard.


https://gerrit.osmocom.org/c/osmo-bts/+/38553/comment/6f6d21b9_7f65168a?usp=email
 :
PS2, Line 2045:  * back-to-back every 40 ms on the same frame number, */
> why are we sending with same frame number if they have different data? this 
> looks weird.
If you read my comment carefully, you'll see that I don't say anything about 
sending packets "with" the same frame number - instead I point out that we send 
two RTP packets **on** the same frame number. There is no GSM frame number 
field in RTP packets, hence sending "with" the same FN would be meaningless - 
but because we send two RTP packets (with different content, carrying first and 
second chunks of "virtual" 20-ms-worth) at the same instant in physical time, 
at the same point in our event-driven code execution, we are "on" the same GSM 
FN in terms of osmo-bts-trx view of GSM Um.

The timestamp field in these two successive RTP packets does increment 
correctly - I verified it by examining pcap files from TTCN3 tests performed by 
@[email protected], attached in OS#6579 ticket.

As for the weirdness of sending two RTP packets directly back to back in the 
first place, I concur - but what is the alternative? How else would you satisfy 
the requirements of TS 48.103 section 5.6 for CSD-HR, how would you reconcile 
the requirement for 20 ms RTP packets with the physical reality of the channel 
coder running only every 40 ms statistically? Perhaps artificially delay the 
second half by buffering it and then sending it out 20 ms later? But why add 
artificial delays, if we have the data "in hand", why not send it out right 
away, even though it will be 20 ms early relative to when an E1 BTS would begin 
serially transmitting the equivalent TRAU frame...



-- 
To view, visit https://gerrit.osmocom.org/c/osmo-bts/+/38553?usp=email
To unsubscribe, or for help writing mail filters, visit 
https://gerrit.osmocom.org/settings?usp=email

Gerrit-MessageType: comment
Gerrit-Project: osmo-bts
Gerrit-Branch: master
Gerrit-Change-Id: Ib35e910df263743cd243f51fb0bd6551ddfcf4c5
Gerrit-Change-Number: 38553
Gerrit-PatchSet: 2
Gerrit-Owner: falconia <[email protected]>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: fixeria <[email protected]>
Gerrit-CC: pespin <[email protected]>
Gerrit-Attention: pespin <[email protected]>
Gerrit-Attention: fixeria <[email protected]>
Gerrit-Comment-Date: Sun, 27 Oct 2024 20:35:49 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: pespin <[email protected]>

Reply via email to