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@freecalypso.org> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: daniel <dwillmann@sysmocom.de> Gerrit-Reviewer: falconia <falcon@freecalypso.org> Gerrit-Reviewer: laforge <laforge@osmocom.org> Gerrit-Reviewer: pespin <pespin@sysmocom.de> Gerrit-MessageType: merged