From 3713af865503f78ad1a49604dc5d39908b94b2be Mon Sep 17 00:00:00 2001 From: Philipp Maier Date: Wed, 27 Feb 2019 16:48:25 +0100 Subject: gsm0808_utils: fix gsm48 multirate configuration generator The function gsm0808_sc_cfg_from_gsm48_mr_cfg() takes an S15 to S0 bitmask and converts that bitmask into an AMR multirate configuration struct. Unfortunately the current implementation implements 3GPP TS 28.062, Table 7.11.3.1.3-2 wrongly in some aspects. Lets fix this. - Fix wrong interpretation of the bitpatterns - 5,15K is invalid and must never be selected - Make sure that no more than 4 rates are selected in the active set - Extend unit-test Change-Id: I6fd7f4073b84093742c322752f2fd878d1071e15 Related: SYS#4470 --- include/osmocom/gsm/gsm0808_utils.h | 2 +- include/osmocom/gsm/protocol/gsm_08_08.h | 14 ++ src/gsm/gsm0808_utils.c | 94 +++++++++++--- tests/gsm0808/gsm0808_test.c | 47 +++++-- tests/gsm0808/gsm0808_test.ok | 213 +++++++++++++++++++++++++++---- 5 files changed, 313 insertions(+), 57 deletions(-) diff --git a/include/osmocom/gsm/gsm0808_utils.h b/include/osmocom/gsm/gsm0808_utils.h index 53f145c5..dedb0298 100644 --- a/include/osmocom/gsm/gsm0808_utils.h +++ b/include/osmocom/gsm/gsm0808_utils.h @@ -130,7 +130,7 @@ int gsm0808_chan_type_to_speech_codec(uint8_t perm_spch); int gsm0808_speech_codec_from_chan_type(struct gsm0808_speech_codec *sc, uint8_t perm_spch); uint16_t gsm0808_sc_cfg_from_gsm48_mr_cfg(const struct gsm48_multi_rate_conf *cfg, bool fr); -void gsm48_mr_cfg_from_gsm0808_sc_cfg(struct gsm48_multi_rate_conf *cfg, uint16_t s15_s0); +int gsm48_mr_cfg_from_gsm0808_sc_cfg(struct gsm48_multi_rate_conf *cfg, uint16_t s15_s0); /*! \returns 3GPP TS 08.08 ยง3.2.2.5 Class of a given Cause */ static inline enum gsm0808_cause_class gsm0808_cause_class(enum gsm0808_cause cause) diff --git a/include/osmocom/gsm/protocol/gsm_08_08.h b/include/osmocom/gsm/protocol/gsm_08_08.h index 67dc6ee1..aa01ee5c 100644 --- a/include/osmocom/gsm/protocol/gsm_08_08.h +++ b/include/osmocom/gsm/protocol/gsm_08_08.h @@ -553,6 +553,20 @@ enum gsm0808_speech_codec_rate_defaults { GSM0808_SC_CFG_DEFAULT_AMR_12_2 = 0xc082 }; +/*! Single speech codec configurations broken down by reate. + * See also: 3GPP TS 28.062, Table 7.11.3.1.3-2: Preferred Configurations for + * the Adaptive Multi-Rate Codec Types. */ +enum gsm0808_speech_codec_rate { + GSM0808_SC_CFG_AMR_4_75 = 0x0001, + GSM0808_SC_CFG_AMR_4_75_5_90_7_40_12_20 = 0x0002, + GSM0808_SC_CFG_AMR_5_90 = 0x0004, + GSM0808_SC_CFG_AMR_6_70 = 0x0008, + GSM0808_SC_CFG_AMR_7_40 = 0x0010, + GSM0808_SC_CFG_AMR_7_95 = 0x0020, + GSM0808_SC_CFG_AMR_10_2 = 0x0040, + GSM0808_SC_CFG_AMR_12_2 = 0x0080, +}; + /* 3GPP TS 48.008 3.2.2.103 Speech Codec List */ #define SPEECH_CODEC_MAXLEN 255 struct gsm0808_speech_codec_list { diff --git a/src/gsm/gsm0808_utils.c b/src/gsm/gsm0808_utils.c index dd14d3ca..2552c086 100644 --- a/src/gsm/gsm0808_utils.c +++ b/src/gsm/gsm0808_utils.c @@ -1346,42 +1346,94 @@ uint16_t gsm0808_sc_cfg_from_gsm48_mr_cfg(const struct gsm48_multi_rate_conf *cf /*! Determine a GSM 04.08 AMR configuration struct from a set of speech codec * configuration bits (S0-S15) * \param[out] cfg AMR configuration in GSM 04.08 format. - * \param[in] s15_s0 configuration bits (S0-S15). */ -void gsm48_mr_cfg_from_gsm0808_sc_cfg(struct gsm48_multi_rate_conf *cfg, - uint16_t s15_s0) + * \param[in] s15_s0 configuration bits (S15-S0, non-ambiguous). + * \returns zero when successful; negative on error */ +int gsm48_mr_cfg_from_gsm0808_sc_cfg(struct gsm48_multi_rate_conf *cfg, + uint16_t s15_s0) { + unsigned int count = 0; + + /* Note: See also: 3GPP TS 28.062 + * Table 7.11.3.1.3-2: Preferred Configurations for the Adaptive + * Multi-Rate Codec Types */ + + /* Note: The resulting multirate-configuration must not contain an + * active set of more than four codec rates. The active set also + * must contain at least one rate. */ + memset(cfg, 0, sizeof(*cfg)); + cfg->ver = 1; + cfg->icmi = 1; /* Strip option bits */ s15_s0 &= 0x00ff; - /* Rate 5,15k must always be present */ - cfg->m5_15 = 1; + /* Rate 5,15k can never be selected (see table) */ + cfg->m5_15 = 0; + + if (s15_s0 & GSM0808_SC_CFG_AMR_4_75_5_90_7_40_12_20 & 0xff) { + /* Table Table 7.11.3.1.3-2 lists one mode that selects 4 + * rates at once (Config-NB-Code = 1). The rates selected + * are known to be compatible between GERAN and UTRAN, since + * an active set must not contain more than four rates at + * a time, we ignore all other settings as they are either + * redundaned or excess settings (invalid) */ + cfg->m4_75 = 1; + cfg->m5_90 = 1; + cfg->m7_40 = 1; + cfg->m12_2 = 1; + count += 4; + } - if ((s15_s0 & GSM0808_SC_CFG_DEFAULT_AMR_4_75 & 0xff) == - (GSM0808_SC_CFG_DEFAULT_AMR_4_75 & 0xff)) + /* Check the bits in s15_s0 and set the flags for the + * respective rates. */ + if (s15_s0 & GSM0808_SC_CFG_AMR_4_75 && !cfg->m4_75) { + if (count >= 4) + return -EINVAL; cfg->m4_75 = 1; - if ((s15_s0 & GSM0808_SC_CFG_DEFAULT_AMR_5_90 & 0xff) == - (GSM0808_SC_CFG_DEFAULT_AMR_5_90 & 0xff)) + count++; + } + if (s15_s0 & GSM0808_SC_CFG_AMR_5_90 && !cfg->m5_90) { + if (count >= 4) + return -EINVAL; cfg->m5_90 = 1; - if ((s15_s0 & GSM0808_SC_CFG_DEFAULT_AMR_6_70 & 0xff) == - (GSM0808_SC_CFG_DEFAULT_AMR_6_70 & 0xff)) + count++; + } + if (s15_s0 & GSM0808_SC_CFG_AMR_6_70) { + if (count >= 4) + return -EINVAL; cfg->m6_70 = 1; - if ((s15_s0 & GSM0808_SC_CFG_DEFAULT_AMR_7_40 & 0xff) == - (GSM0808_SC_CFG_DEFAULT_AMR_7_40 & 0xff)) + count++; + } + if (s15_s0 & GSM0808_SC_CFG_AMR_7_40 && !cfg->m7_40) { + if (count >= 4) + return -EINVAL; cfg->m7_40 = 1; - if ((s15_s0 & GSM0808_SC_CFG_DEFAULT_AMR_7_95 & 0xff) == - (GSM0808_SC_CFG_DEFAULT_AMR_7_95 & 0xff)) + count++; + } + if (s15_s0 & GSM0808_SC_CFG_AMR_7_95) { + if (count >= 4) + return -EINVAL; cfg->m7_95 = 1; - if ((s15_s0 & GSM0808_SC_CFG_DEFAULT_AMR_10_2 & 0xff) == - (GSM0808_SC_CFG_DEFAULT_AMR_10_2 & 0xff)) + count++; + } + if (s15_s0 & GSM0808_SC_CFG_AMR_10_2) { + if (count >= 4) + return -EINVAL; cfg->m10_2 = 1; - if ((s15_s0 & GSM0808_SC_CFG_DEFAULT_AMR_12_2 & 0xff) == - (GSM0808_SC_CFG_DEFAULT_AMR_12_2 & 0xff)) + count++; + } + if (s15_s0 & GSM0808_SC_CFG_AMR_12_2 && !cfg->m12_2) { + if (count >= 4) + return -EINVAL; cfg->m12_2 = 1; + count++; + } - cfg->ver = 1; - cfg->icmi = 1; + if (count == 0) + return -EINVAL; + + return 0; } int gsm0808_get_cipher_reject_cause(const struct tlv_parsed *tp) diff --git a/tests/gsm0808/gsm0808_test.c b/tests/gsm0808/gsm0808_test.c index c448f439..87f5d2a5 100644 --- a/tests/gsm0808/gsm0808_test.c +++ b/tests/gsm0808/gsm0808_test.c @@ -1907,12 +1907,13 @@ static void test_gsm0808_sc_cfg_from_gsm48_mr_cfg(void) static void test_gsm48_mr_cfg_from_gsm0808_sc_cfg_single(uint16_t s15_s0) { struct gsm48_multi_rate_conf cfg; + int rc; printf("Input:\n"); printf(" S15-S0 = %04x = 0b" OSMO_BIN_SPEC OSMO_BIN_SPEC "\n", s15_s0, OSMO_BIN_PRINT(s15_s0 >> 8), OSMO_BIN_PRINT(s15_s0)); - gsm48_mr_cfg_from_gsm0808_sc_cfg(&cfg, s15_s0); + rc = gsm48_mr_cfg_from_gsm0808_sc_cfg(&cfg, s15_s0); printf("Output:\n"); printf(" m4_75= %u smod= %u\n", cfg.m4_75, cfg.smod); @@ -1924,6 +1925,9 @@ static void test_gsm48_mr_cfg_from_gsm0808_sc_cfg_single(uint16_t s15_s0) printf(" m10_2= %u\n", cfg.m10_2); printf(" m12_2= %u\n", cfg.m12_2); + if (rc != 0) + printf(" Result invalid!\n"); + printf("\n"); } @@ -1931,7 +1935,8 @@ void test_gsm48_mr_cfg_from_gsm0808_sc_cfg() { printf("Testing gsm48_mr_cfg_from_gsm0808_sc_cfg():\n"); - /* Only one codec per setting */ + /* Test with settings as defined in 3GPP TS 28.062, Table 7.11.3.1.3-2, + * (up to four codecs may become selected) */ test_gsm48_mr_cfg_from_gsm0808_sc_cfg_single (GSM0808_SC_CFG_DEFAULT_AMR_4_75); test_gsm48_mr_cfg_from_gsm0808_sc_cfg_single @@ -1949,15 +1954,40 @@ void test_gsm48_mr_cfg_from_gsm0808_sc_cfg() test_gsm48_mr_cfg_from_gsm0808_sc_cfg_single (GSM0808_SC_CFG_DEFAULT_AMR_12_2); - /* Combinations */ + /* Test with settings as defined in 3GPP TS 28.062, Table 7.11.3.1.3-2, + * but pick only one distinctive setting at a time */ + test_gsm48_mr_cfg_from_gsm0808_sc_cfg_single(GSM0808_SC_CFG_AMR_4_75); test_gsm48_mr_cfg_from_gsm0808_sc_cfg_single - (GSM0808_SC_CFG_DEFAULT_AMR_4_75 | GSM0808_SC_CFG_DEFAULT_AMR_6_70 | - GSM0808_SC_CFG_DEFAULT_AMR_10_2); + (GSM0808_SC_CFG_AMR_4_75_5_90_7_40_12_20); + test_gsm48_mr_cfg_from_gsm0808_sc_cfg_single(GSM0808_SC_CFG_AMR_5_90); + test_gsm48_mr_cfg_from_gsm0808_sc_cfg_single(GSM0808_SC_CFG_AMR_6_70); + test_gsm48_mr_cfg_from_gsm0808_sc_cfg_single(GSM0808_SC_CFG_AMR_7_40); + test_gsm48_mr_cfg_from_gsm0808_sc_cfg_single(GSM0808_SC_CFG_AMR_7_95); + test_gsm48_mr_cfg_from_gsm0808_sc_cfg_single(GSM0808_SC_CFG_AMR_10_2); + test_gsm48_mr_cfg_from_gsm0808_sc_cfg_single(GSM0808_SC_CFG_AMR_12_2); + + /* Arbitrary, but valid combinations */ + test_gsm48_mr_cfg_from_gsm0808_sc_cfg_single(GSM0808_SC_CFG_AMR_7_40 | + GSM0808_SC_CFG_AMR_6_70 | + GSM0808_SC_CFG_AMR_10_2); + test_gsm48_mr_cfg_from_gsm0808_sc_cfg_single(GSM0808_SC_CFG_AMR_7_95 | + GSM0808_SC_CFG_AMR_4_75); + test_gsm48_mr_cfg_from_gsm0808_sc_cfg_single(GSM0808_SC_CFG_AMR_5_90 | + GSM0808_SC_CFG_AMR_12_2); test_gsm48_mr_cfg_from_gsm0808_sc_cfg_single - (GSM0808_SC_CFG_DEFAULT_AMR_10_2 | GSM0808_SC_CFG_DEFAULT_AMR_12_2 | - GSM0808_SC_CFG_DEFAULT_AMR_7_40); + (GSM0808_SC_CFG_AMR_4_75_5_90_7_40_12_20 | GSM0808_SC_CFG_AMR_5_90 | + GSM0808_SC_CFG_AMR_12_2); + + /* Invalid combinations */ test_gsm48_mr_cfg_from_gsm0808_sc_cfg_single - (GSM0808_SC_CFG_DEFAULT_AMR_7_95 | GSM0808_SC_CFG_DEFAULT_AMR_12_2); + (GSM0808_SC_CFG_AMR_4_75_5_90_7_40_12_20 | GSM0808_SC_CFG_AMR_6_70); + test_gsm48_mr_cfg_from_gsm0808_sc_cfg_single(GSM0808_SC_CFG_AMR_7_40 | + GSM0808_SC_CFG_AMR_6_70 | + GSM0808_SC_CFG_AMR_10_2 | + GSM0808_SC_CFG_AMR_7_95 | + GSM0808_SC_CFG_AMR_4_75); + test_gsm48_mr_cfg_from_gsm0808_sc_cfg_single(0x0000); + test_gsm48_mr_cfg_from_gsm0808_sc_cfg_single(0xffff); } struct test_cell_id_matching_data { @@ -2359,7 +2389,6 @@ int main(int argc, char **argv) test_gsm0808_enc_dec_cell_id_ci(); test_gsm0808_enc_dec_cell_id_lac_and_ci(); test_gsm0808_enc_dec_cell_id_global(); - test_gsm0808_sc_cfg_from_gsm48_mr_cfg(); test_gsm48_mr_cfg_from_gsm0808_sc_cfg(); diff --git a/tests/gsm0808/gsm0808_test.ok b/tests/gsm0808/gsm0808_test.ok index 60353262..9fce0e8e 100644 --- a/tests/gsm0808/gsm0808_test.ok +++ b/tests/gsm0808/gsm0808_test.ok @@ -315,43 +315,44 @@ Input: S15-S0 = ff03 = 0b1111111100000011 Output: m4_75= 1 smod= 0 - m5_15= 1 spare= 0 - m5_90= 0 icmi= 1 + m5_15= 0 spare= 0 + m5_90= 1 icmi= 1 m6_70= 0 nscb= 0 - m7_40= 0 ver= 1 + m7_40= 1 ver= 1 m7_95= 0 m10_2= 0 - m12_2= 0 + m12_2= 1 Input: S15-S0 = 0000 = 0b0000000000000000 Output: m4_75= 0 smod= 0 - m5_15= 1 spare= 0 + m5_15= 0 spare= 0 m5_90= 0 icmi= 1 m6_70= 0 nscb= 0 m7_40= 0 ver= 1 m7_95= 0 m10_2= 0 m12_2= 0 + Result invalid! Input: S15-S0 = ff06 = 0b1111111100000110 Output: - m4_75= 0 smod= 0 - m5_15= 1 spare= 0 + m4_75= 1 smod= 0 + m5_15= 0 spare= 0 m5_90= 1 icmi= 1 m6_70= 0 nscb= 0 - m7_40= 0 ver= 1 + m7_40= 1 ver= 1 m7_95= 0 m10_2= 0 - m12_2= 0 + m12_2= 1 Input: S15-S0 = 3e08 = 0b0011111000001000 Output: m4_75= 0 smod= 0 - m5_15= 1 spare= 0 + m5_15= 0 spare= 0 m5_90= 0 icmi= 1 m6_70= 1 nscb= 0 m7_40= 0 ver= 1 @@ -362,20 +363,20 @@ Output: Input: S15-S0 = 0c12 = 0b0000110000010010 Output: - m4_75= 0 smod= 0 - m5_15= 1 spare= 0 - m5_90= 0 icmi= 1 + m4_75= 1 smod= 0 + m5_15= 0 spare= 0 + m5_90= 1 icmi= 1 m6_70= 0 nscb= 0 m7_40= 1 ver= 1 m7_95= 0 m10_2= 0 - m12_2= 0 + m12_2= 1 Input: S15-S0 = c020 = 0b1100000000100000 Output: m4_75= 0 smod= 0 - m5_15= 1 spare= 0 + m5_15= 0 spare= 0 m5_90= 0 icmi= 1 m6_70= 0 nscb= 0 m7_40= 0 ver= 1 @@ -387,7 +388,7 @@ Input: S15-S0 = 3040 = 0b0011000001000000 Output: m4_75= 0 smod= 0 - m5_15= 1 spare= 0 + m5_15= 0 spare= 0 m5_90= 0 icmi= 1 m6_70= 0 nscb= 0 m7_40= 0 ver= 1 @@ -398,50 +399,210 @@ Output: Input: S15-S0 = c082 = 0b1100000010000010 Output: - m4_75= 0 smod= 0 - m5_15= 1 spare= 0 + m4_75= 1 smod= 0 + m5_15= 0 spare= 0 + m5_90= 1 icmi= 1 + m6_70= 0 nscb= 0 + m7_40= 1 ver= 1 + m7_95= 0 + m10_2= 0 + m12_2= 1 + +Input: + S15-S0 = 0001 = 0b0000000000000001 +Output: + m4_75= 1 smod= 0 + m5_15= 0 spare= 0 m5_90= 0 icmi= 1 m6_70= 0 nscb= 0 m7_40= 0 ver= 1 m7_95= 0 m10_2= 0 - m12_2= 1 + m12_2= 0 Input: - S15-S0 = ff4b = 0b1111111101001011 + S15-S0 = 0002 = 0b0000000000000010 Output: m4_75= 1 smod= 0 - m5_15= 1 spare= 0 + m5_15= 0 spare= 0 + m5_90= 1 icmi= 1 + m6_70= 0 nscb= 0 + m7_40= 1 ver= 1 + m7_95= 0 + m10_2= 0 + m12_2= 1 + +Input: + S15-S0 = 0004 = 0b0000000000000100 +Output: + m4_75= 0 smod= 0 + m5_15= 0 spare= 0 + m5_90= 1 icmi= 1 + m6_70= 0 nscb= 0 + m7_40= 0 ver= 1 + m7_95= 0 + m10_2= 0 + m12_2= 0 + +Input: + S15-S0 = 0008 = 0b0000000000001000 +Output: + m4_75= 0 smod= 0 + m5_15= 0 spare= 0 m5_90= 0 icmi= 1 m6_70= 1 nscb= 0 m7_40= 0 ver= 1 m7_95= 0 - m10_2= 1 + m10_2= 0 m12_2= 0 Input: - S15-S0 = fcd2 = 0b1111110011010010 + S15-S0 = 0010 = 0b0000000000010000 Output: m4_75= 0 smod= 0 - m5_15= 1 spare= 0 + m5_15= 0 spare= 0 m5_90= 0 icmi= 1 m6_70= 0 nscb= 0 m7_40= 1 ver= 1 m7_95= 0 + m10_2= 0 + m12_2= 0 + +Input: + S15-S0 = 0020 = 0b0000000000100000 +Output: + m4_75= 0 smod= 0 + m5_15= 0 spare= 0 + m5_90= 0 icmi= 1 + m6_70= 0 nscb= 0 + m7_40= 0 ver= 1 + m7_95= 1 + m10_2= 0 + m12_2= 0 + +Input: + S15-S0 = 0040 = 0b0000000001000000 +Output: + m4_75= 0 smod= 0 + m5_15= 0 spare= 0 + m5_90= 0 icmi= 1 + m6_70= 0 nscb= 0 + m7_40= 0 ver= 1 + m7_95= 0 m10_2= 1 + m12_2= 0 + +Input: + S15-S0 = 0080 = 0b0000000010000000 +Output: + m4_75= 0 smod= 0 + m5_15= 0 spare= 0 + m5_90= 0 icmi= 1 + m6_70= 0 nscb= 0 + m7_40= 0 ver= 1 + m7_95= 0 + m10_2= 0 m12_2= 1 Input: - S15-S0 = c0a2 = 0b1100000010100010 + S15-S0 = 0058 = 0b0000000001011000 Output: m4_75= 0 smod= 0 - m5_15= 1 spare= 0 + m5_15= 0 spare= 0 + m5_90= 0 icmi= 1 + m6_70= 1 nscb= 0 + m7_40= 1 ver= 1 + m7_95= 0 + m10_2= 1 + m12_2= 0 + +Input: + S15-S0 = 0021 = 0b0000000000100001 +Output: + m4_75= 1 smod= 0 + m5_15= 0 spare= 0 m5_90= 0 icmi= 1 m6_70= 0 nscb= 0 m7_40= 0 ver= 1 m7_95= 1 m10_2= 0 + m12_2= 0 + +Input: + S15-S0 = 0084 = 0b0000000010000100 +Output: + m4_75= 0 smod= 0 + m5_15= 0 spare= 0 + m5_90= 1 icmi= 1 + m6_70= 0 nscb= 0 + m7_40= 0 ver= 1 + m7_95= 0 + m10_2= 0 + m12_2= 1 + +Input: + S15-S0 = 0086 = 0b0000000010000110 +Output: + m4_75= 1 smod= 0 + m5_15= 0 spare= 0 + m5_90= 1 icmi= 1 + m6_70= 0 nscb= 0 + m7_40= 1 ver= 1 + m7_95= 0 + m10_2= 0 + m12_2= 1 + +Input: + S15-S0 = 000a = 0b0000000000001010 +Output: + m4_75= 1 smod= 0 + m5_15= 0 spare= 0 + m5_90= 1 icmi= 1 + m6_70= 0 nscb= 0 + m7_40= 1 ver= 1 + m7_95= 0 + m10_2= 0 + m12_2= 1 + Result invalid! + +Input: + S15-S0 = 0079 = 0b0000000001111001 +Output: + m4_75= 1 smod= 0 + m5_15= 0 spare= 0 + m5_90= 0 icmi= 1 + m6_70= 1 nscb= 0 + m7_40= 1 ver= 1 + m7_95= 1 + m10_2= 0 + m12_2= 0 + Result invalid! + +Input: + S15-S0 = 0000 = 0b0000000000000000 +Output: + m4_75= 0 smod= 0 + m5_15= 0 spare= 0 + m5_90= 0 icmi= 1 + m6_70= 0 nscb= 0 + m7_40= 0 ver= 1 + m7_95= 0 + m10_2= 0 + m12_2= 0 + Result invalid! + +Input: + S15-S0 = ffff = 0b1111111111111111 +Output: + m4_75= 1 smod= 0 + m5_15= 0 spare= 0 + m5_90= 1 icmi= 1 + m6_70= 0 nscb= 0 + m7_40= 1 ver= 1 + m7_95= 0 + m10_2= 0 m12_2= 1 + Result invalid! test_cell_id_matching -- cgit v1.2.3