[M] Change in libosmo-abis[master]: trau: new function osmo_trau2rtp_ufe() - gerrit-log

12 Mar 2025

Attention is currently required from: fixeria, laforge, pespin.

falconia has posted comments on this change by falconia. (
https://gerrit.osmocom.org/c/libosmo-abis/+/39731?usp=email )

Change subject: trau: new function osmo_trau2rtp_ufe()
......................................................................

Patch Set 2:

(2 comments)

File src/trau/trau_rtp_conv.c:

https://gerrit.osmocom.org/c/libosmo-abis/+/39731/comment/86fdaeab_d1091b40… :
PS2, Line 137:  *  \param[in] emit_twts001 self-explanatory
...
  It may be self explanatory to you, but no to others.
 Please add something like: "Generate RTP in TW-TS-001 format" or whatever the
name of the spec it is. 
OK, fine. In the next spin of this patch, I'll change the doxygen comment for
`emit_twts001` to "Generate RTP in TW-TS-001 format", and the one for
`emit_twts002` to "Generate RTP in TW-TS-002 format".

...
  Also some reference to where that spec may be found
may be useful. 
I don't agree that internal Doxygen comments for static functions internal to trau2rtp
implementation are the right place for such verbose documentation. The comment block for
`osmo_trau2rtp()` public API seems like the more sensible place. Please read the comments
that are already there, including description of RFC vs TW-TS output format selection, and
then tell me if you think that description should be extended.

https://gerrit.osmocom.org/c/libosmo-abis/+/39731/comment/3f9fc05d_5fe855db… :
PS2, Line 359: 
...
  Let's better initialize ufe always if present, to
avoid uninitialized memory. I disagree: I intentionally designed the new
`osmo_trau2rtp_ufe()` API to set `*ufe` to true if a UFE condition is detected, but
**not** set it to false otherwise. This aspect of the API is clearly documented in the
header comment for the new API function.

Rationale: CRC errors (HR and EFR) or invalid control bit pattern (HR only) are only one
type of UFE condition, but it is the UFE condition that is most naturally caught
simultaneously with conversion to RTP, as opposed to a separate check function. There are
other sources of UFE condition: errors in the frame sync pattern, or the frame type code
in C1..C5 bits (decoded in `osmo_trau_frame_decode_16k()` or `osmo_trau_frame_decode_8k()`
rather than here) suddenly being received as neither the original expected frame type nor
a different valid type that could be a plausible result of a Channel Mode Modify
operation. My vision is that an application such as `tw-e1abis-mgw` will need to maintain
a `bool ufe` flag in its TRAU state structure, it will be initialized (by the app) to
false at the beginning of Rx frame processing, and `osmo_trau2rtp_ufe()` is just one step
out of several that can change it from false to true.

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

Gerrit-MessageType: comment
Gerrit-Project: libosmo-abis
Gerrit-Branch: master
Gerrit-Change-Id: I7a90eca296b17b1211e5c2125fb1496cd362e1c9
Gerrit-Change-Number: 39731
Gerrit-PatchSet: 2
Gerrit-Owner: falconia &lt;falcon(a)freecalypso.org&gt;
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: fixeria &lt;vyanitskiy(a)sysmocom.de&gt;
Gerrit-Reviewer: laforge &lt;laforge(a)osmocom.org&gt;
Gerrit-CC: pespin &lt;pespin(a)sysmocom.de&gt;
Gerrit-Attention: laforge &lt;laforge(a)osmocom.org&gt;
Gerrit-Attention: pespin &lt;pespin(a)sysmocom.de&gt;
Gerrit-Attention: fixeria &lt;vyanitskiy(a)sysmocom.de&gt;
Gerrit-Comment-Date: Wed, 12 Mar 2025 19:31:50 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: pespin &lt;pespin(a)sysmocom.de&gt;