Attention is currently required from: pespin.
osmith has posted comments on this change. ( https://gerrit.osmocom.org/c/libosmo-netif/+/30173 )
Change subject: tests/osmux: Test replay of one lost RTP packet when generating osmux batches
......................................................................
Patch Set 2: Code-Review+1
--
To view, visit https://gerrit.osmocom.org/c/libosmo-netif/+/30173
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings
Gerrit-Project: libosmo-netif
Gerrit-Branch: master
Gerrit-Change-Id: I913c2ccfc3ad4e7ed801344d64e33e166a0817cf
Gerrit-Change-Number: 30173
Gerrit-PatchSet: 2
Gerrit-Owner: pespin <pespin(a)sysmocom.de>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: laforge <laforge(a)osmocom.org>
Gerrit-Reviewer: osmith <osmith(a)sysmocom.de>
Gerrit-Attention: pespin <pespin(a)sysmocom.de>
Gerrit-Comment-Date: Thu, 17 Nov 2022 09:06:50 +0000
Gerrit-HasComments: No
Gerrit-Has-Labels: Yes
Gerrit-MessageType: comment
Attention is currently required from: pespin.
neels has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-msc/+/30126 )
Change subject: [codecs filter] use filter result
......................................................................
Patch Set 1:
(2 comments)
Patchset:
PS1:
looking at this patch again, it looks rather unclear why the codecs filter result ends up in assignment... it looks much cleaner in a later patch, where i must still investigate a minor odd change in MNCC behavior. I adjusted the commit log to explain...
I think i will wait until that follow up patch is ready before submitting a new patch set.
BTW, it is super complex to explain the legacy code and its transition to a new implementation, i'm beginning to think i should just have secretly merged that code bomb without anyone noticing, instead of kneading through all of these weird details ;)
File tests/msc_vlr/msc_vlr_test_call.err:
https://gerrit.osmocom.org/c/osmo-msc/+/30126/comment/09923322_ae1f2455
PS1, Line 320: MGW <--CRCX to RTP_TO_CN-- MSC: callref=0x80000001 codecs=VND.3GPP.IUFP#96
> Is this change AMR->IUFP expected?
yes, i also find it curious... note that the surrounding logging indicates UTRAN, so I expect that previous code erratically used AMR in the first CRCX instead of the IUFP that we are actually (so far) feeding through to the CN. It is definitely correct to show IUFP here.
--
To view, visit https://gerrit.osmocom.org/c/osmo-msc/+/30126
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings
Gerrit-Project: osmo-msc
Gerrit-Branch: master
Gerrit-Change-Id: I66e7c8c5e401f4f3a7d3d42b9525b2c6e99691d9
Gerrit-Change-Number: 30126
Gerrit-PatchSet: 1
Gerrit-Owner: neels <nhofmeyr(a)sysmocom.de>
Gerrit-Reviewer: Jenkins Builder
Gerrit-CC: pespin <pespin(a)sysmocom.de>
Gerrit-Attention: pespin <pespin(a)sysmocom.de>
Gerrit-Comment-Date: Thu, 17 Nov 2022 00:31:57 +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.
neels has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-msc/+/30121 )
Change subject: [codecs filter] MT call: apply remote call leg codecs
......................................................................
Patch Set 1:
(1 comment)
File src/libmsc/gsm_04_08_cc.c:
https://gerrit.osmocom.org/c/osmo-msc/+/30121/comment/206b6f18_32b24930
PS1, Line 723: trans->bearer_cap = bearer_cap;
> why not using trans->bearer_cap directly here instead of using a local var, resetting it, then copyi […]
The main aim in this function is to send a Bearer Cap to the MS in a CC Setup message, that is what the local variable bearer_cap is about.
Previous code also placed this info in trans->bearer_cap, which I believe is no longer necessary once the codecs filter is fully in place -- it is likely to be dropped, so trans->bearer_cap is just a legacy leftover. Keeping it for now.
BTW, my intention was to separate the patches that store info in the codecs filter from patches that actually use the filter result -- this patch did both, as i noticed now, so i separated this patch further; see I9586221ef56352b7ce4b2604ae0dc04554145a78 (to be submitted soon) for the part that i removed here.
--
To view, visit https://gerrit.osmocom.org/c/osmo-msc/+/30121
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings
Gerrit-Project: osmo-msc
Gerrit-Branch: master
Gerrit-Change-Id: I84d9bbca3e4061da622b1b2fc0bde8868e7e3521
Gerrit-Change-Number: 30121
Gerrit-PatchSet: 1
Gerrit-Owner: neels <nhofmeyr(a)sysmocom.de>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: laforge <laforge(a)osmocom.org>
Gerrit-Reviewer: pespin <pespin(a)sysmocom.de>
Gerrit-Attention: pespin <pespin(a)sysmocom.de>
Gerrit-Comment-Date: Thu, 17 Nov 2022 00:15:00 +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.
neels has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-msc/+/30120 )
Change subject: [codecs filter] MT call: apply BSS codec list
......................................................................
Patch Set 1:
(1 comment)
File src/libmsc/gsm_04_08_cc.c:
https://gerrit.osmocom.org/c/osmo-msc/+/30120/comment/b126db84_a9043d85
PS1, Line 685: codec_filter_set_bss(&trans->cc.codecs, &trans->msc_a->cc.compl_l3_codec_list_bss_supported);
> (related to previous commit) See, you are no calling run() here.
i also stumbled on this when separating the patches, but this place here is so early in the CC that it hardly makes sense to run the filter now.
sounds like i am contradicting my previous comment ("cheap to run, rather run too often") -- well, i do know that the result of this patch series is able to establish voice calls accurately and hope to not spend more time on these marginal aspects now...
--
To view, visit https://gerrit.osmocom.org/c/osmo-msc/+/30120
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings
Gerrit-Project: osmo-msc
Gerrit-Branch: master
Gerrit-Change-Id: I530409a64d11da48518a3dc60aa3a4e47c384663
Gerrit-Change-Number: 30120
Gerrit-PatchSet: 1
Gerrit-Owner: neels <nhofmeyr(a)sysmocom.de>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: laforge <laforge(a)osmocom.org>
Gerrit-Reviewer: pespin <pespin(a)sysmocom.de>
Gerrit-Attention: pespin <pespin(a)sysmocom.de>
Gerrit-Comment-Date: Wed, 16 Nov 2022 23:00:36 +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.
neels has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-msc/+/30119 )
Change subject: [codecs filter] MO call: apply BSS codec list
......................................................................
Patch Set 1:
(1 comment)
File src/libmsc/gsm_04_08_cc.c:
https://gerrit.osmocom.org/c/osmo-msc/+/30119/comment/a62a9ec2_5736118d
PS1, Line 619: codec_filter_run(&trans->cc.codecs);
> Why running it now instead of waiting to do so when you need to retrieve the result? […]
i first wanted to run it implicitly whenever any of the input information changes, so that it always reflects an accurate result.
but that doesn't make sense when we're going to place N inputs directly one after another, so i put the "run" in a separately called function.
what you say is true, but running the filter is cheap, and i'd rather err on running it too often. (possibly this makes more sense after some later patches, but i'm not perfectly sure. i'm busy breaking down a code bomb into readable pieces... and it is taking way too long, so i hope that this is fine with you, certainly not harmful)
--
To view, visit https://gerrit.osmocom.org/c/osmo-msc/+/30119
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings
Gerrit-Project: osmo-msc
Gerrit-Branch: master
Gerrit-Change-Id: I062268406ae3f3a63a7f413db51c509c9eaf9e8a
Gerrit-Change-Number: 30119
Gerrit-PatchSet: 1
Gerrit-Owner: neels <nhofmeyr(a)sysmocom.de>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: laforge <laforge(a)osmocom.org>
Gerrit-CC: pespin <pespin(a)sysmocom.de>
Gerrit-Attention: pespin <pespin(a)sysmocom.de>
Gerrit-Comment-Date: Wed, 16 Nov 2022 22:55:32 +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.
neels has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-msc/+/30117 )
Change subject: [codecs filter] add trans.cc.codecs
......................................................................
Patch Set 1:
(1 comment)
Patchset:
PS1:
> Really? a commit only adding a struct without any use?
see the list of [codecs filter] patches; i could merge all of those to one patch, but it would make the individual places harder to separate and understand. so i'm first adding the empty unused instance and piecemeal add the real information; until in the end one "final" patch starts to actually use the gathered information.
--
To view, visit https://gerrit.osmocom.org/c/osmo-msc/+/30117
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings
Gerrit-Project: osmo-msc
Gerrit-Branch: master
Gerrit-Change-Id: Ib3fdeff8d1e1ea0760168d63ee6e1b1fb993aa5f
Gerrit-Change-Number: 30117
Gerrit-PatchSet: 1
Gerrit-Owner: neels <nhofmeyr(a)sysmocom.de>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: laforge <laforge(a)osmocom.org>
Gerrit-CC: pespin <pespin(a)sysmocom.de>
Gerrit-Attention: pespin <pespin(a)sysmocom.de>
Gerrit-Comment-Date: Wed, 16 Nov 2022 22:48:30 +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.
neels has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-msc/+/30116 )
Change subject: [codecs filter] add codec_filter.h,c
......................................................................
Patch Set 1:
(1 comment)
File src/libmsc/codec_filter.c:
https://gerrit.osmocom.org/c/osmo-msc/+/30116/comment/a90de27e_cb9797a4
PS1, Line 124: /* If osmo-msc were able to trigger a re-assignment after the remote side has picked a codec mismatching the
> Does this mean osmo-msc currently doesn't support it but it could support it?
yes.
When we offer the remote call leg a list of codecs, that remote could freely pick one/several of those. But currently osmo-msc first does an Assignment with one specific codec.
So it *could* go like this:
1. local call leg assigns a codec
2. offer a list of possible codecs to other side
3. other side picks a different codec than the assigned one
4. local call leg re-assigns a different codec
but currently osmo-msc has no code to do step 4.
it wouldn't be that hard to implement, but i want to get the current pipeline of patches through without adding more work now.
but, instead, my favorite way to fix this behavior is to postpone assignment to after the remote side has responded with a list of the remote codecs, then we can directly assign a codec that matches both sides. again, something to consider later:
1. offer a list of codecs
2. remote responds with a list of codecs it supports
3. intersect that info and assign a codec matching both call legs
either of these ideas would finally implement some sort of intelligence in matching codecs between call legs; so far we're stuck with MO call leg dictating a single codec, and if the MT side doesn't support it the call is impossible (even if a different codec choice would work on both sides)
'#if ALLOW_REASSIGNMENT' is more like '#if 0' now
--
To view, visit https://gerrit.osmocom.org/c/osmo-msc/+/30116
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings
Gerrit-Project: osmo-msc
Gerrit-Branch: master
Gerrit-Change-Id: I4d90f7ca62f2307a7b93dd164aeecbf4bd98ff0a
Gerrit-Change-Number: 30116
Gerrit-PatchSet: 1
Gerrit-Owner: neels <nhofmeyr(a)sysmocom.de>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: pespin <pespin(a)sysmocom.de>
Gerrit-CC: laforge <laforge(a)osmocom.org>
Gerrit-Attention: pespin <pespin(a)sysmocom.de>
Gerrit-Comment-Date: Wed, 16 Nov 2022 22:48:03 +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.
neels has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-msc/+/30112 )
Change subject: rtp_stream_commit: check missing MGW ep only when ready for RTP
......................................................................
Patch Set 1:
(1 comment)
Patchset:
PS1:
> What's the benefit of this patch? I lack some rationale.
a more meaningful log message: no mgw endpoint was set up because no RTP info is available yet, makes more sense to log the real reason
--
To view, visit https://gerrit.osmocom.org/c/osmo-msc/+/30112
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings
Gerrit-Project: osmo-msc
Gerrit-Branch: master
Gerrit-Change-Id: I24a81a926b97c9f0fb31df782d1cf931eaff9db1
Gerrit-Change-Number: 30112
Gerrit-PatchSet: 1
Gerrit-Owner: neels <nhofmeyr(a)sysmocom.de>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: fixeria <vyanitskiy(a)sysmocom.de>
Gerrit-Reviewer: laforge <laforge(a)osmocom.org>
Gerrit-CC: pespin <pespin(a)sysmocom.de>
Gerrit-Attention: pespin <pespin(a)sysmocom.de>
Gerrit-Comment-Date: Wed, 16 Nov 2022 22:43:15 +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.
laforge has posted comments on this change. ( https://gerrit.osmocom.org/c/libosmo-netif/+/30191 )
Change subject: tests/osmux: Test big seqnum holes (>batch_factor) in incoming RTP stream
......................................................................
Patch Set 2: Code-Review+1
--
To view, visit https://gerrit.osmocom.org/c/libosmo-netif/+/30191
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings
Gerrit-Project: libosmo-netif
Gerrit-Branch: master
Gerrit-Change-Id: I521c2e97a739e8a824b16f06ec2a578333388247
Gerrit-Change-Number: 30191
Gerrit-PatchSet: 2
Gerrit-Owner: pespin <pespin(a)sysmocom.de>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: laforge <laforge(a)osmocom.org>
Gerrit-Attention: pespin <pespin(a)sysmocom.de>
Gerrit-Comment-Date: Wed, 16 Nov 2022 21:15:46 +0000
Gerrit-HasComments: No
Gerrit-Has-Labels: Yes
Gerrit-MessageType: comment