From f9778b2a26ce06959ee7e188eb1d533d896f1846 Mon Sep 17 00:00:00 2001 From: Max Date: Wed, 28 Dec 2016 12:43:09 +0100 Subject: DTX AMR HR: fix inhibition * Unlike in AMR FR, in AMR HR incoming ONSET have to be treated differently depending on whether we've recently sent SID UPDATE or EMPTY frame. Split ST_SID_U FSM state into 2 states to accommodate for that and make sure that additional states specific to AMR HR are not used for AMR FR. * Avoid sending E_VOICE and E_SID_U in corresponding states as those do not initiate FSM state transitions anyway. This decrease extra load from FSM signalling which otherwise would be triggered on per-frame basis. * Introduce separate signal for SID First P1 -> P2 transition to avoid confusion with E_COMPL and E_SID_U initiated transitions from P1 state. * Don't init DTX FSM for SDCCH channels. Change-Id: I229ba39a38a223fada4881fc9aca35d3639371f8 Related: OS#1801 --- include/osmo-bts/dtx_dl_amr_fsm.h | 2 + include/osmo-bts/msg_utils.h | 1 + src/common/bts.c | 1 + src/common/dtx_dl_amr_fsm.c | 55 +++++++++++++++++----- src/common/l1sap.c | 3 +- src/common/msg_utils.c | 97 +++++++++++++++++++++++++++++---------- src/osmo-bts-litecell15/l1_if.c | 6 ++- src/osmo-bts-litecell15/tch.c | 1 + src/osmo-bts-sysmo/l1_if.c | 6 ++- src/osmo-bts-sysmo/tch.c | 1 + 10 files changed, 133 insertions(+), 40 deletions(-) diff --git a/include/osmo-bts/dtx_dl_amr_fsm.h b/include/osmo-bts/dtx_dl_amr_fsm.h index 4fb2f251..f747f9f1 100644 --- a/include/osmo-bts/dtx_dl_amr_fsm.h +++ b/include/osmo-bts/dtx_dl_amr_fsm.h @@ -14,6 +14,7 @@ enum dtx_dl_amr_fsm_states { ST_SID_F2, ST_F1_INH, ST_U_INH, + ST_U_NOINH, ST_F1_INH_REC, ST_U_INH_REC, ST_SID_U, @@ -29,6 +30,7 @@ enum dtx_dl_amr_fsm_events { E_ONSET, E_FACCH, E_COMPL, + E_FIRST, E_INHIB, E_SID_F, E_SID_U, diff --git a/include/osmo-bts/msg_utils.h b/include/osmo-bts/msg_utils.h index 55e8475d..7ddbe88f 100644 --- a/include/osmo-bts/msg_utils.h +++ b/include/osmo-bts/msg_utils.h @@ -37,6 +37,7 @@ bool dtx_dl_amr_enabled(const struct gsm_lchan *lchan); void dtx_dispatch(struct gsm_lchan *lchan, enum dtx_dl_amr_fsm_events e); bool dtx_recursion(const struct gsm_lchan *lchan); void dtx_int_signal(struct gsm_lchan *lchan); +bool dtx_is_first_p1(const struct gsm_lchan *lchan); void dtx_cache_payload(struct gsm_lchan *lchan, const uint8_t *l1_payload, size_t length, uint32_t fn, int update); int dtx_dl_amr_fsm_step(struct gsm_lchan *lchan, const uint8_t *rtp_pl, diff --git a/src/common/bts.c b/src/common/bts.c index 9c2f0e03..d2491370 100644 --- a/src/common/bts.c +++ b/src/common/bts.c @@ -45,6 +45,7 @@ #include #include #include +#include #define MIN_QUAL_RACH 5.0f /* at least 5 dB C/I */ #define MIN_QUAL_NORM -0.5f /* at least -1 dB C/I */ diff --git a/src/common/dtx_dl_amr_fsm.c b/src/common/dtx_dl_amr_fsm.c index d903b0cf..832e8b4e 100644 --- a/src/common/dtx_dl_amr_fsm.c +++ b/src/common/dtx_dl_amr_fsm.c @@ -47,7 +47,7 @@ void dtx_fsm_sid_f1(struct osmo_fsm_inst *fi, uint32_t event, void *data) Was observed during testing, let's just ignore it for now */ break; case E_SID_U: - osmo_fsm_inst_state_chg(fi, ST_SID_U, 0, 0); + osmo_fsm_inst_state_chg(fi, ST_U_NOINH, 0, 0); break; case E_VOICE: osmo_fsm_inst_state_chg(fi, ST_VOICE, 0, 0); @@ -55,7 +55,7 @@ void dtx_fsm_sid_f1(struct osmo_fsm_inst *fi, uint32_t event, void *data) case E_FACCH: osmo_fsm_inst_state_chg(fi, ST_ONSET_F, 0, 0); break; - case E_COMPL: + case E_FIRST: osmo_fsm_inst_state_chg(fi, ST_SID_F2, 0, 0); break; case E_INHIB: @@ -74,7 +74,7 @@ void dtx_fsm_sid_f1(struct osmo_fsm_inst *fi, uint32_t event, void *data) void dtx_fsm_sid_f2(struct osmo_fsm_inst *fi, uint32_t event, void *data) { switch (event) { - case E_SID_U: + case E_COMPL: osmo_fsm_inst_state_chg(fi, ST_SID_U, 0, 0); break; case E_VOICE: @@ -145,7 +145,7 @@ void dtx_fsm_u_inh_rec(struct osmo_fsm_inst *fi, uint32_t event, void *data) } } -void dtx_fsm_sid_upd(struct osmo_fsm_inst *fi, uint32_t event, void *data) +void dtx_fsm_u_noinh(struct osmo_fsm_inst *fi, uint32_t event, void *data) { switch (event) { case E_FACCH: @@ -154,8 +154,8 @@ void dtx_fsm_sid_upd(struct osmo_fsm_inst *fi, uint32_t event, void *data) case E_VOICE: osmo_fsm_inst_state_chg(fi, ST_VOICE, 0, 0); break; - case E_INHIB: - osmo_fsm_inst_state_chg(fi, ST_U_INH, 0, 0); + case E_COMPL: + osmo_fsm_inst_state_chg(fi, ST_SID_U, 0, 0); break; case E_SID_U: case E_SID_F: @@ -172,6 +172,29 @@ void dtx_fsm_sid_upd(struct osmo_fsm_inst *fi, uint32_t event, void *data) } } +void dtx_fsm_sid_upd(struct osmo_fsm_inst *fi, uint32_t event, void *data) +{ + switch (event) { + case E_FACCH: + osmo_fsm_inst_state_chg(fi, ST_ONSET_F, 0, 0); + break; + case E_VOICE: + osmo_fsm_inst_state_chg(fi, ST_VOICE, 0, 0); + break; + case E_INHIB: + osmo_fsm_inst_state_chg(fi, ST_U_INH, 0, 0); + break; + case E_SID_U: + case E_SID_F: + osmo_fsm_inst_state_chg(fi, ST_U_NOINH, 0, 0); + break; + default: + LOGP(DL1P, LOGL_ERROR, "Unexpected event %d\n", event); + OSMO_ASSERT(0); + break; + } +} + void dtx_fsm_onset_v(struct osmo_fsm_inst *fi, uint32_t event, void *data) { switch (event) { @@ -255,15 +278,15 @@ static struct osmo_fsm_state dtx_dl_amr_fsm_states[] = { /* SID-FIRST or SID-FIRST-P1 in case of AMR HR: start of silence period (might be interrupted in case of AMR HR) */ [ST_SID_F1]= { - .in_event_mask = X(E_SID_F) | X(E_SID_U) | X(E_VOICE) | X(E_FACCH) | X(E_COMPL) | X(E_INHIB) | X(E_ONSET), - .out_state_mask = X(ST_SID_U) | X(ST_VOICE) | X(ST_ONSET_F) | X(ST_SID_F2) | X(ST_F1_INH) | X(ST_ONSET_V), + .in_event_mask = X(E_SID_F) | X(E_SID_U) | X(E_VOICE) | X(E_FACCH) | X(E_FIRST) | X(E_INHIB) | X(E_ONSET), + .out_state_mask = X(ST_U_NOINH) | X(ST_VOICE) | X(ST_ONSET_F) | X(ST_SID_F2) | X(ST_F1_INH) | X(ST_ONSET_V), .name = "SID-FIRST (P1)", .action = dtx_fsm_sid_f1, }, /* SID-FIRST P2 (only for AMR HR): actual start of silence period in case of AMR HR */ [ST_SID_F2]= { - .in_event_mask = X(E_SID_U) | X(E_VOICE) | X(E_FACCH) | X(E_ONSET), + .in_event_mask = X(E_COMPL) | X(E_VOICE) | X(E_FACCH) | X(E_ONSET), .out_state_mask = X(ST_SID_U) | X(ST_VOICE) | X(ST_ONSET_F) | X(ST_ONSET_V), .name = "SID-FIRST (P2)", .action = dtx_fsm_sid_f2, @@ -282,6 +305,13 @@ static struct osmo_fsm_state dtx_dl_amr_fsm_states[] = { .name = "SID-UPDATE (Inh)", .action = dtx_fsm_u_inh, }, + /* SID-UPDATE: Inhibited not allowed (only for AMR HR) */ + [ST_U_NOINH]= { + .in_event_mask = X(E_FACCH) | X(E_VOICE) | X(E_COMPL) | X(E_SID_U) | X(E_SID_F) | X(E_ONSET), + .out_state_mask = X(ST_ONSET_F) | X(ST_VOICE) | X(ST_SID_U) | X(ST_ONSET_V), + .name = "SID-UPDATE (NoInh)", + .action = dtx_fsm_u_noinh, + }, /* SID-FIRST Inhibition recursion in progress: Inhibit itself was already sent, now have to send the voice that caused it */ [ST_F1_INH_REC]= { @@ -300,9 +330,9 @@ static struct osmo_fsm_state dtx_dl_amr_fsm_states[] = { }, /* Silence period with periodic comfort noise data updates */ [ST_SID_U]= { - .in_event_mask = X(E_FACCH) | X(E_VOICE) | X(E_INHIB) | X(E_SID_U) | X(E_SID_F) | X(E_ONSET), - .out_state_mask = X(ST_ONSET_F) | X(ST_VOICE) | X(ST_U_INH) | X(ST_SID_U) | X(ST_ONSET_V), - .name = "SID-UPDATE", + .in_event_mask = X(E_FACCH) | X(E_VOICE) | X(E_INHIB) | X(E_SID_U) | X(E_SID_F), + .out_state_mask = X(ST_ONSET_F) | X(ST_VOICE) | X(ST_U_INH) | X(ST_U_NOINH), + .name = "SID-UPDATE (AMR/HR)", .action = dtx_fsm_sid_upd, }, /* ONSET - end of silent period due to incoming SPEECH frame */ @@ -350,6 +380,7 @@ const struct value_string dtx_dl_amr_fsm_event_names[] = { { E_ONSET, "ONSET" }, { E_FACCH, "FACCH" }, { E_COMPL, "Complete" }, + { E_FIRST, "FIRST P1->P2" }, { E_INHIB, "Inhibit" }, { E_SID_F, "SID-FIRST" }, { E_SID_U, "SID-UPDATE" }, diff --git a/src/common/l1sap.c b/src/common/l1sap.c index e9c94f0d..e6e53db6 100644 --- a/src/common/l1sap.c +++ b/src/common/l1sap.c @@ -1161,8 +1161,7 @@ int l1sap_chan_act(struct gsm_bts_trx *trx, uint8_t chan_nr, struct tlv_parsed * return -RSL_ERR_EQUIPMENT_FAIL; /* Init DTX DL FSM if necessary */ - //FIXME: only do it for AMR TCH/* - if (trx->bts->dtxd) + if (trx->bts->dtxd && lchan->type != GSM_LCHAN_SDCCH) lchan->tch.dtx.dl_amr_fsm = osmo_fsm_inst_alloc(&dtx_dl_amr_fsm, tall_bts_ctx, lchan, diff --git a/src/common/msg_utils.c b/src/common/msg_utils.c index 9de9b6d6..062f5e3a 100644 --- a/src/common/msg_utils.c +++ b/src/common/msg_utils.c @@ -93,6 +93,28 @@ static int check_manuf(struct msgb *msg, struct abis_om_hdr *omh, size_t msg_siz return type; } +/* check that DTX is in the middle of silence */ +static inline bool dtx_is_update(const struct gsm_lchan *lchan) +{ + if (!dtx_dl_amr_enabled(lchan)) + return false; + if (lchan->tch.dtx.dl_amr_fsm->state == ST_SID_U || + lchan->tch.dtx.dl_amr_fsm->state == ST_U_NOINH) + return true; + return false; +} + +/* check that DTX is in the beginning of silence for AMR HR */ +bool dtx_is_first_p1(const struct gsm_lchan *lchan) +{ + if (!dtx_dl_amr_enabled(lchan)) + return false; + if ((lchan->type == GSM_LCHAN_TCH_H && + lchan->tch.dtx.dl_amr_fsm->state == ST_SID_F1)) + return true; + return false; +} + /* update lchan SID status */ void lchan_set_marker(bool t, struct gsm_lchan *lchan) { @@ -123,9 +145,7 @@ void dtx_cache_payload(struct gsm_lchan *lchan, const uint8_t *l1_payload, if (update == 0) { lchan->tch.dtx.is_update = false; /* Mark SID FIRST explicitly */ /* for non-AMR case - always update FN for incoming SID FIRST */ - if (!amr || - (dtx_dl_amr_enabled(lchan) && - lchan->tch.dtx.dl_amr_fsm->state != ST_SID_U)) + if (!amr || !dtx_is_update(lchan)) lchan->tch.dtx.fn = fn; /* for AMR case - do not update FN if SID FIRST arrives in a middle of silence: this should not be happening according to @@ -157,12 +177,24 @@ int dtx_dl_amr_fsm_step(struct gsm_lchan *lchan, const uint8_t *rtp_pl, int rc; if (dtx_dl_amr_enabled(lchan)) { - if (lchan->type == GSM_LCHAN_TCH_H && - lchan->tch.dtx.dl_amr_fsm->state == ST_SID_F2 && !rtp_pl) { - *len = 3; /* SID-FIRST P1 -> P2 completion */ - memcpy(l1_payload, lchan->tch.dtx.cache, 2); - dtx_dispatch(lchan, E_SID_U); - return 0; + if (lchan->type == GSM_LCHAN_TCH_H && !rtp_pl) { + /* we're called by gen_empty_tch_msg() to handle states + specific to AMR HR DTX */ + switch (lchan->tch.dtx.dl_amr_fsm->state) { + case ST_SID_F2: + *len = 3; /* SID-FIRST P1 -> P2 completion */ + memcpy(l1_payload, lchan->tch.dtx.cache, 2); + rc = 0; + dtx_dispatch(lchan, E_COMPL); + break; + case ST_SID_U: + rc = -EBADMSG; + dtx_dispatch(lchan, E_SID_U); + break; + default: + rc = -EBADMSG; + } + return rc; } } @@ -171,8 +203,8 @@ int dtx_dl_amr_fsm_step(struct gsm_lchan *lchan, const uint8_t *rtp_pl, rc = osmo_amr_rtp_dec(rtp_pl, rtp_pl_len, &cmr, &cmi, &ft, &bfi, &sti); if (rc < 0) { - LOGP(DRTP, LOGL_ERROR, "failed to decode AMR RTP (length %zu)\n", - rtp_pl_len); + LOGP(DRTP, LOGL_ERROR, "failed to decode AMR RTP (length %zu, " + "%p)\n", rtp_pl_len, rtp_pl); return rc; } @@ -197,19 +229,29 @@ int dtx_dl_amr_fsm_step(struct gsm_lchan *lchan, const uint8_t *rtp_pl, return 0; if (osmo_amr_is_speech(ft)) { - /* AMR HR - Inhibition */ - if (lchan->type == GSM_LCHAN_TCH_H && marker && - lchan->tch.dtx.dl_amr_fsm->state == ST_SID_F1) + /* AMR HR - SID-FIRST_P1 Inhibition */ + if (marker && dtx_is_first_p1(lchan)) return osmo_fsm_inst_dispatch(lchan->tch.dtx.dl_amr_fsm, E_INHIB, (void *)lchan); + + /* AMR HR - SID-UPDATE Inhibition */ + if (marker && lchan->type == GSM_LCHAN_TCH_H && + lchan->tch.dtx.dl_amr_fsm->state == ST_SID_U) + return osmo_fsm_inst_dispatch(lchan->tch.dtx.dl_amr_fsm, + E_INHIB, (void *)lchan); + /* AMR FR & HR - generic */ if (marker && (lchan->tch.dtx.dl_amr_fsm->state == ST_SID_F1 || lchan->tch.dtx.dl_amr_fsm->state == ST_SID_F2 || - lchan->tch.dtx.dl_amr_fsm->state == ST_SID_U )) + lchan->tch.dtx.dl_amr_fsm->state == ST_U_NOINH)) return osmo_fsm_inst_dispatch(lchan->tch.dtx.dl_amr_fsm, E_ONSET, (void *)lchan); - return osmo_fsm_inst_dispatch(lchan->tch.dtx.dl_amr_fsm, E_VOICE, - (void *)lchan); + + if (lchan->tch.dtx.dl_amr_fsm->state != ST_VOICE) + return osmo_fsm_inst_dispatch(lchan->tch.dtx.dl_amr_fsm, + E_VOICE, (void *)lchan); + + return 0; } if (ft == AMR_SID) { @@ -220,8 +262,11 @@ int dtx_dl_amr_fsm_step(struct gsm_lchan *lchan, const uint8_t *rtp_pl, dtx_cache_payload(lchan, rtp_pl, rtp_pl_len, fn, false); return osmo_fsm_inst_dispatch(lchan->tch.dtx.dl_amr_fsm, E_SID_F, (void *)lchan); - } else + } else if (lchan->tch.dtx.dl_amr_fsm->state != ST_FACCH) dtx_cache_payload(lchan, rtp_pl, rtp_pl_len, fn, sti); + if (lchan->tch.dtx.dl_amr_fsm->state == ST_SID_F2) + return osmo_fsm_inst_dispatch(lchan->tch.dtx.dl_amr_fsm, + E_COMPL, (void *)lchan); return osmo_fsm_inst_dispatch(lchan->tch.dtx.dl_amr_fsm, sti ? E_SID_U : E_SID_F, (void *)lchan); @@ -357,6 +402,7 @@ bool dtx_recursion(const struct gsm_lchan *lchan) return false; if (lchan->tch.dtx.dl_amr_fsm->state == ST_U_INH || + lchan->tch.dtx.dl_amr_fsm->state == ST_U_INH_REC || lchan->tch.dtx.dl_amr_fsm->state == ST_F1_INH || lchan->tch.dtx.dl_amr_fsm->state == ST_ONSET_F || lchan->tch.dtx.dl_amr_fsm->state == ST_ONSET_V || @@ -389,9 +435,7 @@ void dtx_int_signal(struct gsm_lchan *lchan) if (!dtx_dl_amr_enabled(lchan)) return; - if ((lchan->type == GSM_LCHAN_TCH_H && - lchan->tch.dtx.dl_amr_fsm->state == ST_SID_F1) || - dtx_recursion(lchan)) + if (dtx_is_first_p1(lchan) || dtx_recursion(lchan)) dtx_dispatch(lchan, E_COMPL); } @@ -425,12 +469,17 @@ uint8_t repeat_last_sid(struct gsm_lchan *lchan, uint8_t *dst, uint32_t fn) osmo_fsm_inst_dispatch(lchan->tch.dtx.dl_amr_fsm, E_SID_U, (void *)lchan); dtx_sti_unset(lchan); - } else if (lchan->tch.dtx.dl_amr_fsm->state == - ST_SID_U) { + } else if (dtx_is_update(lchan)) { /* enforce SID UPDATE for next repetition: it might have been altered by FACCH handling */ dtx_sti_set(lchan); - lchan->tch.dtx.is_update = true; + if (lchan->type == GSM_LCHAN_TCH_H && + lchan->tch.dtx.dl_amr_fsm->state == + ST_U_NOINH) + osmo_fsm_inst_dispatch(lchan->tch.dtx.dl_amr_fsm, + E_COMPL, + (void *)lchan); + lchan->tch.dtx.is_update = true; } } memcpy(dst, lchan->tch.dtx.cache, lchan->tch.dtx.len); diff --git a/src/osmo-bts-litecell15/l1_if.c b/src/osmo-bts-litecell15/l1_if.c index d9593382..99533d73 100644 --- a/src/osmo-bts-litecell15/l1_if.c +++ b/src/osmo-bts-litecell15/l1_if.c @@ -416,6 +416,7 @@ static int ph_data_req(struct gsm_bts_trx *trx, struct msgb *msg, } if (sapi == GsmL1_Sapi_FacchH) { sapi = GsmL1_Sapi_TchH; + subCh = L1SAP_CHAN2SS_TCHH(chan_nr); } if (sapi == GsmL1_Sapi_TchH || sapi == GsmL1_Sapi_TchF) { /* FACCH interruption of DTX silence */ @@ -542,7 +543,10 @@ static int ph_tch_req(struct gsm_bts_trx *trx, struct msgb *msg, } /* send message to DSP's queue */ osmo_wqueue_enqueue(&fl1->write_q[MQ_L1_WRITE], nmsg); - dtx_int_signal(lchan); + if (dtx_is_first_p1(lchan)) + dtx_dispatch(lchan, E_FIRST); + else + dtx_int_signal(lchan); if (dtx_recursion(lchan)) /* DTX: send voice after ONSET was sent */ return ph_tch_req(trx, l1sap->oph.msg, l1sap, true, false); diff --git a/src/osmo-bts-litecell15/tch.c b/src/osmo-bts-litecell15/tch.c index de3c7e35..a47a88f6 100644 --- a/src/osmo-bts-litecell15/tch.c +++ b/src/osmo-bts-litecell15/tch.c @@ -322,6 +322,7 @@ int l1if_tch_encode(struct gsm_lchan *lchan, uint8_t *data, uint8_t *len, dtx_cache_payload(lchan, rtp_pl, rtp_pl_len, fn, 0); return 1; case ST_SID_U: + case ST_U_NOINH: return -EAGAIN; case ST_FACCH: return -EBADMSG; diff --git a/src/osmo-bts-sysmo/l1_if.c b/src/osmo-bts-sysmo/l1_if.c index 2a3caf95..ad9aa644 100644 --- a/src/osmo-bts-sysmo/l1_if.c +++ b/src/osmo-bts-sysmo/l1_if.c @@ -411,6 +411,7 @@ static int ph_data_req(struct gsm_bts_trx *trx, struct msgb *msg, } if (sapi == GsmL1_Sapi_FacchH) { sapi = GsmL1_Sapi_TchH; + subCh = L1SAP_CHAN2SS_TCHH(chan_nr); } if (sapi == GsmL1_Sapi_TchH || sapi == GsmL1_Sapi_TchF) { /* FACCH interruption of DTX silence */ @@ -537,7 +538,10 @@ static int ph_tch_req(struct gsm_bts_trx *trx, struct msgb *msg, } /* send message to DSP's queue */ osmo_wqueue_enqueue(&fl1->write_q[MQ_L1_WRITE], nmsg); - dtx_int_signal(lchan); + if (dtx_is_first_p1(lchan)) + dtx_dispatch(lchan, E_FIRST); + else + dtx_int_signal(lchan); if (dtx_recursion(lchan)) /* DTX: send voice after ONSET was sent */ return ph_tch_req(trx, l1sap->oph.msg, l1sap, true, false); diff --git a/src/osmo-bts-sysmo/tch.c b/src/osmo-bts-sysmo/tch.c index 16c2cf3a..bc495d99 100644 --- a/src/osmo-bts-sysmo/tch.c +++ b/src/osmo-bts-sysmo/tch.c @@ -420,6 +420,7 @@ int l1if_tch_encode(struct gsm_lchan *lchan, uint8_t *data, uint8_t *len, dtx_cache_payload(lchan, rtp_pl, rtp_pl_len, fn, 0); return 1; case ST_SID_U: + case ST_U_NOINH: return -EAGAIN; case ST_FACCH: return -EBADMSG; -- cgit v1.2.3