Change in osmo-bsc[master]: codec_pref: fix special handling for AMR rate configuration (S15-S0)

This is merely a historical archive of years 2008-2021, before the migration to mailman3.

A maintained and still updated list archive can be found at https://lists.osmocom.org/hyperkitty/list/gerrit-log@lists.osmocom.org/.

Harald Welte gerrit-no-reply at lists.osmocom.org
Tue Mar 19 13:29:11 UTC 2019


Harald Welte has submitted this change and it was merged. ( https://gerrit.osmocom.org/13093 )

Change subject: codec_pref: fix special handling for AMR rate configuration  (S15-S0)
......................................................................

codec_pref: fix special handling for AMR rate configuration  (S15-S0)

When match_codec_pref() is called and the codec selections from the
ASSIGNMENT COMMAND are matched against the interal capabilities, the
configurations are checked one by one. When a match is found that match
is returned.

However, the implementation currently does not check the AMR S15-S0 bits
when the actual matching happens. This is done afterwards in case AMR
gets picked. Unfortunately if the MSC implementation is not obeying the
settings the MSC has previously communicated in the L3 COMPL message we
may end up with an S15-S0 configuration that has all rate selection
(which eventually end up as active set in RSL) bits set to zero. This is
an invalid configuration and should be prevented. Also the handling of
the S15-S0 bits should happen as part of the matching so that there is a
chance to try the nect codec in the list if AMR is unuseable.

Also S15-S0 has one special setting "Config-NB-Code = 1" (S1 = 1). When
this bit is set, the four (in HR only three) most common rates are
selected into the active set. If there are also other S-bits set besides
S1 we should prefer S1 and discard the other bits.

- Perform handling of S15-S0 while matching
- If S1 is set, prefer this setting and discard all other settings

Change-Id: Ie52376b51fe07ed07056e8df2e9557293ff67a78
Related: SYS#4470
---
M src/osmo-bsc/codec_pref.c
M tests/codec_pref/codec_pref_test.ok
2 files changed, 62 insertions(+), 35 deletions(-)

Approvals:
  Harald Welte: Looks good to me, approved
  Jenkins Builder: Verified



diff --git a/src/osmo-bsc/codec_pref.c b/src/osmo-bsc/codec_pref.c
index a94d6a8..412ed9a 100644
--- a/src/osmo-bsc/codec_pref.c
+++ b/src/osmo-bsc/codec_pref.c
@@ -264,6 +264,47 @@
 	return amr_s15_s0_bts & amr_s15_s0_msc;
 }
 
+/* Special handling for AMR rate configuration bits (S15-S0) */
+static int match_amr_s15_s0(struct channel_mode_and_rate *ch_mode_rate, const struct bsc_msc_data *msc, const struct gsm_bts *bts, const struct gsm0808_speech_codec *sc_match, uint8_t perm_spch)
+{
+	uint16_t amr_s15_s0_supported;
+	
+	/* Normally the MSC should never try to advertise an AMR codec
+	 * configuration that we did not previously advertised as supported.
+	 * However, to ensure that no unsupported AMR codec configuration
+	 * enters the further processing steps we again lookup what we support
+	 * and generate an intersection. All further processing is then done
+	 * with this intersection result. At the same time we will make sure
+	 * that the intersection contains at least one rate setting. */
+	
+	amr_s15_s0_supported = gen_bss_supported_amr_s15_s0(msc, bts, (perm_spch == GSM0808_PERM_HR3));
+
+	/* NOTE: The sc_match pointer points to a speech codec from the speech
+	 * codec list that has been communicated with the ASSIGNMENT COMMAND.
+	 * However, only AoIP based networks will include a speech codec list
+	 * into the ASSIGNMENT COMMAND. For non AoIP based networks, no speech
+	 * codec (sc_match) will be available, so we will fully rely on the
+	 * local configuration for thoses cases. */
+	if (sc_match)
+		ch_mode_rate->s15_s0 = sc_match->cfg & amr_s15_s0_supported;
+	else
+		ch_mode_rate->s15_s0 = amr_s15_s0_supported;
+
+	/* Prefer "Config-NB-Code = 1" (S1) over all other AMR rates setttings.
+	 * When S1 is set, the active set will automatically include 12.2k, 7.4k,
+	 * 5.9k, 4.75k, in case of HR 12,2k is left out. */
+	if (ch_mode_rate->s15_s0 & GSM0808_SC_CFG_AMR_4_75_5_90_7_40_12_20) {
+		ch_mode_rate->s15_s0 &= 0xff00;
+		ch_mode_rate->s15_s0 |= GSM0808_SC_CFG_AMR_4_75_5_90_7_40_12_20;
+	}
+
+	/* Make sure at least one rate is set. */
+	if ((ch_mode_rate->s15_s0 & 0x00ff) == 0x0000)
+		return -EINVAL;
+
+	return 0;
+}
+
 /*! Match the codec preferences from local config with a received codec preferences IEs received from the
  * MSC and the BTS' codec configuration.
  *  \param[out] ch_mode_rate resulting codec and rate information
@@ -283,7 +324,6 @@
 	uint8_t perm_spch;
 	bool match = false;
 	const struct gsm0808_speech_codec *sc_match = NULL;
-	uint16_t amr_s15_s0_supported;
 	int rc;
 
 	/* Note: Normally the MSC should never try to advertise a codec that
@@ -313,10 +353,22 @@
 
 		/* Match the permitted speech value against the codec lists that were
 		 * advertised by the MS and the MSC */
-		if (test_codec_pref(&sc_match, scl, ct, perm_spch)) {
-			match = true;
-			break;
+		if (!test_codec_pref(&sc_match, scl, ct, perm_spch)) {
+			continue;
 		}
+
+		/* Special handling for AMR */
+		if (perm_spch == GSM0808_PERM_HR3 || perm_spch == GSM0808_PERM_FR3) {
+			rc = match_amr_s15_s0(ch_mode_rate, msc, bts, sc_match, perm_spch);
+			if (rc < 0) {
+				ch_mode_rate->s15_s0 = 0;
+				continue;
+			}
+		} else
+			ch_mode_rate->s15_s0 = 0;
+
+		match = true;
+		break;
 	}
 
 	/* Exit without result, in case no match can be deteched */
@@ -330,31 +382,6 @@
 	/* Lookup a channel mode for the selected codec */
 	ch_mode_rate->chan_mode = gsm88_to_chan_mode(perm_spch);
 
-	/* Special handling for AMR */
-	if (perm_spch == GSM0808_PERM_HR3 || perm_spch == GSM0808_PERM_FR3) {
-		/* Normally the MSC should never try to advertise an AMR codec
-		 * configuration that we did not previously advertise as
-		 * supported. However, to ensure that no unsupported AMR codec
-		 * configuration enters the further processing steps we again
-		 * lookup what we support and generate an intersection. All
-		 * further processing is then done with this intersection
-		 * result */
-		amr_s15_s0_supported = gen_bss_supported_amr_s15_s0(msc, bts, (perm_spch == GSM0808_PERM_HR3));
-		if (sc_match)
-			ch_mode_rate->s15_s0 = sc_match->cfg & amr_s15_s0_supported;
-		else
-			ch_mode_rate->s15_s0 = amr_s15_s0_supported;
-
-		/* NOTE: The function test_codec_pref() will populate the
-		 * sc_match pointer from the searched speech codec list. For
-		 * AoIP based networks, no speech codec list will be present
-		 * and therefore no sc_match will be populated. For those
-		 * cases only the local configuration will influence s15_s0.
-		 * However s15_s0 is always populated with a meaningful value,
-		 * regardless if AoIP is in use or not. */
-	} else
-		ch_mode_rate->s15_s0 = 0;
-
 	return 0;
 }
 
diff --git a/tests/codec_pref/codec_pref_test.ok b/tests/codec_pref/codec_pref_test.ok
index 16d86ba..11b9ede 100644
--- a/tests/codec_pref/codec_pref_test.ok
+++ b/tests/codec_pref/codec_pref_test.ok
@@ -54,7 +54,7 @@
    codec->hr=0
    codec->efr=0
    codec->amr=1
- * result: rc=0, full_rate=1, s15_s0=57ff, chan_mode=SPEECH_AMR
+ * result: rc=0, full_rate=1, s15_s0=5702, chan_mode=SPEECH_AMR
 
 Determining channel mode and rate:
  * MS: speech codec list (1 items):
@@ -68,7 +68,7 @@
    codec->hr=0
    codec->efr=0
    codec->amr=1
- * result: rc=0, full_rate=0, s15_s0=073f, chan_mode=SPEECH_AMR
+ * result: rc=0, full_rate=0, s15_s0=0702, chan_mode=SPEECH_AMR
 
 Determining channel mode and rate:
  * MS: speech codec list (2 items):
@@ -241,7 +241,7 @@
    codec->hr=1
    codec->efr=1
    codec->amr=1
- * result: rc=0, full_rate=1, s15_s0=57ff, chan_mode=SPEECH_AMR
+ * result: rc=0, full_rate=1, s15_s0=5702, chan_mode=SPEECH_AMR
 
 Determining channel mode and rate:
  * MS: speech codec list (1 items):
@@ -263,7 +263,7 @@
    codec->hr=1
    codec->efr=1
    codec->amr=1
- * result: rc=0, full_rate=0, s15_s0=073f, chan_mode=SPEECH_AMR
+ * result: rc=0, full_rate=0, s15_s0=0702, chan_mode=SPEECH_AMR
 
 Determining channel mode and rate:
  * MS: speech codec list (2 items):
@@ -450,7 +450,7 @@
    codec->hr=1
    codec->efr=1
    codec->amr=1
- * result: rc=0, full_rate=1, s15_s0=57ff, chan_mode=SPEECH_AMR
+ * result: rc=0, full_rate=1, s15_s0=5702, chan_mode=SPEECH_AMR
 
 Determining channel mode and rate:
  * MS: speech codec list (5 items):
@@ -472,7 +472,7 @@
    codec->hr=1
    codec->efr=1
    codec->amr=1
- * result: rc=0, full_rate=0, s15_s0=073f, chan_mode=SPEECH_AMR
+ * result: rc=0, full_rate=0, s15_s0=0702, chan_mode=SPEECH_AMR
 
 Determining channel mode and rate:
  * MS: speech codec list (5 items):

-- 
To view, visit https://gerrit.osmocom.org/13093
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings

Gerrit-Project: osmo-bsc
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: Ie52376b51fe07ed07056e8df2e9557293ff67a78
Gerrit-Change-Number: 13093
Gerrit-PatchSet: 10
Gerrit-Owner: dexter <pmaier at sysmocom.de>
Gerrit-Reviewer: Harald Welte <laforge at gnumonks.org>
Gerrit-Reviewer: Jenkins Builder (1000002)
Gerrit-Reviewer: dexter <pmaier at sysmocom.de>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.osmocom.org/pipermail/gerrit-log/attachments/20190319/e3a49175/attachment.htm>


More information about the gerrit-log mailing list