[L] Change in osmo-mgw[master]: add fmtp string to ptmap: allow all possible fmtp
Attention is currently required from: laforge, neels. pespin has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-mgw/+/34900?usp=email ) Change subject: add fmtp string to ptmap: allow all possible fmtp .. Patch Set 10: (1 comment) File src/libosmo-mgcp-client/mgcp_client.c: https://gerrit.osmocom.org/c/osmo-mgw/+/34900/comment/67fc76e4_e93b0851 PS9, Line 1290: && (mgcp_msg->ptmap[i].codec == CODEC_AMR_8000_1 > - it's much more efficient to read the condition with operators at the start. > […] sure, just wanted to point thsi out since it appeared here. -- To view, visit https://gerrit.osmocom.org/c/osmo-mgw/+/34900?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: If58590bda8627519ff07e0b6f43aa47a274f052b Gerrit-Change-Number: 34900 Gerrit-PatchSet: 10 Gerrit-Owner: neels Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: laforge Gerrit-Reviewer: neels Gerrit-Reviewer: pespin Gerrit-CC: dexter Gerrit-Attention: neels Gerrit-Attention: laforge Gerrit-Comment-Date: Wed, 13 Dec 2023 11:40:53 + Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: neels Comment-In-Reply-To: pespin Gerrit-MessageType: comment
[L] Change in osmo-mgw[master]: add fmtp string to ptmap: allow all possible fmtp
Attention is currently required from: laforge, pespin. neels has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-mgw/+/34900?usp=email ) Change subject: add fmtp string to ptmap: allow all possible fmtp .. Patch Set 10: (3 comments) File include/osmocom/mgcp/fmtp.h: https://gerrit.osmocom.org/c/osmo-mgw/+/34900/comment/09d33fc1_163b5854 PS10, Line 1: #pragma once > libosmocore is too generic for mgcp specific stuff. […] ok, will try that File src/libosmo-mgcp-client/mgcp_client.c: https://gerrit.osmocom.org/c/osmo-mgw/+/34900/comment/8e78d209_ad654820 PS6, Line 1372: ! > == (somehow gerrit lost the right place for this. the == is below.) File src/libosmo-mgcp-client/mgcp_client.c: https://gerrit.osmocom.org/c/osmo-mgw/+/34900/comment/3ab13f10_31029c60 PS9, Line 1290: && (mgcp_msg->ptmap[i].codec == CODEC_AMR_8000_1 > (fact: the problem with the "&&" at the start of the line instead of > appending at the end of last on […] - it's much more efficient to read the condition with operators at the start. at the start, they are in a fixed position, and form a tree structure (akin to a file tree view) with the operators as the node bullets. With operators at the end, the eye needs to scan around. - the loss of indent happens only once per logical branch in the condition and is only 3 chars... i would very much like to continue using this structuring in my conditions. -- To view, visit https://gerrit.osmocom.org/c/osmo-mgw/+/34900?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: If58590bda8627519ff07e0b6f43aa47a274f052b Gerrit-Change-Number: 34900 Gerrit-PatchSet: 10 Gerrit-Owner: neels Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: laforge Gerrit-Reviewer: neels Gerrit-Reviewer: pespin Gerrit-CC: dexter Gerrit-Attention: laforge Gerrit-Attention: pespin Gerrit-Comment-Date: Wed, 13 Dec 2023 01:46:56 + Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: neels Comment-In-Reply-To: laforge Comment-In-Reply-To: pespin Gerrit-MessageType: comment
[L] Change in osmo-mgw[master]: add fmtp string to ptmap: allow all possible fmtp
Attention is currently required from: laforge, neels. pespin has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-mgw/+/34900?usp=email ) Change subject: add fmtp string to ptmap: allow all possible fmtp .. Patch Set 10: (1 comment) File include/osmocom/mgcp/fmtp.h: https://gerrit.osmocom.org/c/osmo-mgw/+/34900/comment/10e1607d_e2e0a21e PS10, Line 1: #pragma once > libosmocore is too generic for mgcp specific stuff. […] Agree with Harald. Some mgcp_common.a may make sense. -- To view, visit https://gerrit.osmocom.org/c/osmo-mgw/+/34900?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: If58590bda8627519ff07e0b6f43aa47a274f052b Gerrit-Change-Number: 34900 Gerrit-PatchSet: 10 Gerrit-Owner: neels Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: laforge Gerrit-Reviewer: neels Gerrit-Reviewer: pespin Gerrit-CC: dexter Gerrit-Attention: neels Gerrit-Attention: laforge Gerrit-Comment-Date: Mon, 11 Dec 2023 09:36:16 + Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: neels Comment-In-Reply-To: laforge Gerrit-MessageType: comment
[L] Change in osmo-mgw[master]: add fmtp string to ptmap: allow all possible fmtp
Attention is currently required from: neels. pespin has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-mgw/+/34900?usp=email ) Change subject: add fmtp string to ptmap: allow all possible fmtp .. Patch Set 10: Code-Review+1 -- To view, visit https://gerrit.osmocom.org/c/osmo-mgw/+/34900?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: If58590bda8627519ff07e0b6f43aa47a274f052b Gerrit-Change-Number: 34900 Gerrit-PatchSet: 10 Gerrit-Owner: neels Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: laforge Gerrit-Reviewer: neels Gerrit-Reviewer: pespin Gerrit-CC: dexter Gerrit-Attention: neels Gerrit-Comment-Date: Mon, 11 Dec 2023 09:35:40 + Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment
[L] Change in osmo-mgw[master]: add fmtp string to ptmap: allow all possible fmtp
Attention is currently required from: neels, pespin. laforge has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-mgw/+/34900?usp=email ) Change subject: add fmtp string to ptmap: allow all possible fmtp .. Patch Set 10: (1 comment) File include/osmocom/mgcp/fmtp.h: https://gerrit.osmocom.org/c/osmo-mgw/+/34900/comment/84a3fd26_6a9f4b4d PS10, Line 1: #pragma once > RFC: add this header to libosmocore instead? […] libosmocore is too generic for mgcp specific stuff. I don't really think it would be wrong to link osmo-mgw also against libosmo-mgcp-client, even though it's ugly. You might also be able to * add it to libosmo-mgcp-client * also link to that one spcecific file from osmo-mgw (either directly, or via a separate internal .a libary) -- To view, visit https://gerrit.osmocom.org/c/osmo-mgw/+/34900?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: If58590bda8627519ff07e0b6f43aa47a274f052b Gerrit-Change-Number: 34900 Gerrit-PatchSet: 10 Gerrit-Owner: neels Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: laforge Gerrit-Reviewer: neels Gerrit-Reviewer: pespin Gerrit-CC: dexter Gerrit-Attention: neels Gerrit-Attention: pespin Gerrit-Comment-Date: Sun, 10 Dec 2023 11:25:29 + Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: neels Gerrit-MessageType: comment
[L] Change in osmo-mgw[master]: add fmtp string to ptmap: allow all possible fmtp
Attention is currently required from: laforge, pespin. neels has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-mgw/+/34900?usp=email ) Change subject: add fmtp string to ptmap: allow all possible fmtp .. Patch Set 10: (1 comment) File include/osmocom/mgcp/fmtp.h: https://gerrit.osmocom.org/c/osmo-mgw/+/34900/comment/1970e2da_9068ebec PS10, Line 1: #pragma once RFC: add this header to libosmocore instead? Rationale: This is to be used in osmo-mgw, also useful for MGCP clients that link libosmo-mgcp-client, to compose fmtp (for AMR). Now, if we install the header in libosmo-mgcp-client, can we still use this header in osmo-mgw? So far the only shared part is an "internal" header. But when I add this in libosmocore, it is trivially available everywhere. -- To view, visit https://gerrit.osmocom.org/c/osmo-mgw/+/34900?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: If58590bda8627519ff07e0b6f43aa47a274f052b Gerrit-Change-Number: 34900 Gerrit-PatchSet: 10 Gerrit-Owner: neels Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: laforge Gerrit-Reviewer: neels Gerrit-Reviewer: pespin Gerrit-CC: dexter Gerrit-Attention: laforge Gerrit-Attention: pespin Gerrit-Comment-Date: Sun, 10 Dec 2023 06:29:53 + Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment
[L] Change in osmo-mgw[master]: add fmtp string to ptmap: allow all possible fmtp
Attention is currently required from: laforge, neels, pespin.
Hello Jenkins Builder, laforge, pespin,
I'd like you to reexamine a change. Please visit
https://gerrit.osmocom.org/c/osmo-mgw/+/34900?usp=email
to look at the new patch set (#10).
The following approvals got outdated and were removed:
Code-Review+1 by pespin, Verified+1 by Jenkins Builder
Change subject: add fmtp string to ptmap: allow all possible fmtp
..
add fmtp string to ptmap: allow all possible fmtp
Remove the limit of having only one AMR octet-aligned fmtp parameter per
MGCP message. Instead allow any arbitrary fmtp options, one per every
codec.
Deprecate all use of struct mgcp_codec_param. Instead, store and pass
plain fmtp strings.
We need to know fmtp details only for AMR, and only to probe whether it
is octet-aligned. So add a separate fmtp string parser that returns that
information flexibly, as in
if (osmo_mgcp_fmtp_get_int("octet-aligned", 0) == 1) ...
Provide legacy shims that still act correctly for any callers that may
pass the old struct mgcp_codec_param. (I'm not sure if we need to keep
this, but we can always drop it in another patch.)
Adjust one mgcp_test.c: instead of returning only the octet-aligned
parameter, now osmo-mgw keeps and returns all the fmtp parameters that
the user provided. So add the missing "mode-change-capability".
Related: OS#6171
Related: osmo-msc Ief9225c9bcf7525a9a0a07c282ffb8cc0d092186
Change-Id: If58590bda8627519ff07e0b6f43aa47a274f052b
---
M include/osmocom/mgcp/Makefile.am
A include/osmocom/mgcp/fmtp.h
M include/osmocom/mgcp/mgcp_codec.h
M include/osmocom/mgcp/mgcp_common.h
M include/osmocom/mgcp/mgcp_network.h
M include/osmocom/mgcp_client/mgcp_client.h
M include/osmocom/mgcp_client/mgcp_client_fsm.h
M src/libosmo-mgcp-client/mgcp_client.c
M src/libosmo-mgcp/Makefile.am
A src/libosmo-mgcp/fmtp.c
M src/libosmo-mgcp/mgcp_codec.c
M src/libosmo-mgcp/mgcp_network.c
M src/libosmo-mgcp/mgcp_protocol.c
M src/libosmo-mgcp/mgcp_sdp.c
M tests/mgcp/mgcp_test.c
15 files changed, 265 insertions(+), 100 deletions(-)
git pull ssh://gerrit.osmocom.org:29418/osmo-mgw refs/changes/00/34900/10
--
To view, visit https://gerrit.osmocom.org/c/osmo-mgw/+/34900?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: If58590bda8627519ff07e0b6f43aa47a274f052b
Gerrit-Change-Number: 34900
Gerrit-PatchSet: 10
Gerrit-Owner: neels
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: laforge
Gerrit-Reviewer: neels
Gerrit-Reviewer: pespin
Gerrit-CC: dexter
Gerrit-Attention: neels
Gerrit-Attention: laforge
Gerrit-Attention: pespin
Gerrit-MessageType: newpatchset
[L] Change in osmo-mgw[master]: add fmtp string to ptmap: allow all possible fmtp
Attention is currently required from: laforge, neels. pespin has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-mgw/+/34900?usp=email ) Change subject: add fmtp string to ptmap: allow all possible fmtp .. Patch Set 9: Code-Review+1 (1 comment) File src/libosmo-mgcp-client/mgcp_client.c: https://gerrit.osmocom.org/c/osmo-mgw/+/34900/comment/1ce0b0e1_d11b3880 PS9, Line 1290: && (mgcp_msg->ptmap[i].codec == CODEC_AMR_8000_1 (fact: the problem with the "&&" at the start of the line instead of appending at the end of last one is that then everything following in the next lines is indented more to the right, loosing line space.) -- To view, visit https://gerrit.osmocom.org/c/osmo-mgw/+/34900?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: If58590bda8627519ff07e0b6f43aa47a274f052b Gerrit-Change-Number: 34900 Gerrit-PatchSet: 9 Gerrit-Owner: neels Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: laforge Gerrit-Reviewer: neels Gerrit-Reviewer: pespin Gerrit-CC: dexter Gerrit-Attention: neels Gerrit-Attention: laforge Gerrit-Comment-Date: Fri, 08 Dec 2023 13:14:54 + Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Gerrit-MessageType: comment
[L] Change in osmo-mgw[master]: add fmtp string to ptmap: allow all possible fmtp
Attention is currently required from: laforge, neels, pespin.
Hello Jenkins Builder, laforge, pespin,
I'd like you to reexamine a change. Please visit
https://gerrit.osmocom.org/c/osmo-mgw/+/34900?usp=email
to look at the new patch set (#9).
The following approvals got outdated and were removed:
Verified+1 by Jenkins Builder
Change subject: add fmtp string to ptmap: allow all possible fmtp
..
add fmtp string to ptmap: allow all possible fmtp
Remove the limit of having only one AMR octet-aligned fmtp parameter per
MGCP message. Instead allow any arbitrary fmtp options, one per every
codec.
Deprecate all use of struct mgcp_codec_param. Instead, store and pass
plain fmtp strings.
We need to know fmtp details only for AMR, and only to probe whether it
is octet-aligned. So add a separate fmtp string parser that returns that
information flexibly, as in
if (osmo_mgcp_fmtp_get_int("octet-aligned", 0) == 1) ...
Provide legacy shims that still act correctly for any callers that may
pass the old struct mgcp_codec_param. (I'm not sure if we need to keep
this, but we can always drop it in another patch.)
Adjust one mgcp_test.c: instead of returning only the octet-aligned
parameter, now osmo-mgw keeps and returns all the fmtp parameters that
the user provided. So add the missing "mode-change-capability".
Related: OS#6171
Change-Id: If58590bda8627519ff07e0b6f43aa47a274f052b
---
M include/osmocom/mgcp/Makefile.am
A include/osmocom/mgcp/fmtp.h
M include/osmocom/mgcp/mgcp_codec.h
M include/osmocom/mgcp/mgcp_common.h
M include/osmocom/mgcp/mgcp_network.h
M include/osmocom/mgcp_client/mgcp_client.h
M include/osmocom/mgcp_client/mgcp_client_fsm.h
M src/libosmo-mgcp-client/mgcp_client.c
M src/libosmo-mgcp/Makefile.am
A src/libosmo-mgcp/fmtp.c
M src/libosmo-mgcp/mgcp_codec.c
M src/libosmo-mgcp/mgcp_network.c
M src/libosmo-mgcp/mgcp_protocol.c
M src/libosmo-mgcp/mgcp_sdp.c
M tests/mgcp/mgcp_test.c
15 files changed, 265 insertions(+), 100 deletions(-)
git pull ssh://gerrit.osmocom.org:29418/osmo-mgw refs/changes/00/34900/9
--
To view, visit https://gerrit.osmocom.org/c/osmo-mgw/+/34900?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: If58590bda8627519ff07e0b6f43aa47a274f052b
Gerrit-Change-Number: 34900
Gerrit-PatchSet: 9
Gerrit-Owner: neels
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: laforge
Gerrit-Reviewer: neels
Gerrit-Reviewer: pespin
Gerrit-CC: dexter
Gerrit-Attention: neels
Gerrit-Attention: laforge
Gerrit-Attention: pespin
Gerrit-MessageType: newpatchset
[L] Change in osmo-mgw[master]: add fmtp string to ptmap: allow all possible fmtp
Attention is currently required from: laforge, neels, pespin.
Hello Jenkins Builder, laforge, pespin,
I'd like you to reexamine a change. Please visit
https://gerrit.osmocom.org/c/osmo-mgw/+/34900?usp=email
to look at the new patch set (#7).
The following approvals got outdated and were removed:
Code-Review+1 by pespin, Code-Review+2 by laforge, Code-Review-1 by neels,
Verified+1 by Jenkins Builder
The change is no longer submittable: Code-Review and Verified are unsatisfied
now.
Change subject: add fmtp string to ptmap: allow all possible fmtp
..
add fmtp string to ptmap: allow all possible fmtp
Remove the limit of having only one AMR octet-aligned fmtp parameter per
MGCP message. Instead allow any arbitrary fmtp options, one per every
codec.
Deprecate all use of struct mgcp_codec_param. Instead, store and pass
plain fmtp strings.
We need to know fmtp details only for AMR, and only to probe whether it
is octet-aligned. So add a separate fmtp string parser that returns that
information flexibly, as in
if (osmo_mgcp_fmtp_get_int("octet-aligned", 0) == 1) ...
Provide legacy shims that still act correctly for any callers that may
pass the old struct mgcp_codec_param. (I'm not sure if we need to keep
this, but we can always drop it in another patch.)
Adjust one mgcp_test.c: instead of returning only the octet-aligned
parameter, now osmo-mgw keeps and returns all the fmtp parameters that
the user provided. So add the missing "mode-change-capability".
Related: OS#6171
Change-Id: If58590bda8627519ff07e0b6f43aa47a274f052b
---
M include/osmocom/mgcp/Makefile.am
A include/osmocom/mgcp/fmtp.h
M include/osmocom/mgcp/mgcp_codec.h
M include/osmocom/mgcp/mgcp_common.h
M include/osmocom/mgcp/mgcp_network.h
M include/osmocom/mgcp_client/mgcp_client.h
M include/osmocom/mgcp_client/mgcp_client_fsm.h
M src/libosmo-mgcp-client/mgcp_client.c
M src/libosmo-mgcp/Makefile.am
A src/libosmo-mgcp/fmtp.c
M src/libosmo-mgcp/mgcp_codec.c
M src/libosmo-mgcp/mgcp_network.c
M src/libosmo-mgcp/mgcp_protocol.c
M src/libosmo-mgcp/mgcp_sdp.c
M tests/mgcp/mgcp_test.c
15 files changed, 268 insertions(+), 100 deletions(-)
git pull ssh://gerrit.osmocom.org:29418/osmo-mgw refs/changes/00/34900/7
--
To view, visit https://gerrit.osmocom.org/c/osmo-mgw/+/34900?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: If58590bda8627519ff07e0b6f43aa47a274f052b
Gerrit-Change-Number: 34900
Gerrit-PatchSet: 7
Gerrit-Owner: neels
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: laforge
Gerrit-Reviewer: neels
Gerrit-Reviewer: pespin
Gerrit-CC: dexter
Gerrit-Attention: neels
Gerrit-Attention: laforge
Gerrit-Attention: pespin
Gerrit-MessageType: newpatchset
[L] Change in osmo-mgw[master]: add fmtp string to ptmap: allow all possible fmtp
neels has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-mgw/+/34900?usp=email ) Change subject: add fmtp string to ptmap: allow all possible fmtp .. Patch Set 6: (1 comment) File tests/mgcp/mgcp_test.c: https://gerrit.osmocom.org/c/osmo-mgw/+/34900/comment/19b274fc_3b5f792a PS6, Line 848: fflush(stdout); separate patch -- To view, visit https://gerrit.osmocom.org/c/osmo-mgw/+/34900?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: If58590bda8627519ff07e0b6f43aa47a274f052b Gerrit-Change-Number: 34900 Gerrit-PatchSet: 6 Gerrit-Owner: neels Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: laforge Gerrit-Reviewer: neels Gerrit-Reviewer: pespin Gerrit-CC: dexter Gerrit-Comment-Date: Thu, 30 Nov 2023 02:55:54 + Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment
[L] Change in osmo-mgw[master]: add fmtp string to ptmap: allow all possible fmtp
neels has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-mgw/+/34900?usp=email ) Change subject: add fmtp string to ptmap: allow all possible fmtp .. Patch Set 6: Code-Review-1 (4 comments) File src/libosmo-mgcp-client/mgcp_client.c: https://gerrit.osmocom.org/c/osmo-mgw/+/34900/comment/f23a3aec_9a725e02 PS6, Line 1372: ! == https://gerrit.osmocom.org/c/osmo-mgw/+/34900/comment/dd0fc1b9_19ab61ee PS6, Line 1376: octet-align=1 use OSMO_MGCP_AMR_OCTET_ALIGN_EQUALS(1) https://gerrit.osmocom.org/c/osmo-mgw/+/34900/comment/35cc54c6_06ae9018 PS6, Line 1376: a=fmtp:%u there should be some def like #define FOO "a=fmtp:" https://gerrit.osmocom.org/c/osmo-mgw/+/34900/comment/af9539a1_fd1629a3 PS6, Line 1378: octet use OSMO_MGCP_AMR_OCTET_ALIGN_EQUALS(0) -- To view, visit https://gerrit.osmocom.org/c/osmo-mgw/+/34900?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: If58590bda8627519ff07e0b6f43aa47a274f052b Gerrit-Change-Number: 34900 Gerrit-PatchSet: 6 Gerrit-Owner: neels Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: laforge Gerrit-Reviewer: neels Gerrit-Reviewer: pespin Gerrit-CC: dexter Gerrit-Comment-Date: Thu, 30 Nov 2023 02:54:23 + Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Gerrit-MessageType: comment
[L] Change in osmo-mgw[master]: add fmtp string to ptmap: allow all possible fmtp
Attention is currently required from: neels.
Hello Jenkins Builder, laforge, pespin,
I'd like you to reexamine a change. Please visit
https://gerrit.osmocom.org/c/osmo-mgw/+/34900?usp=email
to look at the new patch set (#6).
The following approvals got outdated and were removed:
Verified+1 by Jenkins Builder
The change is no longer submittable: Verified is unsatisfied now.
Change subject: add fmtp string to ptmap: allow all possible fmtp
..
add fmtp string to ptmap: allow all possible fmtp
Remove the limit of having only one AMR octet-aligned fmtp parameter per
MGCP message. Instead allow any arbitrary fmtp options, one per every
codec.
Deprecate all use of struct mgcp_codec_param. Instead, store and pass
plain fmtp strings.
We need to know fmtp details only for AMR, and only to probe whether it
is octet-aligned. So add a separate fmtp string parser that returns that
information flexibly, as in
if (osmo_mgcp_fmtp_get_int("octet-aligned", 0) == 1) ...
Provide legacy shims that still act correctly for any callers that may
pass the old struct mgcp_codec_param. (I'm not sure if we need to keep
this, but we can always drop it in another patch.)
Adjust one mgcp_test.c: instead of returning only the octet-aligned
parameter, now osmo-mgw keeps and returns all the fmtp parameters that
the user provided. So add the missing "mode-change-capability".
Related: OS#6171
Change-Id: If58590bda8627519ff07e0b6f43aa47a274f052b
---
M include/osmocom/mgcp/Makefile.am
A include/osmocom/mgcp/fmtp.h
M include/osmocom/mgcp/mgcp_codec.h
M include/osmocom/mgcp/mgcp_common.h
M include/osmocom/mgcp/mgcp_network.h
M include/osmocom/mgcp_client/mgcp_client.h
M include/osmocom/mgcp_client/mgcp_client_fsm.h
M src/libosmo-mgcp-client/mgcp_client.c
M src/libosmo-mgcp/Makefile.am
A src/libosmo-mgcp/fmtp.c
M src/libosmo-mgcp/mgcp_codec.c
M src/libosmo-mgcp/mgcp_network.c
M src/libosmo-mgcp/mgcp_protocol.c
M src/libosmo-mgcp/mgcp_sdp.c
M tests/mgcp/mgcp_test.c
15 files changed, 270 insertions(+), 105 deletions(-)
git pull ssh://gerrit.osmocom.org:29418/osmo-mgw refs/changes/00/34900/6
--
To view, visit https://gerrit.osmocom.org/c/osmo-mgw/+/34900?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: If58590bda8627519ff07e0b6f43aa47a274f052b
Gerrit-Change-Number: 34900
Gerrit-PatchSet: 6
Gerrit-Owner: neels
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: laforge
Gerrit-Reviewer: pespin
Gerrit-CC: dexter
Gerrit-Attention: neels
Gerrit-MessageType: newpatchset
[L] Change in osmo-mgw[master]: add fmtp string to ptmap: allow all possible fmtp
Attention is currently required from: neels. laforge has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-mgw/+/34900?usp=email ) Change subject: add fmtp string to ptmap: allow all possible fmtp .. Patch Set 5: Code-Review+2 -- To view, visit https://gerrit.osmocom.org/c/osmo-mgw/+/34900?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: If58590bda8627519ff07e0b6f43aa47a274f052b Gerrit-Change-Number: 34900 Gerrit-PatchSet: 5 Gerrit-Owner: neels Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: laforge Gerrit-Reviewer: pespin Gerrit-CC: dexter Gerrit-Attention: neels Gerrit-Comment-Date: Thu, 02 Nov 2023 21:43:19 + Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment
[L] Change in osmo-mgw[master]: add fmtp string to ptmap: allow all possible fmtp
Attention is currently required from: laforge, neels. pespin has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-mgw/+/34900?usp=email ) Change subject: add fmtp string to ptmap: allow all possible fmtp .. Patch Set 5: Code-Review+1 -- To view, visit https://gerrit.osmocom.org/c/osmo-mgw/+/34900?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: If58590bda8627519ff07e0b6f43aa47a274f052b Gerrit-Change-Number: 34900 Gerrit-PatchSet: 5 Gerrit-Owner: neels Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: laforge Gerrit-Reviewer: pespin Gerrit-CC: dexter Gerrit-Attention: neels Gerrit-Attention: laforge Gerrit-Comment-Date: Thu, 02 Nov 2023 09:45:18 + Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment
[L] Change in osmo-mgw[master]: add fmtp string to ptmap: allow all possible fmtp
Attention is currently required from: laforge, pespin. neels has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-mgw/+/34900?usp=email ) Change subject: add fmtp string to ptmap: allow all possible fmtp .. Patch Set 5: (2 comments) File include/osmocom/mgcp_client/mgcp_client.h: https://gerrit.osmocom.org/c/osmo-mgw/+/34900/comment/60561c13_319e5059 PS4, Line 165: * Not all pointers contained in the mgcp_response */ > erm ... no i didn't write this. i started out with dexter's patch, maybe > that's where it came from.. […] Done File tests/mgcp/mgcp_test.c: https://gerrit.osmocom.org/c/osmo-mgw/+/34900/comment/3a136ae5_3a508922 PS4, Line 848: fflush(stderr); > another such something i didn't see, thx Done -- To view, visit https://gerrit.osmocom.org/c/osmo-mgw/+/34900?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: If58590bda8627519ff07e0b6f43aa47a274f052b Gerrit-Change-Number: 34900 Gerrit-PatchSet: 5 Gerrit-Owner: neels Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: laforge Gerrit-CC: dexter Gerrit-CC: pespin Gerrit-Attention: laforge Gerrit-Attention: pespin Gerrit-Comment-Date: Wed, 01 Nov 2023 21:16:52 + Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: neels Comment-In-Reply-To: pespin Gerrit-MessageType: comment
[L] Change in osmo-mgw[master]: add fmtp string to ptmap: allow all possible fmtp
Attention is currently required from: laforge, pespin.
Hello Jenkins Builder, laforge,
I'd like you to reexamine a change. Please visit
https://gerrit.osmocom.org/c/osmo-mgw/+/34900?usp=email
to look at the new patch set (#5).
The following approvals got outdated and were removed:
Code-Review+1 by laforge, Verified+1 by Jenkins Builder
Change subject: add fmtp string to ptmap: allow all possible fmtp
..
add fmtp string to ptmap: allow all possible fmtp
Remove the limit of having only one AMR octet-aligned fmtp parameter per
MGCP message. Instead allow any arbitrary fmtp options, one per every
codec.
Deprecate all use of struct mgcp_codec_param. Instead, store and pass
plain fmtp strings.
We need to know fmtp details only for AMR, and only to probe whether it
is octet-aligned. So add a separate fmtp string parser that returns that
information flexibly, as in
if (osmo_mgcp_fmtp_get_int("octet-aligned", 0) == 1) ...
Provide legacy shims that still act correctly for any callers that may
pass the old struct mgcp_codec_param. (I'm not sure if we need to keep
this, but we can always drop it in another patch.)
Adjust one mgcp_test.c: instead of returning only the octet-aligned
parameter, now osmo-mgw keeps and returns all the fmtp parameters that
the user provided. So add the missing "mode-change-capability".
Related: OS#6171
Change-Id: If58590bda8627519ff07e0b6f43aa47a274f052b
---
M include/osmocom/mgcp/Makefile.am
A include/osmocom/mgcp/fmtp.h
M include/osmocom/mgcp/mgcp_codec.h
M include/osmocom/mgcp/mgcp_common.h
M include/osmocom/mgcp/mgcp_network.h
M include/osmocom/mgcp_client/mgcp_client.h
M include/osmocom/mgcp_client/mgcp_client_fsm.h
M src/libosmo-mgcp-client/mgcp_client.c
M src/libosmo-mgcp/Makefile.am
A src/libosmo-mgcp/fmtp.c
M src/libosmo-mgcp/mgcp_codec.c
M src/libosmo-mgcp/mgcp_network.c
M src/libosmo-mgcp/mgcp_protocol.c
M src/libosmo-mgcp/mgcp_sdp.c
M tests/mgcp/mgcp_test.c
15 files changed, 270 insertions(+), 105 deletions(-)
git pull ssh://gerrit.osmocom.org:29418/osmo-mgw refs/changes/00/34900/5
--
To view, visit https://gerrit.osmocom.org/c/osmo-mgw/+/34900?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: If58590bda8627519ff07e0b6f43aa47a274f052b
Gerrit-Change-Number: 34900
Gerrit-PatchSet: 5
Gerrit-Owner: neels
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: laforge
Gerrit-CC: dexter
Gerrit-CC: pespin
Gerrit-Attention: laforge
Gerrit-Attention: pespin
Gerrit-MessageType: newpatchset
[L] Change in osmo-mgw[master]: add fmtp string to ptmap: allow all possible fmtp
Attention is currently required from: pespin. neels has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-mgw/+/34900?usp=email ) Change subject: add fmtp string to ptmap: allow all possible fmtp .. Patch Set 4: (2 comments) File include/osmocom/mgcp_client/mgcp_client.h: https://gerrit.osmocom.org/c/osmo-mgw/+/34900/comment/d9843f6b_d3a929cb PS4, Line 165: * Not all pointers contained in the mgcp_response */ > what do you mean with this? erm ... no i didn't write this. i started out with dexter's patch, maybe that's where it came from...? what is it trying to say, i guess mgcp_response allocation is not entirely self-contained. Looking, though, it actually does self-contain all its members, except for the 'sdp' pointer that includes the originally parsed SDP string. So seems to have become unrelated to this patch. File tests/mgcp/mgcp_test.c: https://gerrit.osmocom.org/c/osmo-mgw/+/34900/comment/3f2647a6_b3917892 PS4, Line 848: fflush(stderr); > is this really needed? […] another such something i didn't see, thx -- To view, visit https://gerrit.osmocom.org/c/osmo-mgw/+/34900?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: If58590bda8627519ff07e0b6f43aa47a274f052b Gerrit-Change-Number: 34900 Gerrit-PatchSet: 4 Gerrit-Owner: neels Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: laforge Gerrit-CC: dexter Gerrit-CC: pespin Gerrit-Attention: pespin Gerrit-Comment-Date: Wed, 01 Nov 2023 21:04:53 + Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: pespin Gerrit-MessageType: comment
[L] Change in osmo-mgw[master]: add fmtp string to ptmap: allow all possible fmtp
Attention is currently required from: neels. laforge has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-mgw/+/34900?usp=email ) Change subject: add fmtp string to ptmap: allow all possible fmtp .. Patch Set 4: Code-Review+1 -- To view, visit https://gerrit.osmocom.org/c/osmo-mgw/+/34900?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: If58590bda8627519ff07e0b6f43aa47a274f052b Gerrit-Change-Number: 34900 Gerrit-PatchSet: 4 Gerrit-Owner: neels Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: laforge Gerrit-CC: dexter Gerrit-CC: pespin Gerrit-Attention: neels Gerrit-Comment-Date: Mon, 30 Oct 2023 08:39:22 + Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment
[L] Change in osmo-mgw[master]: add fmtp string to ptmap: allow all possible fmtp
Attention is currently required from: neels. pespin has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-mgw/+/34900?usp=email ) Change subject: add fmtp string to ptmap: allow all possible fmtp .. Patch Set 4: (2 comments) File include/osmocom/mgcp_client/mgcp_client.h: https://gerrit.osmocom.org/c/osmo-mgw/+/34900/comment/8a74db89_39be3094 PS4, Line 165: * Not all pointers contained in the mgcp_response */ what do you mean with this? File tests/mgcp/mgcp_test.c: https://gerrit.osmocom.org/c/osmo-mgw/+/34900/comment/3ad1f3d1_95caf45c PS4, Line 848: fflush(stderr); is this really needed? Specially for stderr, it makes no sense imho, because stderr is not buffered. -- To view, visit https://gerrit.osmocom.org/c/osmo-mgw/+/34900?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: If58590bda8627519ff07e0b6f43aa47a274f052b Gerrit-Change-Number: 34900 Gerrit-PatchSet: 4 Gerrit-Owner: neels Gerrit-Reviewer: Jenkins Builder Gerrit-CC: dexter Gerrit-CC: pespin Gerrit-Attention: neels Gerrit-Comment-Date: Fri, 27 Oct 2023 12:35:31 + Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment
[L] Change in osmo-mgw[master]: add fmtp string to ptmap: allow all possible fmtp
neels has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-mgw/+/34900?usp=email ) Change subject: add fmtp string to ptmap: allow all possible fmtp .. Patch Set 4: (1 comment) File include/osmocom/mgcp_client/mgcp_client.h: https://gerrit.osmocom.org/c/osmo-mgw/+/34900/comment/c734d47d_1ee6f103 PS3, Line 84: const char *fmtp; > context: […] Done -- To view, visit https://gerrit.osmocom.org/c/osmo-mgw/+/34900?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: If58590bda8627519ff07e0b6f43aa47a274f052b Gerrit-Change-Number: 34900 Gerrit-PatchSet: 4 Gerrit-Owner: neels Gerrit-Reviewer: Jenkins Builder Gerrit-CC: dexter Gerrit-Comment-Date: Fri, 27 Oct 2023 00:09:36 + Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: neels Gerrit-MessageType: comment
[L] Change in osmo-mgw[master]: add fmtp string to ptmap: allow all possible fmtp
Hello Jenkins Builder,
I'd like you to reexamine a change. Please visit
https://gerrit.osmocom.org/c/osmo-mgw/+/34900?usp=email
to look at the new patch set (#4).
The following approvals got outdated and were removed:
Verified+1 by Jenkins Builder
Change subject: add fmtp string to ptmap: allow all possible fmtp
..
add fmtp string to ptmap: allow all possible fmtp
Remove the limit of having only one AMR octet-aligned fmtp parameter per
MGCP message. Instead allow any arbitrary fmtp options, one per every
codec.
Deprecate all use of struct mgcp_codec_param. Instead, store and pass
plain fmtp strings.
We need to know fmtp details only for AMR, and only to probe whether it
is octet-aligned. So add a separate fmtp string parser that returns that
information flexibly, as in
if (osmo_mgcp_fmtp_get_int("octet-aligned", 0) == 1) ...
Provide legacy shims that still act correctly for any callers that may
pass the old struct mgcp_codec_param. (I'm not sure if we need to keep
this, but we can always drop it in another patch.)
Adjust one mgcp_test.c: instead of returning only the octet-aligned
parameter, now osmo-mgw keeps and returns all the fmtp parameters that
the user provided. So add the missing "mode-change-capability".
Related: OS#6171
Change-Id: If58590bda8627519ff07e0b6f43aa47a274f052b
---
M include/osmocom/mgcp/Makefile.am
A include/osmocom/mgcp/fmtp.h
M include/osmocom/mgcp/mgcp_codec.h
M include/osmocom/mgcp/mgcp_common.h
M include/osmocom/mgcp/mgcp_network.h
M include/osmocom/mgcp_client/mgcp_client.h
M include/osmocom/mgcp_client/mgcp_client_fsm.h
M src/libosmo-mgcp-client/mgcp_client.c
M src/libosmo-mgcp/Makefile.am
A src/libosmo-mgcp/fmtp.c
M src/libosmo-mgcp/mgcp_codec.c
M src/libosmo-mgcp/mgcp_network.c
M src/libosmo-mgcp/mgcp_protocol.c
M src/libosmo-mgcp/mgcp_sdp.c
M tests/mgcp/mgcp_test.c
15 files changed, 273 insertions(+), 106 deletions(-)
git pull ssh://gerrit.osmocom.org:29418/osmo-mgw refs/changes/00/34900/4
--
To view, visit https://gerrit.osmocom.org/c/osmo-mgw/+/34900?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: If58590bda8627519ff07e0b6f43aa47a274f052b
Gerrit-Change-Number: 34900
Gerrit-PatchSet: 4
Gerrit-Owner: neels
Gerrit-Reviewer: Jenkins Builder
Gerrit-CC: dexter
Gerrit-MessageType: newpatchset
[L] Change in osmo-mgw[master]: add fmtp string to ptmap: allow all possible fmtp
neels has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-mgw/+/34900?usp=email ) Change subject: add fmtp string to ptmap: allow all possible fmtp .. Patch Set 3: (3 comments) File include/osmocom/mgcp/mgcp_network.h: https://gerrit.osmocom.org/c/osmo-mgw/+/34900/comment/26f6967e_d57f2eb6 PS3, Line 90: char fmtp[256]; > here is that char fmtp[256], it matches the storage choice of audio_name[] > above. context: this is used in public API, also as out-parameter. Dynamic allocation would be tricky, so leave it static. File include/osmocom/mgcp_client/mgcp_client.h: https://gerrit.osmocom.org/c/osmo-mgw/+/34900/comment/ae818ca6_8024f899 PS3, Line 84: const char *fmtp; > I'm a bit wobbly on the storage choice. Elsewhere I put a char fmtp[256]. […] context: structs with ptmap members are: - struct mgcp_conn_peer - struct mgcp_response If a fmtp is only passed in as function arg, it is fine to be a static pointer that goes bust after the function returns. For persistent storage, a pointer going bust would be bad. So are any of these structs persistent? mgcp_conn_peer is stored persistently in at least mgcp_client_endpoint_fsm.c. It copies a value passed in from a caller outside of this library. So indeed a static array would be safer / simpler here. File src/libosmo-mgcp/mgcp_sdp.c: https://gerrit.osmocom.org/c/osmo-mgw/+/34900/comment/8333a45b_d2dc732c PS3, Line 59: const char *fmtp; > here's another dynamic string... context: this struct is used only internally in this .c file. The storage is never persistent, always limited in a function scope. There already is a bunch of talloc allocation with a temporary ctx, so this is fine to remain a pointer (and not impose a length limit). -- To view, visit https://gerrit.osmocom.org/c/osmo-mgw/+/34900?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: If58590bda8627519ff07e0b6f43aa47a274f052b Gerrit-Change-Number: 34900 Gerrit-PatchSet: 3 Gerrit-Owner: neels Gerrit-Reviewer: Jenkins Builder Gerrit-CC: dexter Gerrit-Comment-Date: Thu, 26 Oct 2023 23:26:26 + Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: neels Gerrit-MessageType: comment
[L] Change in osmo-mgw[master]: add fmtp string to ptmap: allow all possible fmtp
neels has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-mgw/+/34900?usp=email ) Change subject: add fmtp string to ptmap: allow all possible fmtp .. Patch Set 3: (3 comments) File include/osmocom/mgcp/mgcp_network.h: https://gerrit.osmocom.org/c/osmo-mgw/+/34900/comment/7a20c06c_c53adb21 PS3, Line 90: char fmtp[256]; here is that char fmtp[256], it matches the storage choice of audio_name[] above. File include/osmocom/mgcp_client/mgcp_client.h: https://gerrit.osmocom.org/c/osmo-mgw/+/34900/comment/e714e348_5c276466 PS3, Line 84: const char *fmtp; I'm a bit wobbly on the storage choice. Elsewhere I put a char fmtp[256]. So maybe also an array here instead of a pointer? It would make those structs using this completely self-contained. Does 256 seem like a sane limit? dynamic string would have no limit... opinions welcome. File src/libosmo-mgcp/mgcp_sdp.c: https://gerrit.osmocom.org/c/osmo-mgw/+/34900/comment/b572064d_fb311e6a PS3, Line 59: const char *fmtp; here's another dynamic string... -- To view, visit https://gerrit.osmocom.org/c/osmo-mgw/+/34900?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: If58590bda8627519ff07e0b6f43aa47a274f052b Gerrit-Change-Number: 34900 Gerrit-PatchSet: 3 Gerrit-Owner: neels Gerrit-Reviewer: Jenkins Builder Gerrit-CC: dexter Gerrit-Comment-Date: Thu, 26 Oct 2023 23:06:38 + Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment
[L] Change in osmo-mgw[master]: add fmtp string to ptmap: allow all possible fmtp
neels has posted comments on this change. (
https://gerrit.osmocom.org/c/osmo-mgw/+/34900?usp=email )
Change subject: add fmtp string to ptmap: allow all possible fmtp
..
Patch Set 3:
(2 comments)
Commit Message:
https://gerrit.osmocom.org/c/osmo-mgw/+/34900/comment/aa1a455b_57348244
PS2, Line 20: if (osmo_mgcp_fmtp_get_int("octet-aligned", 0) == 1) ...
> I just noticed there is confusion in the code about the name of this
> attribute: […]
Done
Patchset:
PS1:
> ttcn3 tests still fail, @pma...@sysmocom.de would be great if you could take
> a look.. […]
Done
--
To view, visit https://gerrit.osmocom.org/c/osmo-mgw/+/34900?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: If58590bda8627519ff07e0b6f43aa47a274f052b
Gerrit-Change-Number: 34900
Gerrit-PatchSet: 3
Gerrit-Owner: neels
Gerrit-Reviewer: Jenkins Builder
Gerrit-CC: dexter
Gerrit-Comment-Date: Thu, 26 Oct 2023 22:56:27 +
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: neels
Gerrit-MessageType: comment
[L] Change in osmo-mgw[master]: add fmtp string to ptmap: allow all possible fmtp
neels has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-mgw/+/34900?usp=email ) Change subject: add fmtp string to ptmap: allow all possible fmtp .. Patch Set 3: (1 comment) This change is ready for review. Patchset: PS3: tests pass now -- To view, visit https://gerrit.osmocom.org/c/osmo-mgw/+/34900?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: If58590bda8627519ff07e0b6f43aa47a274f052b Gerrit-Change-Number: 34900 Gerrit-PatchSet: 3 Gerrit-Owner: neels Gerrit-Reviewer: Jenkins Builder Gerrit-CC: dexter Gerrit-Comment-Date: Thu, 26 Oct 2023 22:56:10 + Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment
[L] Change in osmo-mgw[master]: add fmtp string to ptmap: allow all possible fmtp
Jenkins Builder has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-mgw/+/34900?usp=email ) Change subject: add fmtp string to ptmap: allow all possible fmtp .. Patch Set 1: (1 comment) File include/osmocom/mgcp_client/mgcp_client.h: Robot Comment from checkpatch (run ID jenkins-gerrit-lint-12114): https://gerrit.osmocom.org/c/osmo-mgw/+/34900/comment/eb876c85_e3480476 PS1, Line 84:const char *fmtp; Statements should start on a tabstop -- To view, visit https://gerrit.osmocom.org/c/osmo-mgw/+/34900?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: If58590bda8627519ff07e0b6f43aa47a274f052b Gerrit-Change-Number: 34900 Gerrit-PatchSet: 1 Gerrit-Owner: neels Gerrit-CC: Jenkins Builder Gerrit-Comment-Date: Thu, 26 Oct 2023 01:50:25 + Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment
[L] Change in osmo-mgw[master]: add fmtp string to ptmap: allow all possible fmtp
neels has uploaded this change for review. (
https://gerrit.osmocom.org/c/osmo-mgw/+/34900?usp=email )
Change subject: add fmtp string to ptmap: allow all possible fmtp
..
add fmtp string to ptmap: allow all possible fmtp
Remove the limit of having only one AMR octet-aligned fmtp parameter per
MGCP message. Instead allow any arbitrary fmtp options, one per every
codec.
Deprecate all use of struct mgcp_codec_param. Instead, store and pass
plain fmtp strings.
We need to know fmtp details only for AMR, and only to probe whether it
is octet-aligned. So add a separate fmtp string parser that returns that
information flexibly, as in
if (osmo_mgcp_fmtp_get_int("octet-aligned", 0) == 1) ...
Provide legacy shims that still act correctly for any callers that may
pass the old struct mgcp_codec_param. (I'm not sure if we need to keep
this, but we can always drop it in another patch.)
Adjust one mgcp_test.c: instead of returning only the octet-aligned
parameter, now osmo-mgw keeps and returns all the fmtp parameters that
the user provided. So add the missing "mode-change-capability".
Related: OS#6171
Change-Id: If58590bda8627519ff07e0b6f43aa47a274f052b
---
M include/osmocom/mgcp/Makefile.am
A include/osmocom/mgcp/fmtp.h
M include/osmocom/mgcp/mgcp_codec.h
M include/osmocom/mgcp/mgcp_network.h
M include/osmocom/mgcp_client/mgcp_client.h
M src/libosmo-mgcp-client/mgcp_client.c
M src/libosmo-mgcp/Makefile.am
A src/libosmo-mgcp/fmtp.c
M src/libosmo-mgcp/mgcp_codec.c
M src/libosmo-mgcp/mgcp_network.c
M src/libosmo-mgcp/mgcp_protocol.c
M src/libosmo-mgcp/mgcp_sdp.c
M tests/mgcp/mgcp_test.c
13 files changed, 264 insertions(+), 93 deletions(-)
git pull ssh://gerrit.osmocom.org:29418/osmo-mgw refs/changes/00/34900/1
diff --git a/include/osmocom/mgcp/Makefile.am b/include/osmocom/mgcp/Makefile.am
index 15ff01a..5d77c09 100644
--- a/include/osmocom/mgcp/Makefile.am
+++ b/include/osmocom/mgcp/Makefile.am
@@ -13,4 +13,5 @@
mgcp_network.h \
mgcp_protocol.h \
mgcp_iuup.h \
+ fmtp.h \
$(NULL)
diff --git a/include/osmocom/mgcp/fmtp.h b/include/osmocom/mgcp/fmtp.h
new file mode 100644
index 000..95a7504
--- /dev/null
+++ b/include/osmocom/mgcp/fmtp.h
@@ -0,0 +1,11 @@
+#pragma once
+
+#include
+#include
+
+int osmo_mgcp_fmtp_get_val(char *val, size_t val_size, const char *fmtp, const
char *option_name);
+int osmo_mgcp_fmtp_get_int(const char *fmtp, const char *option_name, int
default_value);
+bool osmo_mgcp_fmtp_is_octet_aligned(const char *fmtp);
+
+#define OSMO_MGCP_OCTET_ALIGNED_STR "octet-aligned"
+#define OSMO_MGCP_OCTET_ALIGNED_EQUALS_STR(VAL) OSMO_MGCP_OCTET_ALIGNED_STR
"=" VAL
diff --git a/include/osmocom/mgcp/mgcp_codec.h
b/include/osmocom/mgcp/mgcp_codec.h
index cfc8ecf..4702f6d 100644
--- a/include/osmocom/mgcp/mgcp_codec.h
+++ b/include/osmocom/mgcp/mgcp_codec.h
@@ -13,6 +13,7 @@
void mgcp_codec_summary(struct mgcp_conn_rtp *conn);
void mgcp_codec_reset_all(struct mgcp_conn_rtp *conn);
int mgcp_codec_add(struct mgcp_conn_rtp *conn, int payload_type, const char
*audio_name, const struct mgcp_codec_param *param);
+int mgcp_codec_add2(struct mgcp_conn_rtp *conn, int payload_type, const char
*audio_name, const char *fmtp);
int mgcp_codec_decide(struct mgcp_conn_rtp *conn_src, struct mgcp_conn_rtp
*conn_dst);
const struct mgcp_rtp_codec *mgcp_codec_pt_find_by_subtype_name(struct
mgcp_conn_rtp *conn,
const char
*subtype_name, unsigned int match_nr);
diff --git a/include/osmocom/mgcp/mgcp_network.h
b/include/osmocom/mgcp/mgcp_network.h
index e95907d..7f87224 100644
--- a/include/osmocom/mgcp/mgcp_network.h
+++ b/include/osmocom/mgcp/mgcp_network.h
@@ -83,8 +83,11 @@
char audio_name[64];
char subtype_name[64];
+ /* Deprecated. Use the new fmtp string instead. */
bool param_present;
struct mgcp_codec_param param;
+
+ char fmtp[256];
};
/* 'mgcp_rtp_end': basically a wrapper around the RTP+RTCP ports */
diff --git a/include/osmocom/mgcp_client/mgcp_client.h
b/include/osmocom/mgcp_client/mgcp_client.h
index 31f4ae7..c238dc2 100644
--- a/include/osmocom/mgcp_client/mgcp_client.h
+++ b/include/osmocom/mgcp_client/mgcp_client.h
@@ -77,6 +77,11 @@
/*! payload type number (96-127) */
unsigned int pt;
+
+ /*! the MGCP 'a=fmtp:N <...>' string, e.g.
"mode-set=1,2,3;octet-align=0".
+* Suggested storage: talloc allocated string with the struct
mgcp_conn_peer or struct mgcp_response as talloc
+* ctx. */
+const char *fmtp;
};
struct mgcp_response_head {
@@ -158,7 +163,8 @@
uint8_t rate, uint8_t offset);
/* Invoked when an MGCP response is received or sending failed. When the
- * response is passed as NULL, this indicates failure during transmission. */
+ * response is passed as NULL, this indicates
[L] Change in osmo-mgw[master]: add fmtp string to ptmap: allow all possible fmtp
neels has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-mgw/+/34900?usp=email ) Change subject: add fmtp string to ptmap: allow all possible fmtp .. Patch Set 1: (1 comment) This change is ready for review. Patchset: PS1: ttcn3 tests still fail, @pma...@sysmocom.de would be great if you could take a look..? -- To view, visit https://gerrit.osmocom.org/c/osmo-mgw/+/34900?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: If58590bda8627519ff07e0b6f43aa47a274f052b Gerrit-Change-Number: 34900 Gerrit-PatchSet: 1 Gerrit-Owner: neels Gerrit-Reviewer: Jenkins Builder Gerrit-CC: dexter Gerrit-Comment-Date: Thu, 26 Oct 2023 01:56:41 + Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment
[L] Change in osmo-mgw[master]: add fmtp string to ptmap: allow all possible fmtp
neels has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-mgw/+/34900?usp=email ) Change subject: add fmtp string to ptmap: allow all possible fmtp .. Patch Set 1: (1 comment) Patchset: PS1: context: this patch replaces https://gerrit.osmocom.org/c/osmo-mgw/+/34351 -- To view, visit https://gerrit.osmocom.org/c/osmo-mgw/+/34900?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: If58590bda8627519ff07e0b6f43aa47a274f052b Gerrit-Change-Number: 34900 Gerrit-PatchSet: 1 Gerrit-Owner: neels Gerrit-Reviewer: Jenkins Builder Gerrit-Comment-Date: Thu, 26 Oct 2023 01:54:33 + Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment
