Attention is currently required from: pespin, dexter.
2 comments:
File src/common/l1sap.c:
Patch Set #3, 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 […]
@dexter - sure, let's factor it out. I shall prepare another patchset with two static functions factored out: one for SID analysis of just-received RTP frames, and the other for the decision & transformation logic.
File src/common/l1sap.c:
Patch Set #4, 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. […]
It looks like I need to change that comment and include a longer explanation of why I am not supporting HR1 yet. OS#5688 is one part of the problem, but the other (big) part of the problem is that we don't have a function like osmo_{fr,efr}_sid_classify() for HR1. For FR and EFR I am able to make care-free use of simple osmo_{fr,efr}_check_sid() functions only because in an earlier commit we added calls to osmo_{fr,efr}_sid_preen() into the code path that happens just before the present step, and after these preening functions the codec frame can only be either non-SID speech or a perfect error-free SID - the preening functions clean up other possibilities. And those preening functions are in turn based on osmo_{fr,efr}_sid_classify().
Why can't we simply add a SID classification function for HR1 just like I did for FR and EFR? The fundamental problem is that unlike 06.31 and 06.81, the GSM 06.41 spec for HR DTX does NOT prescribe a fixed set of rules for valid and invalid SID classification - instead it is left up to the discretion of channel decoder (!) implementors. In the original E1 Abis architecture the "SID class" ternary flag would travel (as two out-of-band control bits) along with the data bits in TRAU frames, hence only the entity that does the lowest-level channel decoding gets to decide exactly which rules to apply - and all downstream entities follow what those two out-of-band bits say. In the case of TFO calls the TFO block in the downstream TRAU would look at the SID class bits passed inside TFO frames from the upstream TRAU - but with our RTP-based TrFO taking the place of T1/E1 TFO, those original SID class bits are lost.
I plan on covering this topic in June OsmoDevCall. Until then we can either merge the patch adding proper FR & EFR handling while leaving HR1 as a FIXME for other/later developers, or we could postpone merging of this patch until we thoroughly talk about this entire topic in June.
To view, visit change 32714. To unsubscribe, or for help writing mail filters, visit settings.