From acfccb3f028c8417df42de9a6400896eb269a614 Mon Sep 17 00:00:00 2001 From: Max Date: Fri, 28 Oct 2016 16:52:48 +0200 Subject: DTX fix ONSET handling * re-introduce ST_ONSET_F to guard from repetitive ONSET messages in case multiple FACCH occur duriing DTX silence period. * produce ONSET event after both SID FIRST and UPDATE in case of AMR FR. * always dispatch E_SID_F (SID FIRST) signal if in talkspurt. * allow E_SID_* right after ONSET (zero-length talkspurt). * add missing E_ONSET signal description. * fix FSM transitions for AMR HR *Inhibited and First P*. * fix incorrect return from l1if_tch_encode() in ONSET FACCH with incoming SID UPDATE Change-Id: I0e9033c5f169da46aed9a0d1295faff489778dcf Related: OS#1801 --- include/osmo-bts/dtx_dl_amr_fsm.h | 1 + src/common/bts.c | 3 +++ src/common/dtx_dl_amr_fsm.c | 35 ++++++++++++++++++++++++----------- src/common/l1sap.c | 10 ++++++---- src/common/msg_utils.c | 39 ++++++++++++++++++++++++++++----------- src/osmo-bts-litecell15/l1_if.c | 2 +- src/osmo-bts-litecell15/tch.c | 3 +-- src/osmo-bts-sysmo/l1_if.c | 2 +- src/osmo-bts-sysmo/tch.c | 3 +-- 9 files changed, 66 insertions(+), 32 deletions(-) diff --git a/include/osmo-bts/dtx_dl_amr_fsm.h b/include/osmo-bts/dtx_dl_amr_fsm.h index 5c13c198..8b195953 100644 --- a/include/osmo-bts/dtx_dl_amr_fsm.h +++ b/include/osmo-bts/dtx_dl_amr_fsm.h @@ -16,6 +16,7 @@ enum dtx_dl_amr_fsm_states { ST_U_INH, ST_SID_U, ST_ONSET_V, + ST_ONSET_F, ST_FACCH_V, ST_FACCH, }; diff --git a/src/common/bts.c b/src/common/bts.c index 6f621c4d..2005e427 100644 --- a/src/common/bts.c +++ b/src/common/bts.c @@ -40,6 +40,7 @@ #include #include #include +#include #include #include #include @@ -176,6 +177,8 @@ int bts_init(struct gsm_bts *bts) INIT_LLIST_HEAD(&btsb->smscb_state.queue); INIT_LLIST_HEAD(&btsb->oml_queue); + /* register DTX DL FSM */ + osmo_fsm_register(&dtx_dl_amr_fsm); return rc; } diff --git a/src/common/dtx_dl_amr_fsm.c b/src/common/dtx_dl_amr_fsm.c index a75fd00e..50759572 100644 --- a/src/common/dtx_dl_amr_fsm.c +++ b/src/common/dtx_dl_amr_fsm.c @@ -53,7 +53,7 @@ void dtx_fsm_sid_f1(struct osmo_fsm_inst *fi, uint32_t event, void *data) osmo_fsm_inst_state_chg(fi, ST_VOICE, 0, 0); break; case E_FACCH: - osmo_fsm_inst_state_chg(fi, ST_FACCH, 0, 0); + osmo_fsm_inst_state_chg(fi, ST_ONSET_F, 0, 0); break; case E_COMPL: osmo_fsm_inst_state_chg(fi, ST_SID_F2, 0, 0); @@ -81,7 +81,10 @@ void dtx_fsm_sid_f2(struct osmo_fsm_inst *fi, uint32_t event, void *data) osmo_fsm_inst_state_chg(fi, ST_VOICE, 0, 0); break; case E_FACCH: - osmo_fsm_inst_state_chg(fi, ST_FACCH, 0, 0); + osmo_fsm_inst_state_chg(fi, ST_ONSET_F, 0, 0); + break; + case E_ONSET: + osmo_fsm_inst_state_chg(fi, ST_ONSET_V, 0, 0); break; default: LOGP(DL1P, LOGL_ERROR, "Unexpected event %d\n", event); @@ -97,7 +100,7 @@ void dtx_fsm_f1_inh(struct osmo_fsm_inst *fi, uint32_t event, void *data) osmo_fsm_inst_state_chg(fi, ST_ONSET_V, 0, 0); break; case E_FACCH: - osmo_fsm_inst_state_chg(fi, ST_FACCH, 0, 0); + osmo_fsm_inst_state_chg(fi, ST_ONSET_F, 0, 0); break; default: LOGP(DL1P, LOGL_ERROR, "Unexpected event %d\n", event); @@ -110,7 +113,7 @@ void dtx_fsm_u_inh(struct osmo_fsm_inst *fi, uint32_t event, void *data) { switch (event) { case E_VOICE: - osmo_fsm_inst_state_chg(fi, ST_ONSET_V, 0, 0); + osmo_fsm_inst_state_chg(fi, ST_VOICE, 0, 0); break; case E_FACCH: osmo_fsm_inst_state_chg(fi, ST_FACCH, 0, 0); @@ -126,7 +129,7 @@ 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_FACCH, 0, 0); + 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); @@ -152,6 +155,7 @@ void dtx_fsm_sid_upd(struct osmo_fsm_inst *fi, uint32_t event, void *data) void dtx_fsm_onset_v(struct osmo_fsm_inst *fi, uint32_t event, void *data) { switch (event) { + case E_SID_F: case E_SID_U: case E_VOICE: osmo_fsm_inst_state_chg(fi, ST_VOICE, 0, 0); @@ -169,6 +173,7 @@ void dtx_fsm_onset_v(struct osmo_fsm_inst *fi, uint32_t event, void *data) void dtx_fsm_onset_f(struct osmo_fsm_inst *fi, uint32_t event, void *data) { switch (event) { + case E_SID_F: case E_SID_U: break; case E_VOICE: @@ -234,15 +239,15 @@ static struct osmo_fsm_state dtx_dl_amr_fsm_states[] = { 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_FACCH) | X(ST_SID_F2) | X(ST_F1_INH) | X(ST_ONSET_V), + .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), .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), - .out_state_mask = X(ST_SID_U) | X(ST_VOICE) | X(ST_FACCH), + .in_event_mask = X(E_SID_U) | 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, }, @@ -258,24 +263,31 @@ static struct osmo_fsm_state dtx_dl_amr_fsm_states[] = { incoming SPEECH or FACCH (only for AMR HR) */ [ST_U_INH]= { .in_event_mask = X(E_VOICE) | X(E_FACCH), - .out_state_mask = X(ST_VOICE) | X(ST_FACCH_V), + .out_state_mask = X(ST_VOICE) | X(ST_FACCH), .name = "SID-UPDATE (Inh)", .action = dtx_fsm_u_inh, }, /* 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_FACCH) | X(ST_VOICE) | X(ST_U_INH) | X(ST_SID_U) | X(ST_ONSET_V), + .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", .action = dtx_fsm_sid_upd, }, /* ONSET - end of silent period due to incoming SPEECH frame */ [ST_ONSET_V]= { - .in_event_mask = X(E_VOICE) | X(E_FACCH), + .in_event_mask = X(E_SID_F) | X(E_SID_U) | X(E_VOICE) | X(E_FACCH), .out_state_mask = X(ST_VOICE) | X(ST_FACCH_V), .name = "ONSET (SPEECH)", .action = dtx_fsm_onset_v, }, + /* ONSET - end of silent period due to incoming FACCH frame */ + [ST_ONSET_F]= { + .in_event_mask = X(E_VOICE) | X(E_FACCH) | X(E_SID_U) | X(E_SID_F), + .out_state_mask = X(ST_VOICE) | X(ST_FACCH), + .name = "ONSET (FACCH)", + .action = dtx_fsm_onset_f, + }, /* FACCH sending state: SPEECH was observed before so once we're done FSM should get back to VOICE state */ [ST_FACCH_V]= { @@ -296,6 +308,7 @@ static struct osmo_fsm_state dtx_dl_amr_fsm_states[] = { const struct value_string dtx_dl_amr_fsm_event_names[] = { { E_VOICE, "Voice" }, + { E_ONSET, "ONSET" }, { E_FACCH, "FACCH" }, { E_COMPL, "Complete P1 -> P2" }, { E_INHIB, "Inhibit" }, diff --git a/src/common/l1sap.c b/src/common/l1sap.c index 13d8a944..ef248009 100644 --- a/src/common/l1sap.c +++ b/src/common/l1sap.c @@ -1145,10 +1145,12 @@ int l1sap_chan_act(struct gsm_bts_trx *trx, uint8_t chan_nr, struct tlv_parsed * /* Init DTX DL FSM if necessary */ //FIXME: only do it for AMR TCH/* - osmo_fsm_register(&dtx_dl_amr_fsm); - lchan->tch.dtx.dl_amr_fsm = osmo_fsm_inst_alloc(&dtx_dl_amr_fsm, - tall_bts_ctx, lchan, - LOGL_DEBUG, lchan->name); + if (trx->bts->dtxd) + lchan->tch.dtx.dl_amr_fsm = osmo_fsm_inst_alloc(&dtx_dl_amr_fsm, + tall_bts_ctx, + lchan, + LOGL_DEBUG, + lchan->name); return 0; } diff --git a/src/common/msg_utils.c b/src/common/msg_utils.c index 4b213665..851aacb2 100644 --- a/src/common/msg_utils.c +++ b/src/common/msg_utils.c @@ -33,6 +33,8 @@ #include #include +#define STI_BIT_MASK 16 + static int check_fom(struct abis_om_hdr *omh, size_t len) { if (omh->length != len) { @@ -182,14 +184,15 @@ int dtx_dl_amr_fsm_step(struct gsm_lchan *lchan, const uint8_t *rtp_pl, return 0; if (osmo_amr_is_speech(ft)) { - if (lchan->tch.dtx.dl_amr_fsm->state == ST_SID_F1 || - lchan->tch.dtx.dl_amr_fsm->state == ST_SID_U) /* AMR HR */ - if (lchan->type == GSM_LCHAN_TCH_H && marker) - return osmo_fsm_inst_dispatch(lchan->tch.dtx.dl_amr_fsm, - E_INHIB, - (void *)lchan); - /* AMR FR */ - if (marker && lchan->tch.dtx.dl_amr_fsm->state == ST_SID_U) + /* AMR HR - Inhibition */ + if (lchan->type == GSM_LCHAN_TCH_H && marker && + lchan->tch.dtx.dl_amr_fsm->state == ST_SID_F1) + 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 )) 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, @@ -198,6 +201,9 @@ int dtx_dl_amr_fsm_step(struct gsm_lchan *lchan, const uint8_t *rtp_pl, if (ft == AMR_SID) { dtx_cache_payload(lchan, rtp_pl, rtp_pl_len, fn, sti); + if (lchan->tch.dtx.dl_amr_fsm->state == ST_VOICE) + return osmo_fsm_inst_dispatch(lchan->tch.dtx.dl_amr_fsm, + E_SID_F, (void *)lchan); return osmo_fsm_inst_dispatch(lchan->tch.dtx.dl_amr_fsm, sti ? E_SID_U : E_SID_F, (void *)lchan); @@ -215,6 +221,16 @@ int dtx_dl_amr_fsm_step(struct gsm_lchan *lchan, const uint8_t *rtp_pl, return 0; } +static inline void dtx_sti_set(struct gsm_lchan *lchan) +{ + lchan->tch.dtx.cache[6 + 2] |= STI_BIT_MASK; +} +/* STI is located in payload byte 6, cache contains 2 byte prefix (CMR/CMI) */ +static inline void dtx_sti_unset(struct gsm_lchan *lchan) +{ + lchan->tch.dtx.cache[6 + 2] &= ~STI_BIT_MASK; +} + /*! \brief Check if enough time has passed since last SID (if any) to repeat it * \param[in] lchan Logical channel on which we check scheduling * \param[in] fn Frame Number for which we check scheduling @@ -226,9 +242,10 @@ static inline bool dtx_amr_sid_optional(struct gsm_lchan *lchan, uint32_t fn) uint32_t dx26 = 120 * (fn - lchan->tch.dtx.fn); /* We're resuming after FACCH interruption */ - if (lchan->tch.dtx.dl_amr_fsm->state == ST_FACCH) { + if (lchan->tch.dtx.dl_amr_fsm->state == ST_FACCH || + lchan->tch.dtx.dl_amr_fsm->state == ST_ONSET_F) { /* force STI bit to 0 so cache is treated as SID FIRST */ - lchan->tch.dtx.cache[6 + 2] &= ~16; + dtx_sti_unset(lchan); lchan->tch.dtx.is_update = false; osmo_fsm_inst_dispatch(lchan->tch.dtx.dl_amr_fsm, E_SID_F, (void *)lchan); @@ -306,7 +323,7 @@ uint8_t repeat_last_sid(struct gsm_lchan *lchan, uint8_t *dst, uint32_t fn) lchan->tch.dtx.fn = fn; /* enforce SID UPDATE for next repetition - it might have been altered by FACCH handling */ - lchan->tch.dtx.cache[6 + 2] |= 16; + dtx_sti_set(lchan); if (lchan->tch.dtx.dl_amr_fsm->state == ST_SID_U) lchan->tch.dtx.is_update = true; return lchan->tch.dtx.len + 1; diff --git a/src/osmo-bts-litecell15/l1_if.c b/src/osmo-bts-litecell15/l1_if.c index 795172bb..0b1bad4d 100644 --- a/src/osmo-bts-litecell15/l1_if.c +++ b/src/osmo-bts-litecell15/l1_if.c @@ -410,7 +410,7 @@ static int ph_data_req(struct gsm_bts_trx *trx, struct msgb *msg, memcpy(l1p->u.phDataReq.msgUnitParam.u8Buffer, lchan->tch.dtx.facch, msgb_l2len(msg)); else if (trx->bts->dtxd && lchan->tch.dtx.dl_amr_fsm && - lchan->tch.dtx.dl_amr_fsm->state == ST_FACCH) { + lchan->tch.dtx.dl_amr_fsm->state == ST_ONSET_F) { if (sapi == GsmL1_Sapi_FacchF) { sapi = GsmL1_Sapi_TchF; } diff --git a/src/osmo-bts-litecell15/tch.c b/src/osmo-bts-litecell15/tch.c index 70764f5f..4337d680 100644 --- a/src/osmo-bts-litecell15/tch.c +++ b/src/osmo-bts-litecell15/tch.c @@ -328,8 +328,7 @@ int l1if_tch_encode(struct gsm_lchan *lchan, uint8_t *data, uint8_t *len, return -EAGAIN; case ST_FACCH_V: case ST_FACCH: - /* FIXME: if this possible at all? */ - return 0; + return -EBADMSG; default: LOGP(DRTP, LOGL_ERROR, "Unhandled DTX DL AMR FSM state " "%d\n", lchan->tch.dtx.dl_amr_fsm->state); diff --git a/src/osmo-bts-sysmo/l1_if.c b/src/osmo-bts-sysmo/l1_if.c index f7585ce8..51bde8b5 100644 --- a/src/osmo-bts-sysmo/l1_if.c +++ b/src/osmo-bts-sysmo/l1_if.c @@ -405,7 +405,7 @@ static int ph_data_req(struct gsm_bts_trx *trx, struct msgb *msg, memcpy(l1p->u.phDataReq.msgUnitParam.u8Buffer, lchan->tch.dtx.facch, msgb_l2len(msg)); else if (trx->bts->dtxd && lchan->tch.dtx.dl_amr_fsm && - lchan->tch.dtx.dl_amr_fsm->state == ST_FACCH) { + lchan->tch.dtx.dl_amr_fsm->state == ST_ONSET_F) { if (sapi == GsmL1_Sapi_FacchF) { sapi = GsmL1_Sapi_TchF; } diff --git a/src/osmo-bts-sysmo/tch.c b/src/osmo-bts-sysmo/tch.c index fbb42b27..db5ca782 100644 --- a/src/osmo-bts-sysmo/tch.c +++ b/src/osmo-bts-sysmo/tch.c @@ -426,8 +426,7 @@ int l1if_tch_encode(struct gsm_lchan *lchan, uint8_t *data, uint8_t *len, return -EAGAIN; case ST_FACCH_V: case ST_FACCH: - /* FIXME: if this possible at all? */ - return 0; + return -EBADMSG; default: LOGP(DRTP, LOGL_ERROR, "Unhandled DTX DL AMR FSM state " "%d\n", lchan->tch.dtx.dl_amr_fsm->state); -- cgit v1.2.3