Attention is currently required from: falconia.
Patch set 4:Code-Review +1
5 comments:
Patchset:
This looks very clean to me. The elaborated comments are also very helpful to understand whats going on.
File src/common/l1sap.c:
Patch Set #3, Line 1308: static inline bool nonamr_mand_sid_position(struct gsm_lchan *lchan,
In the latest patchset just uploaded, I renamed this function to fr_hr_efr_sid_position().
Done
Patch Set #3, Line 1441: if (!resp_msg && lchan->tch.dtx_fr_efr.last_rtp_input_was_sid &&
In the new patchset I added a bunch of comments, extensively documenting the flow of that complex co […]
maybe just putting the whole code path in a static function would already improve the readability of the code.
File src/common/l1sap.c:
Patch Set #4, Line 1384: /* non-AMR SID handling */
maybe /* FR, HR, EFR SID handling */, or just /* SID handling */ (I think is obvious that this only affects HR/FR and EFR.)
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. I have submitted a patch (I25728299b757fbc87dd1b3f5adaec9b8b240c5d1) to make osmo_hr_check_sid() compatible with both payload types. So you can use it here without worrying about in which format the payload is.
To view, visit change 32714. To unsubscribe, or for help writing mail filters, visit settings.