Speech codings which are not supported by BTS will be removed from the bearer capability information element after parsing. This way it is not required for the MNCC application to consider support of each BTS.
Only GSM full rate is supported by default. --- openbsc/include/openbsc/gsm_data_shared.h | 9 ++++ openbsc/src/libbsc/bsc_vty.c | 72 +++++++++++++++++++++++++++++ openbsc/src/libmsc/gsm_04_08.c | 28 +++++++++++ 3 files changed, 109 insertions(+), 0 deletions(-)
diff --git a/openbsc/include/openbsc/gsm_data_shared.h b/openbsc/include/openbsc/gsm_data_shared.h index 84d15ef..4a41498 100644 --- a/openbsc/include/openbsc/gsm_data_shared.h +++ b/openbsc/include/openbsc/gsm_data_shared.h @@ -143,6 +143,12 @@ struct bts_ul_meas { uint8_t inv_rssi; };
+struct bts_codec_conf { + uint8_t hr; + uint8_t efr; + uint8_t amr; +}; + struct amr_mode { uint8_t mode; uint8_t threshold; @@ -704,6 +710,9 @@ struct gsm_bts {
/* exclude the BTS from the global RF Lock handling */ int excl_from_rf_lock; + + /* supported codecs beside FR */ + struct bts_codec_conf codec; #endif /* ROLE_BSC */ void *role; }; diff --git a/openbsc/src/libbsc/bsc_vty.c b/openbsc/src/libbsc/bsc_vty.c index b034981..f444427 100644 --- a/openbsc/src/libbsc/bsc_vty.c +++ b/openbsc/src/libbsc/bsc_vty.c @@ -629,6 +629,15 @@ static void config_write_bts_single(struct vty *vty, struct gsm_bts *bts) } }
+ vty_out(vty, " codec-support fr"); + if (bts->codec.hr) + vty_out(vty, " hr"); + if (bts->codec.efr) + vty_out(vty, " efr"); + if (bts->codec.amr) + vty_out(vty, " amr"); + vty_out(vty, "%s", VTY_NEWLINE); + config_write_bts_gprs(vty, bts);
if (bts->excl_from_rf_lock) @@ -2635,6 +2644,65 @@ DEFUN(cfg_bts_no_excl_rf_lock, return CMD_SUCCESS; }
+static void _get_codec_from_arg(struct vty *vty, int argc, const char *argv[]) +{ + struct gsm_bts *bts = vty->index; + struct bts_codec_conf *codec = &bts->codec; + int i; + + codec->hr = 0; + codec->efr = 0; + codec->amr = 0; + for (i = 0; i < argc; i++) { + if (!strcmp(argv[i], "hr")) + codec->hr = 1; + if (!strcmp(argv[i], "efr")) + codec->efr = 1; + if (!strcmp(argv[i], "amr")) + codec->amr = 1; + } +} + +#define CODEC_PAR_STR " (fr|hr|efr|amr)" +#define CODEC_HELP_STR "Full Rate (mandatory)\nHalf Rate\n" \ + "Enhanced Full Rate\nAdaptive Multirate\n" + +DEFUN(cfg_bts_codec1, cfg_bts_codec1_cmd, + "codec-support" CODEC_PAR_STR, + "Codec Support settings\n" + CODEC_HELP_STR) +{ + _get_codec_from_arg(vty, 1, argv); + return CMD_SUCCESS; +} + +DEFUN(cfg_bts_codec2, cfg_bts_codec2_cmd, + "codec-support" CODEC_PAR_STR CODEC_PAR_STR, + "Codec Support settings\n" + CODEC_HELP_STR CODEC_HELP_STR) +{ + _get_codec_from_arg(vty, 2, argv); + return CMD_SUCCESS; +} + +DEFUN(cfg_bts_codec3, cfg_bts_codec3_cmd, + "codec-support" CODEC_PAR_STR CODEC_PAR_STR CODEC_PAR_STR, + "Codec Support settings\n" + CODEC_HELP_STR CODEC_HELP_STR CODEC_HELP_STR) +{ + _get_codec_from_arg(vty, 3, argv); + return CMD_SUCCESS; +} + +DEFUN(cfg_bts_codec4, cfg_bts_codec4_cmd, + "codec-support" CODEC_PAR_STR CODEC_PAR_STR CODEC_PAR_STR CODEC_PAR_STR, + "Codec Support settings\n" + CODEC_HELP_STR CODEC_HELP_STR CODEC_HELP_STR CODEC_HELP_STR) +{ + _get_codec_from_arg(vty, 4, argv); + return CMD_SUCCESS; +} + #define TRX_TEXT "Radio Transceiver\n"
/* per TRX configuration */ @@ -3231,6 +3299,10 @@ int bsc_vty_init(const struct log_info *cat) install_element(BTS_NODE, &cfg_bts_si5_neigh_cmd); install_element(BTS_NODE, &cfg_bts_excl_rf_lock_cmd); install_element(BTS_NODE, &cfg_bts_no_excl_rf_lock_cmd); + install_element(BTS_NODE, &cfg_bts_codec1_cmd); + install_element(BTS_NODE, &cfg_bts_codec2_cmd); + install_element(BTS_NODE, &cfg_bts_codec3_cmd); + install_element(BTS_NODE, &cfg_bts_codec4_cmd);
install_element(BTS_NODE, &cfg_trx_cmd); install_node(&trx_node, dummy_config_write); diff --git a/openbsc/src/libmsc/gsm_04_08.c b/openbsc/src/libmsc/gsm_04_08.c index 3cfc455..dec97f2 100644 --- a/openbsc/src/libmsc/gsm_04_08.c +++ b/openbsc/src/libmsc/gsm_04_08.c @@ -79,6 +79,29 @@ struct gsm_lai { uint16_t lac; };
+static int apply_codec_restrictions(struct gsm_bts *bts, + struct gsm_mncc_bearer_cap *bcap) +{ + int i, j; + + /* remove unsupported speech versions from list */ + for (i = 0, j = 0; bcap->speech_ver[i] >= 0; i++) { + if (bcap->speech_ver[i] == GSM48_BCAP_SV_FR) + bcap->speech_ver[j++] = GSM48_BCAP_SV_FR; + if (bcap->speech_ver[i] == GSM48_BCAP_SV_EFR && bts->codec.efr) + bcap->speech_ver[j++] = GSM48_BCAP_SV_EFR; + if (bcap->speech_ver[i] == GSM48_BCAP_SV_AMR_F && bts->codec.amr) + bcap->speech_ver[j++] = GSM48_BCAP_SV_AMR_F; + if (bcap->speech_ver[i] == GSM48_BCAP_SV_HR && bts->codec.hr) + bcap->speech_ver[j++] = GSM48_BCAP_SV_HR; + if (bcap->speech_ver[i] == GSM48_BCAP_SV_AMR_H && bts->codec.amr) + bcap->speech_ver[j++] = GSM48_BCAP_SV_AMR_H; + } + bcap->speech_ver[j] = -1; + + return 0; +} + static uint32_t new_callref = 0x80000001;
void cc_tx_to_mncc(struct gsm_network *net, struct msgb *msg) @@ -1763,6 +1786,7 @@ static int gsm48_cc_rx_setup(struct gsm_trans *trans, struct msgb *msg) setup.fields |= MNCC_F_BEARER_CAP; gsm48_decode_bearer_cap(&setup.bearer_cap, TLVP_VAL(&tp, GSM48_IE_BEARER_CAP)-1); + apply_codec_restrictions(trans->conn->bts, &setup.bearer_cap); } /* facility */ if (TLVP_PRESENT(&tp, GSM48_IE_FACILITY)) { @@ -1916,6 +1940,7 @@ static int gsm48_cc_rx_call_conf(struct gsm_trans *trans, struct msgb *msg) call_conf.fields |= MNCC_F_BEARER_CAP; gsm48_decode_bearer_cap(&call_conf.bearer_cap, TLVP_VAL(&tp, GSM48_IE_BEARER_CAP)-1); + apply_codec_restrictions(trans->conn->bts, &call_conf.bearer_cap); } /* cause */ if (TLVP_PRESENT(&tp, GSM48_IE_CAUSE)) { @@ -2604,6 +2629,7 @@ static int gsm48_cc_rx_modify(struct gsm_trans *trans, struct msgb *msg) modify.fields |= MNCC_F_BEARER_CAP; gsm48_decode_bearer_cap(&modify.bearer_cap, TLVP_VAL(&tp, GSM48_IE_BEARER_CAP)-1); + apply_codec_restrictions(trans->conn->bts, &modify.bearer_cap); }
new_cc_state(trans, GSM_CSTATE_MO_ORIG_MODIFY); @@ -2646,6 +2672,7 @@ static int gsm48_cc_rx_modify_complete(struct gsm_trans *trans, struct msgb *msg modify.fields |= MNCC_F_BEARER_CAP; gsm48_decode_bearer_cap(&modify.bearer_cap, TLVP_VAL(&tp, GSM48_IE_BEARER_CAP)-1); + apply_codec_restrictions(trans->conn->bts, &modify.bearer_cap); }
new_cc_state(trans, GSM_CSTATE_ACTIVE); @@ -2686,6 +2713,7 @@ static int gsm48_cc_rx_modify_reject(struct gsm_trans *trans, struct msgb *msg) modify.fields |= GSM48_IE_BEARER_CAP; gsm48_decode_bearer_cap(&modify.bearer_cap, TLVP_VAL(&tp, GSM48_IE_BEARER_CAP)-1); + apply_codec_restrictions(trans->conn->bts, &modify.bearer_cap); } /* cause */ if (TLVP_PRESENT(&tp, GSM48_IE_CAUSE)) {
On Sat, Dec 07, 2013 at 06:32:28PM +0100, Andreas Eversberg wrote:
Hi!
openbsc/src/libbsc/bsc_vty.c | 72 +++++++++++++++++++++++++++++
I just wanted to merge the code but
- vty_out(vty, " codec-support fr");
so your code assumes that FR is supported anyway.
+#define CODEC_PAR_STR " (fr|hr|efr|amr)"
What is the point of being allowed to not configure it?
Holger Hans Peter Freyther wrote:
so your code assumes that FR is supported anyway.
+#define CODEC_PAR_STR " (fr|hr|efr|amr)"
What is the point of being allowed to not configure it?
i just wanted to show in the vty config that fr is supported, even if it is mandatory and cannot be made unsupported.
On Wed, Dec 18, 2013 at 05:17:16PM +0100, Andreas Eversberg wrote:
i just wanted to show in the vty config that fr is supported, even if it is mandatory and cannot be made unsupported.
What about this? It makes it mandatory to define fr first. The other things I noticed is:
* Currently the default codec MNCC-intern uses is EFR. After the change it will be FR. * The BTS should be involved as well to ack/nack the codec support (we can do that later) * The BSC part should start to honor this support too.
diff --git a/openbsc/src/libbsc/bsc_vty.c b/openbsc/src/libbsc/bsc_vty.c index 1bc0ea1..9801b6a 100644 --- a/openbsc/src/libbsc/bsc_vty.c +++ b/openbsc/src/libbsc/bsc_vty.c @@ -2663,13 +2663,13 @@ static void _get_codec_from_arg(struct vty *vty, int argc, const char *argv[]) } }
-#define CODEC_PAR_STR " (fr|hr|efr|amr)" -#define CODEC_HELP_STR "Full Rate (mandatory)\nHalf Rate\n" \ +#define CODEC_PAR_STR " (hr|efr|amr)" +#define CODEC_HELP_STR "Half Rate\n" \ "Enhanced Full Rate\nAdaptive Multirate\n"
DEFUN(cfg_bts_codec1, cfg_bts_codec1_cmd, - "codec-support" CODEC_PAR_STR, - "Codec Support settings\n" + "codec-support fr" CODEC_PAR_STR, + "Codec Support settings\nFullrate\n" CODEC_HELP_STR) { _get_codec_from_arg(vty, 1, argv); @@ -2677,8 +2677,8 @@ DEFUN(cfg_bts_codec1, cfg_bts_codec1_cmd, }
DEFUN(cfg_bts_codec2, cfg_bts_codec2_cmd, - "codec-support" CODEC_PAR_STR CODEC_PAR_STR, - "Codec Support settings\n" + "codec-support fr" CODEC_PAR_STR CODEC_PAR_STR, + "Codec Support settings\nFullrate\n" CODEC_HELP_STR CODEC_HELP_STR) { _get_codec_from_arg(vty, 2, argv); @@ -2686,8 +2686,8 @@ DEFUN(cfg_bts_codec2, cfg_bts_codec2_cmd, }
DEFUN(cfg_bts_codec3, cfg_bts_codec3_cmd, - "codec-support" CODEC_PAR_STR CODEC_PAR_STR CODEC_PAR_STR, - "Codec Support settings\n" + "codec-support fr" CODEC_PAR_STR CODEC_PAR_STR CODEC_PAR_STR, + "Codec Support settings\nFullrate\n" CODEC_HELP_STR CODEC_HELP_STR CODEC_HELP_STR) { _get_codec_from_arg(vty, 3, argv); @@ -2695,8 +2695,8 @@ DEFUN(cfg_bts_codec3, cfg_bts_codec3_cmd, }
DEFUN(cfg_bts_codec4, cfg_bts_codec4_cmd, - "codec-support" CODEC_PAR_STR CODEC_PAR_STR CODEC_PAR_STR CODEC_PAR_STR, - "Codec Support settings\n" + "codec-support fr" CODEC_PAR_STR CODEC_PAR_STR CODEC_PAR_STR CODEC_PAR_STR, + "Codec Support settings\nFullrate\n" CODEC_HELP_STR CODEC_HELP_STR CODEC_HELP_STR CODEC_HELP_STR) { _get_codec_from_arg(vty, 4, argv);
Holger Hans Peter Freyther wrote:
What about this? It makes it mandatory to define fr first. The other things I noticed is:
- Currently the default codec MNCC-intern uses is EFR. After the change it will be FR.
- The BTS should be involved as well to ack/nack the codec support (we can do that later)
- The BSC part should start to honor this support too.
dear holger,
i updated/rebased and tested the patch.
i changed the default codec for built-in MNCC call control to FR. in later patches this setting for default codec becomes obsolete, because codec selection (negotiation) is then done by exermining capabilities/preferences of the calling and the called phone and the codec-support of their BTSes.
what do you mean by "The BSC part should start to honor this support too."?
regards,
andreas
On Tue, Jan 07, 2014 at 12:05:53PM +0100, Andreas Eversberg wrote:
what do you mean by "The BSC part should start to honor this support too."?
Codec selection in a real network works a bit differently. It is applied in the BSC and the MSC. The MSC can/will look at the bearer capabilities and then ask the BSC to choose from a list of codecs.
In the future bssmap_handle_assignm_req should start to use the new list instead of the "codec-list" from the "msc" section.
And in the long run NITB should do what a MSC does and select candidates and the BSC-API should then select from this list based on the capabilities of the hardware.
holger
Holger Hans Peter Freyther wrote:
Codec selection in a real network works a bit differently. It is applied in the BSC and the MSC. The MSC can/will look at the bearer capabilities and then ask the BSC to choose from a list of codecs.
in oder to select a codec, the MSC must know. - what codecs it supports (or the other end, in case of TFO) - what codecs the MS supports (bearer capabilities) - what codecs the BTS supports
in my testing branch: the negotiation is done with LCR and even with built-in MNCC support of osmo-nitb. in case of built-in MNCC, a commonly supported codec (by both MS in a call) is used.
i do not know how the MSC (in a real network) knows about what codecs are supported by the BTS in order to select a supported codec. in this patch i simply remove unsupported codecs from bearer capabilities, so all remaining codecs are supported by BTS and the MS. the MSC will only receive a list of commonly supported codecs.
my question is: how does the MSC in a real network knows what codecs are supported by BTS, so it can skip unsupported codecs when negotiating with the other end?
Hello,
On 01/09/2014 11:13 AM, Andreas Eversberg wrote:
my question is: how does the MSC in a real network knows what codecs are supported by BTS, so it can skip unsupported codecs when negotiating with the other end?
My understanding is that the BTS is not involved at all in this negotiation. Only MS, BSC and MSC. BTS just passes the messages between MS and BSC.
In the initial call setup procedure the MS is sending to MSC the supported codecs list. Information is sent in an L3 DTAP message through BSC but BSC is not involved this time. MSC is building after that a list of supported codecs by both ends and is sending the list to the BSC. If the codec chosen by BSC is supported by both ends than you have a transcoder free operation. All elements involved needs to support TrFO to work. More in 23.153 and 45.009.
In case of A over TDM you can not have TrFO working, just for A over IP.
I looked over the code (I'm not a programmer) and I cannot see where you specify the bit rate for AMR codec. In 45.009 3.4.3 you can check the rules for initial codec mode and cases when the bit rate can change.
In real networks I saw most of the time AMR used and I also saw same BSCs overwriting the ICM by selecting the highest bitrate when message is sent through Abis interface to MS. Normal behavior is to follow the rules in 3.4.3.
In TrFO the bitrate will not be signalled by MSC in L3 messages (this function is not even implemented from what I saw) because there are cases when MSC is not involved like intra-BSC handover with a local MS.
I hope I wrote everything correctly.
Regards, R.
On Tue, Jan 07, 2014 at 12:05:53PM +0100, Andreas Eversberg wrote:
+static void _get_codec_from_arg(struct vty *vty, int argc, const char *argv[])
and I was too quick. "_" symbols are reserved for the system GCC/GLIBC. Right now this should not be a problem though.
On Tue, Jan 07, 2014 at 12:05:53PM +0100, Andreas Eversberg wrote:
commit 037c6084ab68785666a84c89a5f5d45de6a44620
hi,
git am fails on the patch. Can you run git format-patch again and send the patch?
holger
On Tue, Jan 07, 2014 at 12:05:53PM +0100, Andreas Eversberg wrote:
Each BTS can be configured for speech support (other than GSM full rate) Speech codings which are not supported by BTS will be removed from the bearer capability information element after parsing. This way it is not required for the MNCC application to consider support of each BTS. Only GSM full rate is supported by default.
coverity complains about two issues. Make sure to put "Fixes: Coverity CID .." into the commit message.
** CID 1155311: Dereference after null check (FORWARD_NULL) /src/libmsc/gsm_04_08.c: 1825 in gsm48_cc_rx_setup()
** CID 1155312: Dereference after null check (FORWARD_NULL) /src/libmsc/gsm_04_08.c: 1979 in gsm48_cc_rx_call_conf()
_______________________________________________________________________________________________ +_________ *** CID 1155311: Dereference after null check (FORWARD_NULL) /src/libmsc/gsm_04_08.c: 1825 in gsm48_cc_rx_setup() 1819 1820 /* bearer capability */ 1821 if (TLVP_PRESENT(&tp, GSM48_IE_BEARER_CAP)) { 1822 setup.fields |= MNCC_F_BEARER_CAP; 1823 gsm48_decode_bearer_cap(&setup.bearer_cap, 1824 TLVP_VAL(&tp, GSM48_IE_BEARER_CAP)-1);
CID 1155311: Dereference after null check (FORWARD_NULL) Dereferencing null pointer "trans->conn".
1825 apply_codec_restrictions(trans->conn->bts, &setup.bearer_cap); 1826 } 1827 /* facility */ 1828 if (TLVP_PRESENT(&tp, GSM48_IE_FACILITY)) { 1829 setup.fields |= MNCC_F_FACILITY; 1830 gsm48_decode_facility(&setup.facility,
*** CID 1155312: Dereference after null check (FORWARD_NULL) /src/libmsc/gsm_04_08.c: 1979 in gsm48_cc_rx_call_conf() 1973 #endif 1974 /* bearer capability */ 1975 if (TLVP_PRESENT(&tp, GSM48_IE_BEARER_CAP)) { 1976 call_conf.fields |= MNCC_F_BEARER_CAP; 1977 gsm48_decode_bearer_cap(&call_conf.bearer_cap, 1978 TLVP_VAL(&tp, GSM48_IE_BEARER_CAP)-1);
CID 1155312: Dereference after null check (FORWARD_NULL) Dereferencing null pointer "trans->conn".
1979 apply_codec_restrictions(trans->conn->bts, &call_conf.bearer_cap); 1980 } 1981 /* cause */ 1982 if (TLVP_PRESENT(&tp, GSM48_IE_CAUSE)) { 1983 call_conf.fields |= MNCC_F_CAUSE; 1984 gsm48_decode_cause(&call_conf.cause,
Holger Hans Peter Freyther wrote:
** CID 1155311: Dereference after null check (FORWARD_NULL) /src/libmsc/gsm_04_08.c: 1825 in gsm48_cc_rx_setup()
** CID 1155312: Dereference after null check (FORWARD_NULL) /src/libmsc/gsm_04_08.c: 1979 in gsm48_cc_rx_call_conf()
hi holger,
in both functions is a check for conn and lchan not beeing NULL:
if (trans->conn && trans->conn->lchan) setup.lchan_type = trans->conn->lchan->type;
the functions are called by gsm0408_rcv_cc(). from there i can see that trans->conn->lchan is always set, so the if-condition above is not required.
regards,
andreas
On Wed, Jan 15, 2014 at 04:23:23PM +0100, Andreas Eversberg wrote:
in both functions is a check for conn and lchan not beeing NULL:
if (trans->conn && trans->conn->lchan) setup.lchan_type = trans->conn->lchan->type;
the functions are called by gsm0408_rcv_cc(). from there i can see that trans->conn->lchan is always set, so the if-condition above is not required.
Exactly. Coverity points out that first you check if (trans->conn), there is no return and later we _unconditionally_ do "trans->conn->bts".
a.) Coverity is right and there are conditions it can crash b.) The if (trans->conn) is not needed.
Could you please elaborate which of the two is the case here?
holger
On Wed, Jan 15, 2014 at 06:10:06PM +0100, Andreas Eversberg wrote:
Holger Hans Peter Freyther wrote:
b.) The if (trans->conn) is not needed.
this is the case.
and why is that? Who calls the method and are the callers already doing the null check? Please create a patch and mention the CIDs in the commit message.
-
Andreas Eversberg -
Andreas Eversberg -
Holger Hans Peter Freyther -
Labs