falconia has submitted this change. ( https://gerrit.osmocom.org/c/osmo-mgw/+/37299?usp=email )
Change subject: E1: support HRv1 codec on both 16k and 8k subslots
......................................................................
E1: support HRv1 codec on both 16k and 8k subslots
HRv1 support in OsmoMGW-E1 was previously broken (couldn't work
on either 16k or 8k subslots) because of inconsistency: the TRAU
frame type was set to OSMO_TRAU16_FT_HR, but the TRAU sync pattern
was set to OSMO_TRAU_SYNCP_8_HR. However, now that libosmotrau
has proper support for HRv1 TRAU frame encoding and RTP conversion
in both 16k and 8k formats, drive it correctly in OsmoMGW-E1.
While at it, change the code structure to avoid else-after-return.
Was the original code written and merged in a time before strict
linter checks?
Change-Id: Ifadbdc68905178c6ffdd673a6fb71c18610c9847
---
M src/libosmo-mgcp/mgcp_e1.c
1 file changed, 56 insertions(+), 16 deletions(-)
Approvals:
laforge: Looks good to me, but someone else must approve
pespin: Looks good to me, but someone else must approve
daniel: Looks good to me, but someone else must approve
Jenkins Builder: Verified
falconia: Looks good to me, approved
diff --git a/src/libosmo-mgcp/mgcp_e1.c b/src/libosmo-mgcp/mgcp_e1.c
index 20aff95..1180e03 100644
--- a/src/libosmo-mgcp/mgcp_e1.c
+++ b/src/libosmo-mgcp/mgcp_e1.c
@@ -465,11 +465,21 @@
{
if (strcmp(sdp_subtype_name, "GSM") == 0)
return OSMO_TRAU16_FT_FR;
- else if (strcmp(sdp_subtype_name, "GSM-EFR") == 0)
+
+ if (strcmp(sdp_subtype_name, "GSM-EFR") == 0)
return OSMO_TRAU16_FT_EFR;
- else if (strcmp(sdp_subtype_name, "GSM-HR-08") == 0)
- return OSMO_TRAU16_FT_HR;
- else if (strcmp(sdp_subtype_name, "AMR") == 0) {
+
+ if (strcmp(sdp_subtype_name, "GSM-HR-08") == 0) {
+ if (i460_rate == OSMO_I460_RATE_16k)
+ return OSMO_TRAU16_FT_HR;
+ if (i460_rate == OSMO_I460_RATE_8k)
+ return OSMO_TRAU8_SPEECH;
+ LOGPENDP(endp, DE1, LOGL_ERROR,
+ "E1-TRAU-TX: unsupported or illegal I.460 rate for HR\n");
+ return OSMO_TRAU_FT_NONE;
+ }
+
+ if (strcmp(sdp_subtype_name, "AMR") == 0) {
if (i460_rate == OSMO_I460_RATE_8k) {
switch (amr_ft) {
case AMR_4_75:
@@ -487,11 +497,11 @@
}
}
return OSMO_TRAU16_FT_AMR;
- } else {
- LOGPENDP(endp, DE1, LOGL_ERROR, "E1-TRAU-TX: unsupported or illegal codec subtype name: %s\n",
- sdp_subtype_name);
- return OSMO_TRAU_FT_NONE;
}
+
+ LOGPENDP(endp, DE1, LOGL_ERROR, "E1-TRAU-TX: unsupported or illegal codec subtype name: %s\n",
+ sdp_subtype_name);
+ return OSMO_TRAU_FT_NONE;
}
/* Determine a suitable TRAU frame type for a given codec */
@@ -500,11 +510,21 @@
{
if (strcmp(sdp_subtype_name, "GSM") == 0)
return OSMO_TRAU_SYNCP_16_FR_EFR;
- else if (strcmp(sdp_subtype_name, "GSM-EFR") == 0)
+
+ if (strcmp(sdp_subtype_name, "GSM-EFR") == 0)
return OSMO_TRAU_SYNCP_16_FR_EFR;
- else if (strcmp(sdp_subtype_name, "GSM-HR-08") == 0)
- return OSMO_TRAU_SYNCP_8_HR;
- else if (strcmp(sdp_subtype_name, "AMR") == 0) {
+
+ if (strcmp(sdp_subtype_name, "GSM-HR-08") == 0) {
+ if (i460_rate == OSMO_I460_RATE_16k)
+ return OSMO_TRAU_SYNCP_16_FR_EFR;
+ if (i460_rate == OSMO_I460_RATE_8k)
+ return OSMO_TRAU_SYNCP_8_HR;
+ LOGPENDP(endp, DE1, LOGL_ERROR,
+ "E1-TRAU-TX: unsupported or illegal I.460 rate for HR\n");
+ return OSMO_TRAU_SYNCP_16_FR_EFR;
+ }
+
+ if (strcmp(sdp_subtype_name, "AMR") == 0) {
if (i460_rate == OSMO_I460_RATE_8k) {
switch (amr_ft) {
case AMR_4_75:
@@ -522,11 +542,11 @@
}
}
return OSMO_TRAU_SYNCP_16_FR_EFR;
- } else {
- LOGPENDP(endp, DE1, LOGL_ERROR, "E1-TRAU-TX: unsupported or illegal codec subtype name: %s\n",
- sdp_subtype_name);
- return OSMO_TRAU_SYNCP_16_FR_EFR;
}
+
+ LOGPENDP(endp, DE1, LOGL_ERROR, "E1-TRAU-TX: unsupported or illegal codec subtype name: %s\n",
+ sdp_subtype_name);
+ return OSMO_TRAU_SYNCP_16_FR_EFR;
}
/* Find out if a given TRAU frame type is AMR */
--
To view, visit https://gerrit.osmocom.org/c/osmo-mgw/+/37299?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: Ifadbdc68905178c6ffdd673a6fb71c18610c9847
Gerrit-Change-Number: 37299
Gerrit-PatchSet: 2
Gerrit-Owner: falconia <falcon(a)freecalypso.org>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: daniel <dwillmann(a)sysmocom.de>
Gerrit-Reviewer: falconia <falcon(a)freecalypso.org>
Gerrit-Reviewer: laforge <laforge(a)osmocom.org>
Gerrit-Reviewer: pespin <pespin(a)sysmocom.de>
Gerrit-MessageType: merged
falconia has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-mgw/+/37299?usp=email )
Change subject: E1: support HRv1 codec on both 16k and 8k subslots
......................................................................
Patch Set 2: Code-Review+2
--
To view, visit https://gerrit.osmocom.org/c/osmo-mgw/+/37299?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: Ifadbdc68905178c6ffdd673a6fb71c18610c9847
Gerrit-Change-Number: 37299
Gerrit-PatchSet: 2
Gerrit-Owner: falconia <falcon(a)freecalypso.org>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: daniel <dwillmann(a)sysmocom.de>
Gerrit-Reviewer: falconia <falcon(a)freecalypso.org>
Gerrit-Reviewer: laforge <laforge(a)osmocom.org>
Gerrit-Reviewer: pespin <pespin(a)sysmocom.de>
Gerrit-Comment-Date: Sat, 29 Jun 2024 22:14:53 +0000
Gerrit-HasComments: No
Gerrit-Has-Labels: Yes
Gerrit-MessageType: comment
Attention is currently required from: falconia.
laforge has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-mgw/+/37299?usp=email )
Change subject: E1: support HRv1 codec on both 16k and 8k subslots
......................................................................
Patch Set 2: Code-Review+1
--
To view, visit https://gerrit.osmocom.org/c/osmo-mgw/+/37299?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: Ifadbdc68905178c6ffdd673a6fb71c18610c9847
Gerrit-Change-Number: 37299
Gerrit-PatchSet: 2
Gerrit-Owner: falconia <falcon(a)freecalypso.org>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: daniel <dwillmann(a)sysmocom.de>
Gerrit-Reviewer: laforge <laforge(a)osmocom.org>
Gerrit-Reviewer: pespin <pespin(a)sysmocom.de>
Gerrit-Attention: falconia <falcon(a)freecalypso.org>
Gerrit-Comment-Date: Sat, 29 Jun 2024 20:06:55 +0000
Gerrit-HasComments: No
Gerrit-Has-Labels: Yes
Gerrit-MessageType: comment
fixeria has submitted this change. ( https://gerrit.osmocom.org/c/erlang/osmo-s1gw/+/37306?usp=email )
Change subject: contrib/jenkins.sh: also run Dialyzer
......................................................................
contrib/jenkins.sh: also run Dialyzer
Dialyzer is a static code analyzer for Erlang, see:
https://www.erlang.org/docs/23/man/dialyzer.html
Change-Id: Id9fe11e9eb36d7ed468288094095a6d38b2d1d49
---
M Makefile
M contrib/jenkins.sh
2 files changed, 17 insertions(+), 0 deletions(-)
Approvals:
Jenkins Builder: Verified
fixeria: Looks good to me, approved
pespin: Looks good to me, but someone else must approve
daniel: Looks good to me, but someone else must approve
diff --git a/Makefile b/Makefile
index 6d3ded2..2a14fe7 100644
--- a/Makefile
+++ b/Makefile
@@ -18,6 +18,9 @@
check: $(GEN_FILES)
rebar3 eunit
+analyze: $(GEN_FILES)
+ rebar3 dialyzer
+
clean:
# Avoid running rebar3 clean if _build doesn't exist, since it would try
# to fetch deps from the Internet and that may not be avaialble when in
diff --git a/contrib/jenkins.sh b/contrib/jenkins.sh
index df99a89..c738791 100755
--- a/contrib/jenkins.sh
+++ b/contrib/jenkins.sh
@@ -3,3 +3,4 @@
make clean
make
make check
+make analyze
--
To view, visit https://gerrit.osmocom.org/c/erlang/osmo-s1gw/+/37306?usp=email
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings
Gerrit-Project: erlang/osmo-s1gw
Gerrit-Branch: master
Gerrit-Change-Id: Id9fe11e9eb36d7ed468288094095a6d38b2d1d49
Gerrit-Change-Number: 37306
Gerrit-PatchSet: 1
Gerrit-Owner: fixeria <vyanitskiy(a)sysmocom.de>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: daniel <dwillmann(a)sysmocom.de>
Gerrit-Reviewer: fixeria <vyanitskiy(a)sysmocom.de>
Gerrit-Reviewer: pespin <pespin(a)sysmocom.de>
Gerrit-MessageType: merged
fixeria has posted comments on this change. ( https://gerrit.osmocom.org/c/erlang/osmo-s1gw/+/37306?usp=email )
Change subject: contrib/jenkins.sh: also run Dialyzer
......................................................................
Patch Set 1: Code-Review+2
--
To view, visit https://gerrit.osmocom.org/c/erlang/osmo-s1gw/+/37306?usp=email
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings
Gerrit-Project: erlang/osmo-s1gw
Gerrit-Branch: master
Gerrit-Change-Id: Id9fe11e9eb36d7ed468288094095a6d38b2d1d49
Gerrit-Change-Number: 37306
Gerrit-PatchSet: 1
Gerrit-Owner: fixeria <vyanitskiy(a)sysmocom.de>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: daniel <dwillmann(a)sysmocom.de>
Gerrit-Reviewer: fixeria <vyanitskiy(a)sysmocom.de>
Gerrit-Reviewer: pespin <pespin(a)sysmocom.de>
Gerrit-Comment-Date: Fri, 28 Jun 2024 20:54:07 +0000
Gerrit-HasComments: No
Gerrit-Has-Labels: Yes
Gerrit-MessageType: comment