<p>Harald Welte <strong>merged</strong> this change.</p><p><a href="https://gerrit.osmocom.org/13093">View Change</a></p><div style="white-space:pre-wrap">Approvals:
Harald Welte: Looks good to me, approved
Jenkins Builder: Verified
</div><pre style="font-family: monospace,monospace; white-space: pre-wrap;">codec_pref: fix special handling for AMR rate configuration (S15-S0)<br><br>When match_codec_pref() is called and the codec selections from the<br>ASSIGNMENT COMMAND are matched against the interal capabilities, the<br>configurations are checked one by one. When a match is found that match<br>is returned.<br><br>However, the implementation currently does not check the AMR S15-S0 bits<br>when the actual matching happens. This is done afterwards in case AMR<br>gets picked. Unfortunately if the MSC implementation is not obeying the<br>settings the MSC has previously communicated in the L3 COMPL message we<br>may end up with an S15-S0 configuration that has all rate selection<br>(which eventually end up as active set in RSL) bits set to zero. This is<br>an invalid configuration and should be prevented. Also the handling of<br>the S15-S0 bits should happen as part of the matching so that there is a<br>chance to try the nect codec in the list if AMR is unuseable.<br><br>Also S15-S0 has one special setting "Config-NB-Code = 1" (S1 = 1). When<br>this bit is set, the four (in HR only three) most common rates are<br>selected into the active set. If there are also other S-bits set besides<br>S1 we should prefer S1 and discard the other bits.<br><br>- Perform handling of S15-S0 while matching<br>- If S1 is set, prefer this setting and discard all other settings<br><br>Change-Id: Ie52376b51fe07ed07056e8df2e9557293ff67a78<br>Related: SYS#4470<br>---<br>M src/osmo-bsc/codec_pref.c<br>M tests/codec_pref/codec_pref_test.ok<br>2 files changed, 62 insertions(+), 35 deletions(-)<br><br></pre><pre style="font-family: monospace,monospace; white-space: pre-wrap;"><span>diff --git a/src/osmo-bsc/codec_pref.c b/src/osmo-bsc/codec_pref.c</span><br><span>index a94d6a8..412ed9a 100644</span><br><span>--- a/src/osmo-bsc/codec_pref.c</span><br><span>+++ b/src/osmo-bsc/codec_pref.c</span><br><span>@@ -264,6 +264,47 @@</span><br><span> return amr_s15_s0_bts & amr_s15_s0_msc;</span><br><span> }</span><br><span> </span><br><span style="color: hsl(120, 100%, 40%);">+/* Special handling for AMR rate configuration bits (S15-S0) */</span><br><span style="color: hsl(120, 100%, 40%);">+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)</span><br><span style="color: hsl(120, 100%, 40%);">+{</span><br><span style="color: hsl(120, 100%, 40%);">+ uint16_t amr_s15_s0_supported;</span><br><span style="color: hsl(120, 100%, 40%);">+ </span><br><span style="color: hsl(120, 100%, 40%);">+ /* Normally the MSC should never try to advertise an AMR codec</span><br><span style="color: hsl(120, 100%, 40%);">+ * configuration that we did not previously advertised as supported.</span><br><span style="color: hsl(120, 100%, 40%);">+ * However, to ensure that no unsupported AMR codec configuration</span><br><span style="color: hsl(120, 100%, 40%);">+ * enters the further processing steps we again lookup what we support</span><br><span style="color: hsl(120, 100%, 40%);">+ * and generate an intersection. All further processing is then done</span><br><span style="color: hsl(120, 100%, 40%);">+ * with this intersection result. At the same time we will make sure</span><br><span style="color: hsl(120, 100%, 40%);">+ * that the intersection contains at least one rate setting. */</span><br><span style="color: hsl(120, 100%, 40%);">+ </span><br><span style="color: hsl(120, 100%, 40%);">+ amr_s15_s0_supported = gen_bss_supported_amr_s15_s0(msc, bts, (perm_spch == GSM0808_PERM_HR3));</span><br><span style="color: hsl(120, 100%, 40%);">+</span><br><span style="color: hsl(120, 100%, 40%);">+ /* NOTE: The sc_match pointer points to a speech codec from the speech</span><br><span style="color: hsl(120, 100%, 40%);">+ * codec list that has been communicated with the ASSIGNMENT COMMAND.</span><br><span style="color: hsl(120, 100%, 40%);">+ * However, only AoIP based networks will include a speech codec list</span><br><span style="color: hsl(120, 100%, 40%);">+ * into the ASSIGNMENT COMMAND. For non AoIP based networks, no speech</span><br><span style="color: hsl(120, 100%, 40%);">+ * codec (sc_match) will be available, so we will fully rely on the</span><br><span style="color: hsl(120, 100%, 40%);">+ * local configuration for thoses cases. */</span><br><span style="color: hsl(120, 100%, 40%);">+ if (sc_match)</span><br><span style="color: hsl(120, 100%, 40%);">+ ch_mode_rate->s15_s0 = sc_match->cfg & amr_s15_s0_supported;</span><br><span style="color: hsl(120, 100%, 40%);">+ else</span><br><span style="color: hsl(120, 100%, 40%);">+ ch_mode_rate->s15_s0 = amr_s15_s0_supported;</span><br><span style="color: hsl(120, 100%, 40%);">+</span><br><span style="color: hsl(120, 100%, 40%);">+ /* Prefer "Config-NB-Code = 1" (S1) over all other AMR rates setttings.</span><br><span style="color: hsl(120, 100%, 40%);">+ * When S1 is set, the active set will automatically include 12.2k, 7.4k,</span><br><span style="color: hsl(120, 100%, 40%);">+ * 5.9k, 4.75k, in case of HR 12,2k is left out. */</span><br><span style="color: hsl(120, 100%, 40%);">+ if (ch_mode_rate->s15_s0 & GSM0808_SC_CFG_AMR_4_75_5_90_7_40_12_20) {</span><br><span style="color: hsl(120, 100%, 40%);">+ ch_mode_rate->s15_s0 &= 0xff00;</span><br><span style="color: hsl(120, 100%, 40%);">+ ch_mode_rate->s15_s0 |= GSM0808_SC_CFG_AMR_4_75_5_90_7_40_12_20;</span><br><span style="color: hsl(120, 100%, 40%);">+ }</span><br><span style="color: hsl(120, 100%, 40%);">+</span><br><span style="color: hsl(120, 100%, 40%);">+ /* Make sure at least one rate is set. */</span><br><span style="color: hsl(120, 100%, 40%);">+ if ((ch_mode_rate->s15_s0 & 0x00ff) == 0x0000)</span><br><span style="color: hsl(120, 100%, 40%);">+ return -EINVAL;</span><br><span style="color: hsl(120, 100%, 40%);">+</span><br><span style="color: hsl(120, 100%, 40%);">+ return 0;</span><br><span style="color: hsl(120, 100%, 40%);">+}</span><br><span style="color: hsl(120, 100%, 40%);">+</span><br><span> /*! Match the codec preferences from local config with a received codec preferences IEs received from the</span><br><span> * MSC and the BTS' codec configuration.</span><br><span> * \param[out] ch_mode_rate resulting codec and rate information</span><br><span>@@ -283,7 +324,6 @@</span><br><span> uint8_t perm_spch;</span><br><span> bool match = false;</span><br><span> const struct gsm0808_speech_codec *sc_match = NULL;</span><br><span style="color: hsl(0, 100%, 40%);">- uint16_t amr_s15_s0_supported;</span><br><span> int rc;</span><br><span> </span><br><span> /* Note: Normally the MSC should never try to advertise a codec that</span><br><span>@@ -313,10 +353,22 @@</span><br><span> </span><br><span> /* Match the permitted speech value against the codec lists that were</span><br><span> * advertised by the MS and the MSC */</span><br><span style="color: hsl(0, 100%, 40%);">- if (test_codec_pref(&sc_match, scl, ct, perm_spch)) {</span><br><span style="color: hsl(0, 100%, 40%);">- match = true;</span><br><span style="color: hsl(0, 100%, 40%);">- break;</span><br><span style="color: hsl(120, 100%, 40%);">+ if (!test_codec_pref(&sc_match, scl, ct, perm_spch)) {</span><br><span style="color: hsl(120, 100%, 40%);">+ continue;</span><br><span> }</span><br><span style="color: hsl(120, 100%, 40%);">+</span><br><span style="color: hsl(120, 100%, 40%);">+ /* Special handling for AMR */</span><br><span style="color: hsl(120, 100%, 40%);">+ if (perm_spch == GSM0808_PERM_HR3 || perm_spch == GSM0808_PERM_FR3) {</span><br><span style="color: hsl(120, 100%, 40%);">+ rc = match_amr_s15_s0(ch_mode_rate, msc, bts, sc_match, perm_spch);</span><br><span style="color: hsl(120, 100%, 40%);">+ if (rc < 0) {</span><br><span style="color: hsl(120, 100%, 40%);">+ ch_mode_rate->s15_s0 = 0;</span><br><span style="color: hsl(120, 100%, 40%);">+ continue;</span><br><span style="color: hsl(120, 100%, 40%);">+ }</span><br><span style="color: hsl(120, 100%, 40%);">+ } else</span><br><span style="color: hsl(120, 100%, 40%);">+ ch_mode_rate->s15_s0 = 0;</span><br><span style="color: hsl(120, 100%, 40%);">+</span><br><span style="color: hsl(120, 100%, 40%);">+ match = true;</span><br><span style="color: hsl(120, 100%, 40%);">+ break;</span><br><span> }</span><br><span> </span><br><span> /* Exit without result, in case no match can be deteched */</span><br><span>@@ -330,31 +382,6 @@</span><br><span> /* Lookup a channel mode for the selected codec */</span><br><span> ch_mode_rate->chan_mode = gsm88_to_chan_mode(perm_spch);</span><br><span> </span><br><span style="color: hsl(0, 100%, 40%);">- /* Special handling for AMR */</span><br><span style="color: hsl(0, 100%, 40%);">- if (perm_spch == GSM0808_PERM_HR3 || perm_spch == GSM0808_PERM_FR3) {</span><br><span style="color: hsl(0, 100%, 40%);">- /* Normally the MSC should never try to advertise an AMR codec</span><br><span style="color: hsl(0, 100%, 40%);">- * configuration that we did not previously advertise as</span><br><span style="color: hsl(0, 100%, 40%);">- * supported. However, to ensure that no unsupported AMR codec</span><br><span style="color: hsl(0, 100%, 40%);">- * configuration enters the further processing steps we again</span><br><span style="color: hsl(0, 100%, 40%);">- * lookup what we support and generate an intersection. All</span><br><span style="color: hsl(0, 100%, 40%);">- * further processing is then done with this intersection</span><br><span style="color: hsl(0, 100%, 40%);">- * result */</span><br><span style="color: hsl(0, 100%, 40%);">- amr_s15_s0_supported = gen_bss_supported_amr_s15_s0(msc, bts, (perm_spch == GSM0808_PERM_HR3));</span><br><span style="color: hsl(0, 100%, 40%);">- if (sc_match)</span><br><span style="color: hsl(0, 100%, 40%);">- ch_mode_rate->s15_s0 = sc_match->cfg & amr_s15_s0_supported;</span><br><span style="color: hsl(0, 100%, 40%);">- else</span><br><span style="color: hsl(0, 100%, 40%);">- ch_mode_rate->s15_s0 = amr_s15_s0_supported;</span><br><span style="color: hsl(0, 100%, 40%);">-</span><br><span style="color: hsl(0, 100%, 40%);">- /* NOTE: The function test_codec_pref() will populate the</span><br><span style="color: hsl(0, 100%, 40%);">- * sc_match pointer from the searched speech codec list. For</span><br><span style="color: hsl(0, 100%, 40%);">- * AoIP based networks, no speech codec list will be present</span><br><span style="color: hsl(0, 100%, 40%);">- * and therefore no sc_match will be populated. For those</span><br><span style="color: hsl(0, 100%, 40%);">- * cases only the local configuration will influence s15_s0.</span><br><span style="color: hsl(0, 100%, 40%);">- * However s15_s0 is always populated with a meaningful value,</span><br><span style="color: hsl(0, 100%, 40%);">- * regardless if AoIP is in use or not. */</span><br><span style="color: hsl(0, 100%, 40%);">- } else</span><br><span style="color: hsl(0, 100%, 40%);">- ch_mode_rate->s15_s0 = 0;</span><br><span style="color: hsl(0, 100%, 40%);">-</span><br><span> return 0;</span><br><span> }</span><br><span> </span><br><span>diff --git a/tests/codec_pref/codec_pref_test.ok b/tests/codec_pref/codec_pref_test.ok</span><br><span>index 16d86ba..11b9ede 100644</span><br><span>--- a/tests/codec_pref/codec_pref_test.ok</span><br><span>+++ b/tests/codec_pref/codec_pref_test.ok</span><br><span>@@ -54,7 +54,7 @@</span><br><span> codec->hr=0</span><br><span> codec->efr=0</span><br><span> codec->amr=1</span><br><span style="color: hsl(0, 100%, 40%);">- * result: rc=0, full_rate=1, s15_s0=57ff, chan_mode=SPEECH_AMR</span><br><span style="color: hsl(120, 100%, 40%);">+ * result: rc=0, full_rate=1, s15_s0=5702, chan_mode=SPEECH_AMR</span><br><span> </span><br><span> Determining channel mode and rate:</span><br><span> * MS: speech codec list (1 items):</span><br><span>@@ -68,7 +68,7 @@</span><br><span> codec->hr=0</span><br><span> codec->efr=0</span><br><span> codec->amr=1</span><br><span style="color: hsl(0, 100%, 40%);">- * result: rc=0, full_rate=0, s15_s0=073f, chan_mode=SPEECH_AMR</span><br><span style="color: hsl(120, 100%, 40%);">+ * result: rc=0, full_rate=0, s15_s0=0702, chan_mode=SPEECH_AMR</span><br><span> </span><br><span> Determining channel mode and rate:</span><br><span> * MS: speech codec list (2 items):</span><br><span>@@ -241,7 +241,7 @@</span><br><span> codec->hr=1</span><br><span> codec->efr=1</span><br><span> codec->amr=1</span><br><span style="color: hsl(0, 100%, 40%);">- * result: rc=0, full_rate=1, s15_s0=57ff, chan_mode=SPEECH_AMR</span><br><span style="color: hsl(120, 100%, 40%);">+ * result: rc=0, full_rate=1, s15_s0=5702, chan_mode=SPEECH_AMR</span><br><span> </span><br><span> Determining channel mode and rate:</span><br><span> * MS: speech codec list (1 items):</span><br><span>@@ -263,7 +263,7 @@</span><br><span> codec->hr=1</span><br><span> codec->efr=1</span><br><span> codec->amr=1</span><br><span style="color: hsl(0, 100%, 40%);">- * result: rc=0, full_rate=0, s15_s0=073f, chan_mode=SPEECH_AMR</span><br><span style="color: hsl(120, 100%, 40%);">+ * result: rc=0, full_rate=0, s15_s0=0702, chan_mode=SPEECH_AMR</span><br><span> </span><br><span> Determining channel mode and rate:</span><br><span> * MS: speech codec list (2 items):</span><br><span>@@ -450,7 +450,7 @@</span><br><span> codec->hr=1</span><br><span> codec->efr=1</span><br><span> codec->amr=1</span><br><span style="color: hsl(0, 100%, 40%);">- * result: rc=0, full_rate=1, s15_s0=57ff, chan_mode=SPEECH_AMR</span><br><span style="color: hsl(120, 100%, 40%);">+ * result: rc=0, full_rate=1, s15_s0=5702, chan_mode=SPEECH_AMR</span><br><span> </span><br><span> Determining channel mode and rate:</span><br><span> * MS: speech codec list (5 items):</span><br><span>@@ -472,7 +472,7 @@</span><br><span> codec->hr=1</span><br><span> codec->efr=1</span><br><span> codec->amr=1</span><br><span style="color: hsl(0, 100%, 40%);">- * result: rc=0, full_rate=0, s15_s0=073f, chan_mode=SPEECH_AMR</span><br><span style="color: hsl(120, 100%, 40%);">+ * result: rc=0, full_rate=0, s15_s0=0702, chan_mode=SPEECH_AMR</span><br><span> </span><br><span> Determining channel mode and rate:</span><br><span> * MS: speech codec list (5 items):</span><br><span></span><br></pre><p>To view, visit <a href="https://gerrit.osmocom.org/13093">change 13093</a>. To unsubscribe, or for help writing mail filters, visit <a href="https://gerrit.osmocom.org/settings">settings</a>.</p><div itemscope itemtype="http://schema.org/EmailMessage"><div itemscope itemprop="action" itemtype="http://schema.org/ViewAction"><link itemprop="url" href="https://gerrit.osmocom.org/13093"/><meta itemprop="name" content="View Change"/></div></div>
<div style="display:none"> Gerrit-Project: osmo-bsc </div>
<div style="display:none"> Gerrit-Branch: master </div>
<div style="display:none"> Gerrit-MessageType: merged </div>
<div style="display:none"> Gerrit-Change-Id: Ie52376b51fe07ed07056e8df2e9557293ff67a78 </div>
<div style="display:none"> Gerrit-Change-Number: 13093 </div>
<div style="display:none"> Gerrit-PatchSet: 10 </div>
<div style="display:none"> Gerrit-Owner: dexter <pmaier@sysmocom.de> </div>
<div style="display:none"> Gerrit-Reviewer: Harald Welte <laforge@gnumonks.org> </div>
<div style="display:none"> Gerrit-Reviewer: Jenkins Builder (1000002) </div>
<div style="display:none"> Gerrit-Reviewer: dexter <pmaier@sysmocom.de> </div>