Attention is currently required from: fixeria, laforge, pespin.
2 comments:
File src/trau/trau_rtp_conv.c:
Patch Set #2, 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.
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 change 39731. To unsubscribe, or for help writing mail filters, visit settings.