[M] Change in osmo-mgw[master]: client: collapse codecs[] and ptmap[]; allow codec variants
Attention is currently required from: fixeria. neels has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-mgw/+/34899?usp=email ) Change subject: client: collapse codecs[] and ptmap[]; allow codec variants .. Patch Set 5: (1 comment) File src/libosmo-mgcp-client/mgcp_client.c: https://gerrit.osmocom.org/c/osmo-mgw/+/34899/comment/f501d984_9c8757f2 PS5, Line 421: <= > Ah of course, it is the mgcp *client* library, which we use in osmo-bsc, > -msg, -hnbgw, so I need to […] True, but also the C tests should catch this... And I in turn realized now that this bug is only in the obscure failure case where there are surplus rtpmap entries in the MGCP, which were not listed in the m=audio... list. So it is this patch's responsibility to add such a test. -- To view, visit https://gerrit.osmocom.org/c/osmo-mgw/+/34899?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: I798e02c6663376d3d52f4a74fc4b32411ce95bed Gerrit-Change-Number: 34899 Gerrit-PatchSet: 5 Gerrit-Owner: neels Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: laforge Gerrit-Reviewer: neels Gerrit-Reviewer: pespin Gerrit-CC: fixeria Gerrit-Attention: fixeria Gerrit-Comment-Date: Wed, 01 Nov 2023 00:04:57 + Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: neels Comment-In-Reply-To: fixeria Gerrit-MessageType: comment
[M] Change in osmo-mgw[master]: client: collapse codecs[] and ptmap[]; allow codec variants
Attention is currently required from: fixeria. neels has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-mgw/+/34899?usp=email ) Change subject: client: collapse codecs[] and ptmap[]; allow codec variants .. Patch Set 5: (1 comment) File src/libosmo-mgcp-client/mgcp_client.c: https://gerrit.osmocom.org/c/osmo-mgw/+/34899/comment/0ae25fd3_33ed68ea PS5, Line 421: <= > oh my goodness, thx. […] Ah of course, it is the mgcp *client* library, which we use in osmo-bsc, -msg, -hnbgw, so I need to run *their* test suites to check this code. -- To view, visit https://gerrit.osmocom.org/c/osmo-mgw/+/34899?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: I798e02c6663376d3d52f4a74fc4b32411ce95bed Gerrit-Change-Number: 34899 Gerrit-PatchSet: 5 Gerrit-Owner: neels Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: laforge Gerrit-Reviewer: neels Gerrit-Reviewer: pespin Gerrit-CC: fixeria Gerrit-Attention: fixeria Gerrit-Comment-Date: Tue, 31 Oct 2023 23:52:31 + Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: neels Comment-In-Reply-To: fixeria Gerrit-MessageType: comment
[M] Change in osmo-mgw[master]: client: collapse codecs[] and ptmap[]; allow codec variants
Attention is currently required from: fixeria. neels has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-mgw/+/34899?usp=email ) Change subject: client: collapse codecs[] and ptmap[]; allow codec variants .. Patch Set 5: (1 comment) File src/libosmo-mgcp-client/mgcp_client.c: https://gerrit.osmocom.org/c/osmo-mgw/+/34899/comment/b88ccba1_6fa16c1b PS5, Line 421: <= > shouldn't this be `>=`? oh my goodness, thx. wait a minute, what does that say about our test suite. I did run these patches by ttcn3-hnbgw-tests! This code path should *always* fail, so how did I not catch it. Is this dead code? Do we not have a=rtpmap in our tests? i don't think that can be true. -- To view, visit https://gerrit.osmocom.org/c/osmo-mgw/+/34899?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: I798e02c6663376d3d52f4a74fc4b32411ce95bed Gerrit-Change-Number: 34899 Gerrit-PatchSet: 5 Gerrit-Owner: neels Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: laforge Gerrit-Reviewer: neels Gerrit-Reviewer: pespin Gerrit-CC: fixeria Gerrit-Attention: fixeria Gerrit-Comment-Date: Tue, 31 Oct 2023 23:51:03 + Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: fixeria Gerrit-MessageType: comment
[M] Change in osmo-mgw[master]: client: collapse codecs[] and ptmap[]; allow codec variants
neels has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-mgw/+/34899?usp=email ) Change subject: client: collapse codecs[] and ptmap[]; allow codec variants .. Patch Set 5: Code-Review-1 (1 comment) Patchset: PS5: I just realized an API compat problem, for example osmo-hnbgw uses the codecs[] array and fails to build with this patch. Any ideas? -- To view, visit https://gerrit.osmocom.org/c/osmo-mgw/+/34899?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: I798e02c6663376d3d52f4a74fc4b32411ce95bed Gerrit-Change-Number: 34899 Gerrit-PatchSet: 5 Gerrit-Owner: neels Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: laforge Gerrit-Reviewer: neels Gerrit-Reviewer: pespin Gerrit-CC: fixeria Gerrit-Comment-Date: Tue, 31 Oct 2023 23:40:49 + Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Gerrit-MessageType: comment
[M] Change in osmo-mgw[master]: client: collapse codecs[] and ptmap[]; allow codec variants
Attention is currently required from: neels. laforge has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-mgw/+/34899?usp=email ) Change subject: client: collapse codecs[] and ptmap[]; allow codec variants .. Patch Set 5: Code-Review+1 -- To view, visit https://gerrit.osmocom.org/c/osmo-mgw/+/34899?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: I798e02c6663376d3d52f4a74fc4b32411ce95bed Gerrit-Change-Number: 34899 Gerrit-PatchSet: 5 Gerrit-Owner: neels Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: laforge Gerrit-Reviewer: pespin Gerrit-CC: fixeria Gerrit-Attention: neels Gerrit-Comment-Date: Mon, 30 Oct 2023 08:38:51 + Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment
[M] Change in osmo-mgw[master]: client: collapse codecs[] and ptmap[]; allow codec variants
Attention is currently required from: neels. fixeria has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-mgw/+/34899?usp=email ) Change subject: client: collapse codecs[] and ptmap[]; allow codec variants .. Patch Set 5: (1 comment) File src/libosmo-mgcp-client/mgcp_client.c: https://gerrit.osmocom.org/c/osmo-mgw/+/34899/comment/475d29d4_224fce15 PS5, Line 421: <= shouldn't this be `>=`? -- To view, visit https://gerrit.osmocom.org/c/osmo-mgw/+/34899?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: I798e02c6663376d3d52f4a74fc4b32411ce95bed Gerrit-Change-Number: 34899 Gerrit-PatchSet: 5 Gerrit-Owner: neels Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: pespin Gerrit-CC: fixeria Gerrit-Attention: neels Gerrit-Comment-Date: Fri, 27 Oct 2023 15:39:18 + Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment
[M] Change in osmo-mgw[master]: client: collapse codecs[] and ptmap[]; allow codec variants
Attention is currently required from: neels. pespin has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-mgw/+/34899?usp=email ) Change subject: client: collapse codecs[] and ptmap[]; allow codec variants .. Patch Set 5: Code-Review+1 -- To view, visit https://gerrit.osmocom.org/c/osmo-mgw/+/34899?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: I798e02c6663376d3d52f4a74fc4b32411ce95bed Gerrit-Change-Number: 34899 Gerrit-PatchSet: 5 Gerrit-Owner: neels Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: pespin Gerrit-Attention: neels Gerrit-Comment-Date: Fri, 27 Oct 2023 12:20:56 + Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment
[M] Change in osmo-mgw[master]: client: collapse codecs[] and ptmap[]; allow codec variants
Attention is currently required from: pespin. neels has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-mgw/+/34899?usp=email ) Change subject: client: collapse codecs[] and ptmap[]; allow codec variants .. Patch Set 5: (1 comment) File src/libosmo-mgcp-client/mgcp_client.c: https://gerrit.osmocom.org/c/osmo-mgw/+/34899/comment/8c6e9167_3d6fb608 PS4, Line 398: if (r->ptmap_len >= ARRAY_SIZE(r->ptmap)) { > this check needs to be dropped, because we're just adding info to an existing > item (normally) Done -- To view, visit https://gerrit.osmocom.org/c/osmo-mgw/+/34899?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: I798e02c6663376d3d52f4a74fc4b32411ce95bed Gerrit-Change-Number: 34899 Gerrit-PatchSet: 5 Gerrit-Owner: neels Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: pespin Gerrit-Attention: pespin Gerrit-Comment-Date: Fri, 27 Oct 2023 00:08:16 + Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: neels Gerrit-MessageType: comment
[M] Change in osmo-mgw[master]: client: collapse codecs[] and ptmap[]; allow codec variants
Attention is currently required from: pespin. Hello Jenkins Builder, pespin, I'd like you to reexamine a change. Please visit https://gerrit.osmocom.org/c/osmo-mgw/+/34899?usp=email to look at the new patch set (#5). The following approvals got outdated and were removed: Verified+1 by Jenkins Builder Change subject: client: collapse codecs[] and ptmap[]; allow codec variants .. client: collapse codecs[] and ptmap[]; allow codec variants codecs[] is an array of enum osmo_mgcp_codecs. ptmap[] is an array of { enum osmo_mgcp_codecs, unsigned int ptmap }. MGCP lists first a bunch of payload type numbers and then specifies them again for details, like the numbers 112, 96, 3 in this example: m=audio RTP/AVP 112 96 3 a=rtpmap:112 AMR/8000 a=rtpmap:96 VND.3GPP.IUFP/16000 a=rtpmap:3 GSM-FR/8000 So far we keep these lists in two separate arrays, in both struct mgcp_response and struct mgcp_msg: - codecs[], codecs_len stores the 'm=audio' list - ptmap[], ptmap_len stores the 'a=rtpmap' list These are semantically identical, and the separation means we cannot have the same codec twice, like AMR with different fmtp variations. It also leads to checks, conversions and dear hopes of correct ordering. So let's keep only one list with both codec and payload type number in it. The 'm=audio' list establishes the order of the pt numbers, and the 'a=rtpmap' list adds codec information to the established entries. Related: OS#6171 Change-Id: I798e02c6663376d3d52f4a74fc4b32411ce95bed --- 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-client/mgcp_client_fsm.c M tests/mgcp_client/mgcp_client_test.c 5 files changed, 107 insertions(+), 68 deletions(-) git pull ssh://gerrit.osmocom.org:29418/osmo-mgw refs/changes/99/34899/5 -- To view, visit https://gerrit.osmocom.org/c/osmo-mgw/+/34899?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: I798e02c6663376d3d52f4a74fc4b32411ce95bed Gerrit-Change-Number: 34899 Gerrit-PatchSet: 5 Gerrit-Owner: neels Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: pespin Gerrit-Attention: pespin Gerrit-MessageType: newpatchset
[M] Change in osmo-mgw[master]: client: collapse codecs[] and ptmap[]; allow codec variants
Attention is currently required from: pespin. neels has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-mgw/+/34899?usp=email ) Change subject: client: collapse codecs[] and ptmap[]; allow codec variants .. Patch Set 4: (1 comment) File src/libosmo-mgcp-client/mgcp_client.c: https://gerrit.osmocom.org/c/osmo-mgw/+/34899/comment/3a105493_58c6be0c PS4, Line 398: if (r->ptmap_len >= ARRAY_SIZE(r->ptmap)) { this check needs to be dropped, because we're just adding info to an existing item (normally) -- To view, visit https://gerrit.osmocom.org/c/osmo-mgw/+/34899?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: I798e02c6663376d3d52f4a74fc4b32411ce95bed Gerrit-Change-Number: 34899 Gerrit-PatchSet: 4 Gerrit-Owner: neels Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: pespin Gerrit-Attention: pespin Gerrit-Comment-Date: Thu, 26 Oct 2023 23:58:12 + Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment
[M] Change in osmo-mgw[master]: client: collapse codecs[] and ptmap[]; allow codec variants
Attention is currently required from: neels, pespin. Hello Jenkins Builder, pespin, I'd like you to reexamine a change. Please visit https://gerrit.osmocom.org/c/osmo-mgw/+/34899?usp=email to look at the new patch set (#4). The following approvals got outdated and were removed: Code-Review+1 by pespin, Verified+1 by Jenkins Builder Change subject: client: collapse codecs[] and ptmap[]; allow codec variants .. client: collapse codecs[] and ptmap[]; allow codec variants codecs[] is an array of enum osmo_mgcp_codecs. ptmap[] is an array of { enum osmo_mgcp_codecs, unsigned int ptmap }. MGCP lists first a bunch of payload type numbers and then specifies them again for details, like the numbers 112, 96, 3 in this example: m=audio RTP/AVP 112 96 3 a=rtpmap:112 AMR/8000 a=rtpmap:96 VND.3GPP.IUFP/16000 a=rtpmap:3 GSM-FR/8000 So far we keep these lists in two separate arrays, in both struct mgcp_response and struct mgcp_msg: - codecs[], codecs_len stores the 'm=audio' list - ptmap[], ptmap_len stores the 'a=rtpmap' list These are semantically identical, and the separation means we cannot have the same codec twice, like AMR with different fmtp variations. It also leads to checks, conversions and dear hopes of correct ordering. So let's keep only one list with both codec and payload type number in it. The 'm=audio' list establishes the order of the pt numbers, and the 'a=rtpmap' list adds codec information to the established entries. Related: OS#6171 Change-Id: I798e02c6663376d3d52f4a74fc4b32411ce95bed --- 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-client/mgcp_client_fsm.c M tests/mgcp_client/mgcp_client_test.c 5 files changed, 109 insertions(+), 63 deletions(-) git pull ssh://gerrit.osmocom.org:29418/osmo-mgw refs/changes/99/34899/4 -- To view, visit https://gerrit.osmocom.org/c/osmo-mgw/+/34899?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: I798e02c6663376d3d52f4a74fc4b32411ce95bed Gerrit-Change-Number: 34899 Gerrit-PatchSet: 4 Gerrit-Owner: neels Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: pespin Gerrit-Attention: neels Gerrit-Attention: pespin Gerrit-MessageType: newpatchset
[M] Change in osmo-mgw[master]: client: collapse codecs[] and ptmap[]; allow codec variants
Attention is currently required from: neels. pespin has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-mgw/+/34899?usp=email ) Change subject: client: collapse codecs[] and ptmap[]; allow codec variants .. Patch Set 3: Code-Review+1 -- To view, visit https://gerrit.osmocom.org/c/osmo-mgw/+/34899?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: I798e02c6663376d3d52f4a74fc4b32411ce95bed Gerrit-Change-Number: 34899 Gerrit-PatchSet: 3 Gerrit-Owner: neels Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: pespin Gerrit-Attention: neels Gerrit-Comment-Date: Thu, 26 Oct 2023 12:25:01 + Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment
[M] Change in osmo-mgw[master]: client: collapse codecs[] and ptmap[]; allow codec variants
Hello Jenkins Builder, I'd like you to reexamine a change. Please visit https://gerrit.osmocom.org/c/osmo-mgw/+/34899?usp=email to look at the new patch set (#3). Change subject: client: collapse codecs[] and ptmap[]; allow codec variants .. client: collapse codecs[] and ptmap[]; allow codec variants codecs[] is an array of enum osmo_mgcp_codecs. ptmap[] is an array of { enum osmo_mgcp_codecs, unsigned int ptmap }. MGCP lists first a bunch of payload type numbers and then specifies them again for details, like the numbers 112, 96, 3 in this example: m=audio RTP/AVP 112 96 3 a=rtpmap:112 AMR/8000 a=rtpmap:96 VND.3GPP.IUFP/16000 a=rtpmap:3 GSM-FR/8000 So far we keep these lists in two separate arrays, in both struct mgcp_response and struct mgcp_msg: - codecs[], codecs_len stores the 'm=audio' list - ptmap[], ptmap_len stores the 'a=rtpmap' list These are semantically identical, and the separation means we cannot have the same codec twice, like AMR with different fmtp variations. It also leads to checks, conversions and dear hopes of correct ordering. So let's keep only one list with both codec and payload type number in it. The 'm=audio' list establishes the order of the pt numbers, and the 'a=rtpmap' list adds codec information to the established entries. Related: OS#6171 Change-Id: I798e02c6663376d3d52f4a74fc4b32411ce95bed --- 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-client/mgcp_client_fsm.c M tests/mgcp_client/mgcp_client_test.c 5 files changed, 109 insertions(+), 63 deletions(-) git pull ssh://gerrit.osmocom.org:29418/osmo-mgw refs/changes/99/34899/3 -- To view, visit https://gerrit.osmocom.org/c/osmo-mgw/+/34899?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: I798e02c6663376d3d52f4a74fc4b32411ce95bed Gerrit-Change-Number: 34899 Gerrit-PatchSet: 3 Gerrit-Owner: neels Gerrit-Reviewer: Jenkins Builder Gerrit-MessageType: newpatchset
[M] Change in osmo-mgw[master]: client: collapse codecs[] and ptmap[]; allow codec variants
Hello Jenkins Builder, I'd like you to reexamine a change. Please visit https://gerrit.osmocom.org/c/osmo-mgw/+/34899?usp=email to look at the new patch set (#2). Change subject: client: collapse codecs[] and ptmap[]; allow codec variants .. client: collapse codecs[] and ptmap[]; allow codec variants codecs[] is an array of enum osmo_mgcp_codecs. ptmap[] is an array of { enum osmo_mgcp_codecs, unsigned int ptmap }. MGCP lists first a bunch of payload type numbers and then specifies them again for details, like the numbers 112, 96, 3 in this example: m=audio RTP/AVP 112 96 3 a=rtpmap:112 AMR/8000 a=rtpmap:96 VND.3GPP.IUFP/16000 a=rtpmap:3 GSM-FR/8000 So far we keep these lists in two separate arrays, in both struct mgcp_response and struct mgcp_msg: - codecs[], codecs_len stores the 'm=audio' list - ptmap[], ptmap_len stores the 'a=rtpmap' list These are semantically identical, and the separation means we cannot have the same codec twice, like AMR with different fmtp variations. It also leads to checks, conversions and dear hopes of correct ordering. So let's keep only one list with both codec and payload type number in it. The 'm=audio' list establishes the order of the pt numbers, and the 'a=rtpmap' list adds codec information to the established entries. Change-Id: I798e02c6663376d3d52f4a74fc4b32411ce95bed --- 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-client/mgcp_client_fsm.c M tests/mgcp_client/mgcp_client_test.c 5 files changed, 108 insertions(+), 63 deletions(-) git pull ssh://gerrit.osmocom.org:29418/osmo-mgw refs/changes/99/34899/2 -- To view, visit https://gerrit.osmocom.org/c/osmo-mgw/+/34899?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: I798e02c6663376d3d52f4a74fc4b32411ce95bed Gerrit-Change-Number: 34899 Gerrit-PatchSet: 2 Gerrit-Owner: neels Gerrit-Reviewer: Jenkins Builder Gerrit-MessageType: newpatchset