[M] Change in osmo-msc[master]: implement re-assignment to match codecs
neels has submitted this change. (
https://gerrit.osmocom.org/c/osmo-msc/+/35051?usp=email )
Change subject: implement re-assignment to match codecs
..
implement re-assignment to match codecs
This is the last missing piece that allows osmo-msc to make good TFO
codecs choices.
Since the codec_filter, osmo-msc properly gathers codec options and
limitations. But the MO call leg still assigns a voice channel before
getting a response from the MT call leg, and is then stuck with that.
Add the capability to adjust the MO call leg's codec in case the MT side
needs a different codec for TFO.
This is only relevant for 2G; on 3G we always have AMR/IuUP.
For inter-MSC handover, keep the behavior unchanged: offer only the
currently assigned codec to the remote side. Codec-changing HO should be
equally trivial to implement, but that is for another day.
msc_vlr_test_call's codec tests are adjusted to test the new feature in
Ib933554f826c1b4347dfa3f6c4f6fe086be8b133. For now, avoid change in
these tests by validating the first codec in SDP lists only.
Related: OS#6258
Related: osmo-ttcn3-hacks I402ed0523a2a87b83f29c5577b2c828102005d53
Change-Id: I8760feaa8598047369ef8c3ab2673013bac8ac8a
---
M include/osmocom/msc/msc_a.h
M src/libmsc/codec_filter.c
M src/libmsc/gsm_04_08_cc.c
M src/libmsc/msc_a.c
M src/libmsc/msc_ho.c
M tests/msc_vlr/msc_vlr_test_call.c
M tests/msc_vlr/msc_vlr_test_call.err
7 files changed, 120 insertions(+), 60 deletions(-)
Approvals:
neels: Looks good to me, approved
daniel: Looks good to me, but someone else must approve
Jenkins Builder: Verified
pespin: Looks good to me, but someone else must approve
diff --git a/include/osmocom/msc/msc_a.h b/include/osmocom/msc/msc_a.h
index 0276d62..4099d4c 100644
--- a/include/osmocom/msc/msc_a.h
+++ b/include/osmocom/msc/msc_a.h
@@ -216,6 +216,7 @@
int msc_a_ensure_cn_local_rtp(struct msc_a *msc_a, struct gsm_trans *cc_trans);
int msc_a_try_call_assignment(struct gsm_trans *cc_trans);
+void msc_a_tx_assignment_cmd(struct msc_a *msc_a);
const char *msc_a_cm_service_type_to_use(struct msc_a *msc_a, enum
osmo_cm_service_type cm_service_type);
diff --git a/src/libmsc/codec_filter.c b/src/libmsc/codec_filter.c
index a9d93a7..7511f90 100644
--- a/src/libmsc/codec_filter.c
+++ b/src/libmsc/codec_filter.c
@@ -98,46 +98,16 @@
if (remote->audio_codecs.count)
sdp_audio_codecs_intersection(r, >audio_codecs, true);
-#if 0
- /* Future: If osmo-msc were able to trigger a re-assignment after the
remote side has picked a codec mismatching
-* the initial Assignment, then this code here would make sense: keep
the other codecs as available to choose
-* from, but put the currently assigned codec in the first position. So
far we only offer the single assigned
-* codec, because we have no way to deal with the remote side picking a
different codec.
-* Another approach would be to postpone assignment until we know the
codecs from the remote side. */
if (sdp_audio_codec_is_set(a)) {
/* Assignment has completed, the chosen codec should be the
first of the resulting SDP.
-* Make sure this is actually listed in the result SDP and move
to first place. */
+* If present, make sure this is listed in first place.
+* If 'select' is NULL, the assigned codec is not present in
the intersection of possible choices for
+* TFO. Just omit the assigned codec from the filter result,
and it is the CC code's responsibility to
+* detect this and assign a working codec instead. */
struct sdp_audio_codec *select = sdp_audio_codecs_by_descr(r,
a);
-
- if (!select) {
- /* Not present. Add. */
- if (sdp_audio_codec_by_payload_type(r, a->payload_type,
false)) {
- /* Oh crunch, that payload type number is
already in use.
-* Find an unused one. */
- for (a->payload_type = 96; a->payload_type <=
127; a->payload_type++) {
- if (!sdp_audio_codec_by_payload_type(r,
a->payload_type, false))
- break;
- }
-
- if (a->payload_type > 127)
- return -ENOSPC;
- }
- select = sdp_audio_codecs_add_copy(r, a);
- }
-
- sdp_audio_codecs_select(r, select);
+ if (select)
+ sdp_audio_codecs_select(r, select);
}
-#else
- /* Currently, osmo-msc does not trigger re-assignment if the remote
side has picked a codec that is different
-* from the already assigned codec.
-* So, if locally,
[M] Change in osmo-msc[master]: implement re-assignment to match codecs
Attention is currently required from: fixeria. neels has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-msc/+/35051?usp=email ) Change subject: implement re-assignment to match codecs .. Patch Set 7: Code-Review+2 -- To view, visit https://gerrit.osmocom.org/c/osmo-msc/+/35051?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-msc Gerrit-Branch: master Gerrit-Change-Id: I8760feaa8598047369ef8c3ab2673013bac8ac8a Gerrit-Change-Number: 35051 Gerrit-PatchSet: 7 Gerrit-Owner: neels Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: daniel Gerrit-Reviewer: fixeria Gerrit-Reviewer: neels Gerrit-Reviewer: pespin Gerrit-Attention: fixeria Gerrit-Comment-Date: Wed, 13 Dec 2023 01:49:07 + Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment
[M] Change in osmo-msc[master]: implement re-assignment to match codecs
Attention is currently required from: fixeria, neels. daniel has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-msc/+/35051?usp=email ) Change subject: implement re-assignment to match codecs .. Patch Set 7: Code-Review+1 -- To view, visit https://gerrit.osmocom.org/c/osmo-msc/+/35051?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-msc Gerrit-Branch: master Gerrit-Change-Id: I8760feaa8598047369ef8c3ab2673013bac8ac8a Gerrit-Change-Number: 35051 Gerrit-PatchSet: 7 Gerrit-Owner: neels Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: daniel Gerrit-Reviewer: fixeria Gerrit-Reviewer: neels Gerrit-Reviewer: pespin Gerrit-Attention: neels Gerrit-Attention: fixeria Gerrit-Comment-Date: Tue, 12 Dec 2023 14:12:52 + Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment
[M] Change in osmo-msc[master]: implement re-assignment to match codecs
Attention is currently required from: fixeria, neels. pespin has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-msc/+/35051?usp=email ) Change subject: implement re-assignment to match codecs .. Patch Set 7: Code-Review+1 -- To view, visit https://gerrit.osmocom.org/c/osmo-msc/+/35051?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-msc Gerrit-Branch: master Gerrit-Change-Id: I8760feaa8598047369ef8c3ab2673013bac8ac8a Gerrit-Change-Number: 35051 Gerrit-PatchSet: 7 Gerrit-Owner: neels Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: fixeria Gerrit-Reviewer: neels Gerrit-Reviewer: pespin Gerrit-Attention: neels Gerrit-Attention: fixeria Gerrit-Comment-Date: Mon, 11 Dec 2023 10:01:57 + Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment
[M] Change in osmo-msc[master]: implement re-assignment to match codecs
Attention is currently required from: fixeria, pespin. neels has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-msc/+/35051?usp=email ) Change subject: implement re-assignment to match codecs .. Patch Set 7: (3 comments) Patchset: PS1: > This patch is pretty cool, […] Done Patchset: PS6: > I ran 3G-3G voice tests and saw that osmo-msc still successfully sets up IUFP > with the MGW, while fo […] Done Patchset: PS7: i would re-add previous votes because this code does the same as before, but it is structured a bit differently, so maybe just CR again... -- To view, visit https://gerrit.osmocom.org/c/osmo-msc/+/35051?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-msc Gerrit-Branch: master Gerrit-Change-Id: I8760feaa8598047369ef8c3ab2673013bac8ac8a Gerrit-Change-Number: 35051 Gerrit-PatchSet: 7 Gerrit-Owner: neels Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: fixeria Gerrit-Reviewer: neels Gerrit-Reviewer: pespin Gerrit-Attention: pespin Gerrit-Attention: fixeria Gerrit-Comment-Date: Sun, 10 Dec 2023 06:34:51 + Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: neels Gerrit-MessageType: comment
[M] Change in osmo-msc[master]: implement re-assignment to match codecs
Attention is currently required from: fixeria, neels, pespin. Hello Jenkins Builder, fixeria, pespin, I'd like you to reexamine a change. Please visit https://gerrit.osmocom.org/c/osmo-msc/+/35051?usp=email to look at the new patch set (#7). The following approvals got outdated and were removed: Code-Review-1 by neels, Verified+1 by Jenkins Builder Change subject: implement re-assignment to match codecs .. implement re-assignment to match codecs This is the last missing piece that allows osmo-msc to make good TFO codecs choices. Since the codec_filter, osmo-msc properly gathers codec options and limitations. But the MO call leg still assigns a voice channel before getting a response from the MT call leg, and is then stuck with that. Add the capability to adjust the MO call leg's codec in case the MT side needs a different codec for TFO. This is only relevant for 2G; on 3G we always have AMR/IuUP. For inter-MSC handover, keep the behavior unchanged: offer only the currently assigned codec to the remote side. Codec-changing HO should be equally trivial to implement, but that is for another day. msc_vlr_test_call's codec tests are adjusted to test the new feature in Ib933554f826c1b4347dfa3f6c4f6fe086be8b133. For now, avoid change in these tests by validating the first codec in SDP lists only. Related: OS#6258 Related: osmo-ttcn3-hacks I402ed0523a2a87b83f29c5577b2c828102005d53 Change-Id: I8760feaa8598047369ef8c3ab2673013bac8ac8a --- M include/osmocom/msc/msc_a.h M src/libmsc/codec_filter.c M src/libmsc/gsm_04_08_cc.c M src/libmsc/msc_a.c M src/libmsc/msc_ho.c M tests/msc_vlr/msc_vlr_test_call.c M tests/msc_vlr/msc_vlr_test_call.err 7 files changed, 120 insertions(+), 60 deletions(-) git pull ssh://gerrit.osmocom.org:29418/osmo-msc refs/changes/51/35051/7 -- To view, visit https://gerrit.osmocom.org/c/osmo-msc/+/35051?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-msc Gerrit-Branch: master Gerrit-Change-Id: I8760feaa8598047369ef8c3ab2673013bac8ac8a Gerrit-Change-Number: 35051 Gerrit-PatchSet: 7 Gerrit-Owner: neels Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: fixeria Gerrit-Reviewer: neels Gerrit-Reviewer: pespin Gerrit-Attention: neels Gerrit-Attention: pespin Gerrit-Attention: fixeria Gerrit-MessageType: newpatchset
[M] Change in osmo-msc[master]: implement re-assignment to match codecs
Attention is currently required from: fixeria, pespin. neels has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-msc/+/35051?usp=email ) Change subject: implement re-assignment to match codecs .. Patch Set 6: (1 comment) Patchset: PS6: > ah no, still need to verify 3G and IUFP I ran 3G-3G voice tests and saw that osmo-msc still successfully sets up IUFP with the MGW, while forwarding plain AMR to the other call leg. 3G-2G still pending. -- To view, visit https://gerrit.osmocom.org/c/osmo-msc/+/35051?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-msc Gerrit-Branch: master Gerrit-Change-Id: I8760feaa8598047369ef8c3ab2673013bac8ac8a Gerrit-Change-Number: 35051 Gerrit-PatchSet: 6 Gerrit-Owner: neels Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: fixeria Gerrit-Reviewer: neels Gerrit-Reviewer: pespin Gerrit-Attention: pespin Gerrit-Attention: fixeria Gerrit-Comment-Date: Mon, 04 Dec 2023 03:15:30 + Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: neels Gerrit-MessageType: comment
[M] Change in osmo-msc[master]: implement re-assignment to match codecs
Attention is currently required from: fixeria, pespin. neels has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-msc/+/35051?usp=email ) Change subject: implement re-assignment to match codecs .. Patch Set 6: Code-Review-1 (1 comment) Patchset: PS6: ah no, still need to verify 3G and IUFP -- To view, visit https://gerrit.osmocom.org/c/osmo-msc/+/35051?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-msc Gerrit-Branch: master Gerrit-Change-Id: I8760feaa8598047369ef8c3ab2673013bac8ac8a Gerrit-Change-Number: 35051 Gerrit-PatchSet: 6 Gerrit-Owner: neels Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: fixeria Gerrit-Reviewer: neels Gerrit-Reviewer: pespin Gerrit-Attention: pespin Gerrit-Attention: fixeria Gerrit-Comment-Date: Thu, 30 Nov 2023 02:18:56 + Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Gerrit-MessageType: comment
[M] Change in osmo-msc[master]: implement re-assignment to match codecs
Attention is currently required from: fixeria, pespin. neels has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-msc/+/35051?usp=email ) Change subject: implement re-assignment to match codecs .. Patch Set 6: Code-Review+2 (1 comment) Patchset: PS6: combine votes -- To view, visit https://gerrit.osmocom.org/c/osmo-msc/+/35051?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-msc Gerrit-Branch: master Gerrit-Change-Id: I8760feaa8598047369ef8c3ab2673013bac8ac8a Gerrit-Change-Number: 35051 Gerrit-PatchSet: 6 Gerrit-Owner: neels Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: fixeria Gerrit-Reviewer: neels Gerrit-Reviewer: pespin Gerrit-Attention: pespin Gerrit-Attention: fixeria Gerrit-Comment-Date: Thu, 30 Nov 2023 02:17:54 + Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Gerrit-MessageType: comment
[M] Change in osmo-msc[master]: implement re-assignment to match codecs
Attention is currently required from: fixeria, pespin. neels has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-msc/+/35051?usp=email ) Change subject: implement re-assignment to match codecs .. Patch Set 6: (2 comments) This change is ready for review. File src/libmsc/gsm_04_08_cc.c: https://gerrit.osmocom.org/c/osmo-msc/+/35051/comment/a3383a1b_5f76f052 PS4, Line 276: LOG_TRANS_CAT_SRC(trans, DMNCC, LOGL_ERROR, file, line, "%s %s: invalid SDP message (trying anyway)\n", > why 2 log calls here? one is ERROR the other is DEBUG File src/libmsc/msc_a.c: https://gerrit.osmocom.org/c/osmo-msc/+/35051/comment/e67182d0_18e79211 PS4, Line 639: void msc_a_tx_assignment(struct msc_a *msc_a) > msc_a_tx_assignment_cmd Done -- To view, visit https://gerrit.osmocom.org/c/osmo-msc/+/35051?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-msc Gerrit-Branch: master Gerrit-Change-Id: I8760feaa8598047369ef8c3ab2673013bac8ac8a Gerrit-Change-Number: 35051 Gerrit-PatchSet: 6 Gerrit-Owner: neels Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: fixeria Gerrit-Reviewer: pespin Gerrit-Attention: pespin Gerrit-Attention: fixeria Gerrit-Comment-Date: Thu, 30 Nov 2023 02:17:37 + Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: pespin Gerrit-MessageType: comment
[M] Change in osmo-msc[master]: implement re-assignment to match codecs
Jenkins Builder has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-msc/+/35051?usp=email ) Change subject: implement re-assignment to match codecs .. Patch Set 1: (1 comment) File src/libmsc/codec_filter.c: Robot Comment from checkpatch (run ID jenkins-gerrit-lint-12445): https://gerrit.osmocom.org/c/osmo-msc/+/35051/comment/4e90_0b19f4aa PS1, Line 101: #if 1 Consider removing the #if 1 and its #endif -- To view, visit https://gerrit.osmocom.org/c/osmo-msc/+/35051?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-msc Gerrit-Branch: master Gerrit-Change-Id: I8760feaa8598047369ef8c3ab2673013bac8ac8a Gerrit-Change-Number: 35051 Gerrit-PatchSet: 1 Gerrit-Owner: neels Gerrit-CC: Jenkins Builder Gerrit-Comment-Date: Fri, 17 Nov 2023 03:39:17 + Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment
[M] Change in osmo-msc[master]: implement re-assignment to match codecs
neels has uploaded this change for review. (
https://gerrit.osmocom.org/c/osmo-msc/+/35051?usp=email )
Change subject: implement re-assignment to match codecs
..
implement re-assignment to match codecs
This is the last missing piece that allows osmo-msc to make good TFO
codecs choices.
Since the codec_filter, osmo-msc properly gathers codec options and
limitations. But the MO call leg still assigns a voice channel before
getting a response from the MT call leg, and is then stuck with that.
Add the capability to adjust the MO call leg's codec in case the MT side
needs a different codec for TFO.
This is only relevant for 2G; on 3G we always have AMR/IuUP.
Related: OS#6258
Related: osmo-ttcn3-hacks I402ed0523a2a87b83f29c5577b2c828102005d53
Change-Id: I8760feaa8598047369ef8c3ab2673013bac8ac8a
---
M include/osmocom/msc/msc_a.h
M src/libmsc/call_leg.c
M src/libmsc/codec_filter.c
M src/libmsc/gsm_04_08_cc.c
M src/libmsc/msc_a.c
M src/libmsc/msc_ho.c
M tests/msc_vlr/msc_vlr_test_call.c
M tests/msc_vlr/msc_vlr_test_call.err
8 files changed, 106 insertions(+), 44 deletions(-)
git pull ssh://gerrit.osmocom.org:29418/osmo-msc refs/changes/51/35051/1
diff --git a/include/osmocom/msc/msc_a.h b/include/osmocom/msc/msc_a.h
index 0276d62..5e1672a 100644
--- a/include/osmocom/msc/msc_a.h
+++ b/include/osmocom/msc/msc_a.h
@@ -216,6 +216,7 @@
int msc_a_ensure_cn_local_rtp(struct msc_a *msc_a, struct gsm_trans *cc_trans);
int msc_a_try_call_assignment(struct gsm_trans *cc_trans);
+void msc_a_tx_assignment(struct msc_a *msc_a);
const char *msc_a_cm_service_type_to_use(struct msc_a *msc_a, enum
osmo_cm_service_type cm_service_type);
diff --git a/src/libmsc/call_leg.c b/src/libmsc/call_leg.c
index b797322..5720417 100644
--- a/src/libmsc/call_leg.c
+++ b/src/libmsc/call_leg.c
@@ -158,7 +158,8 @@
}
if (!established)
break;
- call_leg_state_chg(cl, CALL_LEG_ST_ESTABLISHED);
+ if (cl->fi->state != CALL_LEG_ST_ESTABLISHED)
+ call_leg_state_chg(cl, CALL_LEG_ST_ESTABLISHED);
break;
case CALL_LEG_EV_RTP_STREAM_ADDR_AVAILABLE:
diff --git a/src/libmsc/codec_filter.c b/src/libmsc/codec_filter.c
index a9d93a7..1a163ae 100644
--- a/src/libmsc/codec_filter.c
+++ b/src/libmsc/codec_filter.c
@@ -98,7 +98,7 @@
if (remote->audio_codecs.count)
sdp_audio_codecs_intersection(r, >audio_codecs, true);
-#if 0
+#if 1
/* Future: If osmo-msc were able to trigger a re-assignment after the
remote side has picked a codec mismatching
* the initial Assignment, then this code here would make sense: keep
the other codecs as available to choose
* from, but put the currently assigned codec in the first position. So
far we only offer the single assigned
@@ -108,24 +108,11 @@
/* Assignment has completed, the chosen codec should be the
first of the resulting SDP.
* Make sure this is actually listed in the result SDP and move
to first place. */
struct sdp_audio_codec *select = sdp_audio_codecs_by_descr(r,
a);
-
- if (!select) {
- /* Not present. Add. */
- if (sdp_audio_codec_by_payload_type(r, a->payload_type,
false)) {
- /* Oh crunch, that payload type number is
already in use.
-* Find an unused one. */
- for (a->payload_type = 96; a->payload_type <=
127; a->payload_type++) {
- if (!sdp_audio_codec_by_payload_type(r,
a->payload_type, false))
- break;
- }
-
- if (a->payload_type > 127)
- return -ENOSPC;
- }
- select = sdp_audio_codecs_add_copy(r, a);
- }
-
- sdp_audio_codecs_select(r, select);
+ if (select)
+ sdp_audio_codecs_select(r, select);
+ /* If 'select' is NULL, the assigned codec is not present in
the intersection of possible choices for
+* TFO. We could add it, but it would taint the filter result.
Just omit the assigned codec, and it is
+* the CC code's responsibility to detect this and assign a
working codec. */
}
#else
/* Currently, osmo-msc does not trigger re-assignment if the remote
side has picked a codec that is different
diff --git a/src/libmsc/gsm_04_08_cc.c b/src/libmsc/gsm_04_08_cc.c
index 31fcb23..6b8819f 100644
--- a/src/libmsc/gsm_04_08_cc.c
+++ b/src/libmsc/gsm_04_08_cc.c
@@ -270,7 +270,16 @@
break;
}
- if (sdp && sdp[0] && (sdp_msg_from_sdp_str(_msg, sdp) == 0)) {
+ if (sdp &&
