Attention is currently required from: neels.

pespin has posted comments on this change. ( 
https://gerrit.osmocom.org/c/osmo-mgw/+/35434?usp=email )

Change subject: add fmtp.h
......................................................................


Patch Set 4:

(6 comments)

Commit Message:

https://gerrit.osmocom.org/c/osmo-mgw/+/35434/comment/587ddad9_c963b35b
PS4, Line 9: Upcoming patches will add handling of arbitrary ftmp strings to
fmtp strings


https://gerrit.osmocom.org/c/osmo-mgw/+/35434/comment/b9932791_b59f7fb2
PS4, Line 13: Add generic API for handling ftmp strings. The primary intended 
user is
fmtp


Patchset:

PS4:
Only minor stuff needs improving/fixing, feel free to merge it once you fix 
those.


File src/libosmo-mgcp-client/fmtp.c:

https://gerrit.osmocom.org/c/osmo-mgw/+/35434/comment/eb0de39f_5f52c1e6
PS4, Line 29:   for (; fmtp && *fmtp && *fmtp != ';'; fmtp++);
fmtp being nonull should be checked only once at start.


https://gerrit.osmocom.org/c/osmo-mgw/+/35434/comment/97ac2864_6c03457c
PS4, Line 41:  *   if (osmo_sdp_fmtp_get_val(res, sizeof(res), fmtp_vals, 
"mode-set"))
IIUC res contains "0,2,4,7" here, maybe worth saying that.


https://gerrit.osmocom.org/c/osmo-mgw/+/35434/comment/cad9b599_1fe5110a
PS4, Line 89: int osmo_sdp_fmtp_get_int(const char *fmtp, const char 
*option_name, int default_value)
did you check in SDP spec if int range is enough?



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

Gerrit-Project: osmo-mgw
Gerrit-Branch: master
Gerrit-Change-Id: I3eaea353dbd98c19212199b564342d0ac16cbc07
Gerrit-Change-Number: 35434
Gerrit-PatchSet: 4
Gerrit-Owner: neels <[email protected]>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: laforge <[email protected]>
Gerrit-CC: pespin <[email protected]>
Gerrit-Attention: neels <[email protected]>
Gerrit-Comment-Date: Tue, 02 Jan 2024 09:59:30 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Gerrit-MessageType: comment

Reply via email to