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 <nhofmeyr(a)sysmocom.de>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: laforge <laforge(a)osmocom.org>
Gerrit-CC: pespin <pespin(a)sysmocom.de>
Gerrit-Attention: neels <nhofmeyr(a)sysmocom.de>
Gerrit-Comment-Date: Tue, 02 Jan 2024 09:59:30 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Gerrit-MessageType: comment