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
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
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
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
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
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
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
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
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