March 2023 - gerrit-log - lists.osmocom.org

[M] Change in osmo-mgw[master]: mgcp_codec: fix oa/bwe comparison in mgcp_codec_pt_translate()
by dexter 22 Mar '23

22 Mar '23

Attention is currently required from: pespin. dexter has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-mgw/+/27392 ) Change subject: mgcp_codec: fix oa/bwe comparison in mgcp_codec_pt_translate() ...................................................................... Patch Set 2: (2 comments) File src/libosmo-mgcp/mgcp_codec.c: https://gerrit.osmocom.org/c/osmo-mgw/+/27392/comment/ba7aaa45_62231ef0 PS1, Line 369: static bool codecs_same(struct mgcp_rtp_codec *codec_a, struct mgcp_rtp_codec *codec_b, bool match_pload_fmt) > I'd make this more generic: bool allow_transcoding […] I don't think that this is good. I would find this parameter naming very confusing. Also its not really about transcoding (one codec into another). Its more about format conversion. https://gerrit.osmocom.org/c/osmo-mgw/+/27392/comment/2607880d_9d21d980 PS1, Line 431: /* In case we weren't able to find an exact match, we will try to find a match that is the same codec, but the > Make all this generic. […] I do not see where this is not generic. This would work with any codec that supports different payload formats. (codecs_same would need an upgrade though, but that is a different story) -- To view, visit https://gerrit.osmocom.org/c/osmo-mgw/+/27392 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-mgw Gerrit-Branch: master Gerrit-Change-Id: Ifbd201a2749009a4644a29bd77e1d0fc0c124a9d Gerrit-Change-Number: 27392 Gerrit-PatchSet: 2 Gerrit-Owner: dexter <pmaier(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: msuraev <msuraev(a)sysmocom.de> Gerrit-CC: laforge <laforge(a)osmocom.org> Gerrit-CC: pespin <pespin(a)sysmocom.de> Gerrit-Attention: pespin <pespin(a)sysmocom.de> Gerrit-Comment-Date: Wed, 22 Mar 2023 16:06:21 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: pespin <pespin(a)sysmocom.de> Gerrit-MessageType: comment

1 0

[M] Change in osmo-mgw[master]: mgcp_codec: fix oa/bwe comparison in mgcp_codec_pt_translate()
by dexter 22 Mar '23

22 Mar '23

Attention is currently required from: pespin. Hello Jenkins Builder, msuraev, I'd like you to reexamine a change. Please visit https://gerrit.osmocom.org/c/osmo-mgw/+/27392 to look at the new patch set (#2). Change subject: mgcp_codec: fix oa/bwe comparison in mgcp_codec_pt_translate() ...................................................................... mgcp_codec: fix oa/bwe comparison in mgcp_codec_pt_translate() The function mgcp_codec_pt_translate is very strict when comparing the codecs to each other to find a matching payload type number to be used on the egress side. This poses a problem when one side uses AMR in bandwith-efficient, while the other side uses AMR in octet-aligned payload type format. To the pt translate function the difference in the payload format will appear as if the codec were different and eventually the payload type number cannot be translated. since osmo-mgw offers conversion between the payload type format it would be no problem to ignore the payload type format when making the translation decision. The only exception here would be if one side would announce AMR two times, the first time with octet-aligned and the second time with bandwith-efficient format. Then we would have to use the payload type number from the exact match. (and skip any formatconversion) To archive such an optimized decision we will first go through the codec lists and perform an exact match. If we don't get a match we go through the codec lists a second time, but this time we ignore the payload format. Change-Id: Ifbd201a2749009a4644a29bd77e1d0fc0c124a9d Related: OS#5461 --- M src/libosmo-mgcp/mgcp_codec.c M tests/mgcp/mgcp_test.c M tests/mgcp/mgcp_test.ok 3 files changed, 92 insertions(+), 14 deletions(-) git pull ssh://gerrit.osmocom.org:29418/osmo-mgw refs/changes/92/27392/2 -- To view, visit https://gerrit.osmocom.org/c/osmo-mgw/+/27392 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-mgw Gerrit-Branch: master Gerrit-Change-Id: Ifbd201a2749009a4644a29bd77e1d0fc0c124a9d Gerrit-Change-Number: 27392 Gerrit-PatchSet: 2 Gerrit-Owner: dexter <pmaier(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: msuraev <msuraev(a)sysmocom.de> Gerrit-CC: laforge <laforge(a)osmocom.org> Gerrit-CC: pespin <pespin(a)sysmocom.de> Gerrit-Attention: pespin <pespin(a)sysmocom.de> Gerrit-MessageType: newpatchset

1 0

[S] Change in osmo-pcu[master]: pdch_ul_controller: log reserved frame numbers
by pespin 22 Mar '23

22 Mar '23

Attention is currently required from: dexter. pespin has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-pcu/+/31022 ) Change subject: pdch_ul_controller: log reserved frame numbers ...................................................................... Patch Set 2: (1 comment) File src/pdch_ul_controller.c: https://gerrit.osmocom.org/c/osmo-pcu/+/31022/comment/fbfd5628_a93df5ee PS1, Line 196: LOGPDCH(ulc->pdch, DRLCMAC, LOGL_DEBUG, "reserving FN %u\n", item->fn); > I mean: The pointer it gets assigned in line 202 […] I meant item->type, not it->type, same as you are already using to log the FN ;) -- To view, visit https://gerrit.osmocom.org/c/osmo-pcu/+/31022 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-pcu Gerrit-Branch: master Gerrit-Change-Id: I4277c572a4cc6cbbf3ac4e67442c9036be687627 Gerrit-Change-Number: 31022 Gerrit-PatchSet: 2 Gerrit-Owner: dexter <pmaier(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-CC: laforge <laforge(a)osmocom.org> Gerrit-CC: pespin <pespin(a)sysmocom.de> Gerrit-Attention: dexter <pmaier(a)sysmocom.de> Gerrit-Comment-Date: Wed, 22 Mar 2023 14:50:30 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: pespin <pespin(a)sysmocom.de> Comment-In-Reply-To: dexter <pmaier(a)sysmocom.de> Gerrit-MessageType: comment

1 0

[S] Change in osmo-pcu[master]: pdch_ul_controller: log reserved frame numbers
by dexter 22 Mar '23

22 Mar '23

Attention is currently required from: pespin. dexter has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-pcu/+/31022 ) Change subject: pdch_ul_controller: log reserved frame numbers ...................................................................... Patch Set 2: (1 comment) File src/pdch_ul_controller.c: https://gerrit.osmocom.org/c/osmo-pcu/+/31022/comment/726e8d51_d63cb392 PS1, Line 196: LOGPDCH(ulc->pdch, DRLCMAC, LOGL_DEBUG, "reserving FN %u\n", item->fn); > what are you talking about? it->ype is not populated in any way in this while loop? I mean: The pointer it gets assigned in line 202 it = container_of(*n, struct pdch_ulc_node, node); This happens in the while loop below but the log line is before the while loop. -- To view, visit https://gerrit.osmocom.org/c/osmo-pcu/+/31022 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-pcu Gerrit-Branch: master Gerrit-Change-Id: I4277c572a4cc6cbbf3ac4e67442c9036be687627 Gerrit-Change-Number: 31022 Gerrit-PatchSet: 2 Gerrit-Owner: dexter <pmaier(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-CC: laforge <laforge(a)osmocom.org> Gerrit-CC: pespin <pespin(a)sysmocom.de> Gerrit-Attention: pespin <pespin(a)sysmocom.de> Gerrit-Comment-Date: Wed, 22 Mar 2023 14:45:05 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: pespin <pespin(a)sysmocom.de> Comment-In-Reply-To: dexter <pmaier(a)sysmocom.de> Gerrit-MessageType: comment

1 0

[S] Change in pysim[master]: pySim-shell: add cardinfo command
by dexter 22 Mar '23

22 Mar '23

Attention is currently required from: laforge. dexter has posted comments on this change. ( https://gerrit.osmocom.org/c/pysim/+/25805 ) Change subject: pySim-shell: add cardinfo command ...................................................................... Patch Set 4: (1 comment) File pySim-shell.py: https://gerrit.osmocom.org/c/pysim/+/25805/comment/5e353f95_935542c7 PS2, Line 611: p > ping? Reading the ATR would require a card reset. This is probably something that should be avoided since we would then reset the entire card state. -- To view, visit https://gerrit.osmocom.org/c/pysim/+/25805 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: pysim Gerrit-Branch: master Gerrit-Change-Id: If31ed17102dc0108e27a5eb0344aabaaf19b19f9 Gerrit-Change-Number: 25805 Gerrit-PatchSet: 4 Gerrit-Owner: dexter <pmaier(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: laforge <laforge(a)osmocom.org> Gerrit-Reviewer: osmith <osmith(a)sysmocom.de> Gerrit-Reviewer: pespin <pespin(a)sysmocom.de> Gerrit-Attention: laforge <laforge(a)osmocom.org> Gerrit-Comment-Date: Wed, 22 Mar 2023 14:40:01 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: laforge <laforge(a)osmocom.org> Gerrit-MessageType: comment

1 0

[S] Change in pysim[master]: pySim-shell: add cardinfo command
by dexter 22 Mar '23

22 Mar '23

Attention is currently required from: laforge. Hello osmith, Jenkins Builder, laforge, pespin, I'd like you to reexamine a change. Please visit https://gerrit.osmocom.org/c/pysim/+/25805 to look at the new patch set (#4). Change subject: pySim-shell: add cardinfo command ...................................................................... pySim-shell: add cardinfo command It may sometimes be helpful to get a bit of general information about the card. To sort out problems it sometimes helps to get an idea what card type and ICCID pySim-shell has in memory. Change-Id: If31ed17102dc0108e27a5eb0344aabaaf19b19f9 --- M pySim-shell.py 1 file changed, 23 insertions(+), 0 deletions(-) git pull ssh://gerrit.osmocom.org:29418/pysim refs/changes/05/25805/4 -- To view, visit https://gerrit.osmocom.org/c/pysim/+/25805 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: pysim Gerrit-Branch: master Gerrit-Change-Id: If31ed17102dc0108e27a5eb0344aabaaf19b19f9 Gerrit-Change-Number: 25805 Gerrit-PatchSet: 4 Gerrit-Owner: dexter <pmaier(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: laforge <laforge(a)osmocom.org> Gerrit-Reviewer: osmith <osmith(a)sysmocom.de> Gerrit-Reviewer: pespin <pespin(a)sysmocom.de> Gerrit-Attention: laforge <laforge(a)osmocom.org> Gerrit-MessageType: newpatchset

1 0

[S] Change in osmo-sip-connector[master]: sip: log release caused by status >= 300 as error
by pespin 22 Mar '23

22 Mar '23

Attention is currently required from: osmith. pespin has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-sip-connector/+/32018 ) Change subject: sip: log release caused by status >= 300 as error ...................................................................... Patch Set 1: (1 comment) Patchset: PS1: > But isn't it an error? it's not successful, that would be 2xx. […] I'm fine with ERROR too, but usually ERROR is for unexpected stuff in the program or in the protocol, not for something which is expected to happen maybe. No strong opinion I was just sharing the concern, I'm fine with it. -- To view, visit https://gerrit.osmocom.org/c/osmo-sip-connector/+/32018 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-sip-connector Gerrit-Branch: master Gerrit-Change-Id: I64889d6ce174dc17d44d85aac12e7ee6e6b06164 Gerrit-Change-Number: 32018 Gerrit-PatchSet: 1 Gerrit-Owner: osmith <osmith(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: pespin <pespin(a)sysmocom.de> Gerrit-Attention: osmith <osmith(a)sysmocom.de> Gerrit-Comment-Date: Wed, 22 Mar 2023 14:39:40 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: osmith <osmith(a)sysmocom.de> Comment-In-Reply-To: pespin <pespin(a)sysmocom.de> Gerrit-MessageType: comment

1 0

[S] Change in osmo-sip-connector[master]: sip: log release caused by status >= 300 as error
by osmith 22 Mar '23

22 Mar '23

Attention is currently required from: pespin. osmith has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-sip-connector/+/32018 ) Change subject: sip: log release caused by status >= 300 as error ...................................................................... Patch Set 1: (1 comment) Patchset: PS1: > maybe NOTICE could be better there? But isn't it an error? it's not successful, that would be 2xx. 3xx is for redirection responses that we don't handle (and so I would say it's also an error as it leads to dropping the call), and 4xx, 5xx, 6xx are errors from client/server/global. -- To view, visit https://gerrit.osmocom.org/c/osmo-sip-connector/+/32018 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-sip-connector Gerrit-Branch: master Gerrit-Change-Id: I64889d6ce174dc17d44d85aac12e7ee6e6b06164 Gerrit-Change-Number: 32018 Gerrit-PatchSet: 1 Gerrit-Owner: osmith <osmith(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: pespin <pespin(a)sysmocom.de> Gerrit-Attention: pespin <pespin(a)sysmocom.de> Gerrit-Comment-Date: Wed, 22 Mar 2023 14:37:07 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: pespin <pespin(a)sysmocom.de> Gerrit-MessageType: comment

1 0

[S] Change in osmo-pcu[master]: pdch_ul_controller: log reserved frame numbers
by pespin 22 Mar '23

22 Mar '23

Attention is currently required from: dexter. pespin has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-pcu/+/31022 ) Change subject: pdch_ul_controller: log reserved frame numbers ...................................................................... Patch Set 2: (1 comment) File src/pdch_ul_controller.c: https://gerrit.osmocom.org/c/osmo-pcu/+/31022/comment/d014fc6d_d84d1d0c PS1, Line 196: LOGPDCH(ulc->pdch, DRLCMAC, LOGL_DEBUG, "reserving FN %u\n", item->fn); > We do not know it at this point. It is populated in the while loop. what are you talking about? it->ype is not populated in any way in this while loop? -- To view, visit https://gerrit.osmocom.org/c/osmo-pcu/+/31022 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-pcu Gerrit-Branch: master Gerrit-Change-Id: I4277c572a4cc6cbbf3ac4e67442c9036be687627 Gerrit-Change-Number: 31022 Gerrit-PatchSet: 2 Gerrit-Owner: dexter <pmaier(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-CC: laforge <laforge(a)osmocom.org> Gerrit-CC: pespin <pespin(a)sysmocom.de> Gerrit-Attention: dexter <pmaier(a)sysmocom.de> Gerrit-Comment-Date: Wed, 22 Mar 2023 14:36:12 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: pespin <pespin(a)sysmocom.de> Comment-In-Reply-To: dexter <pmaier(a)sysmocom.de> Gerrit-MessageType: comment

1 0

[S] Change in osmo-sip-connector[master]: sip: log release caused by status >= 300 as error
by pespin 22 Mar '23

22 Mar '23

Attention is currently required from: osmith. pespin has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-sip-connector/+/32018 ) Change subject: sip: log release caused by status >= 300 as error ...................................................................... Patch Set 1: Code-Review+1 (1 comment) Patchset: PS1: maybe NOTICE could be better there? -- To view, visit https://gerrit.osmocom.org/c/osmo-sip-connector/+/32018 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-sip-connector Gerrit-Branch: master Gerrit-Change-Id: I64889d6ce174dc17d44d85aac12e7ee6e6b06164 Gerrit-Change-Number: 32018 Gerrit-PatchSet: 1 Gerrit-Owner: osmith <osmith(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: pespin <pespin(a)sysmocom.de> Gerrit-Attention: osmith <osmith(a)sysmocom.de> Gerrit-Comment-Date: Wed, 22 Mar 2023 14:34:32 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0