[M] Change in osmo-bts[master]: ECU in UL path: move it from trx model to l1sap

falconia has submitted this change. ( https://gerrit.osmocom.org/c/osmo-bts/+/33471 ) Change subject: ECU in UL path: move it from trx model to l1sap ...................................................................... ECU in UL path: move it from trx model to l1sap With this change the application of ECU in the uplink path becomes consistent across all OsmoBTS models, enabled or disabled per vty config setting "rtp internal-uplink-ecu". An additional behavioral change from the previous trx-model-only implementation is that ECU insertion is now done after the link quality check in l1sap, thereby fixing the bug where this quality check would sometimes suppress ECU output and replace it with BFI markers in RTP. In the new implementation when the internal ECU is enabled and available for the selected codec (currently FRv1 only), the RTP output will gap (standard representation of BFI in RTP) only during DTXu pauses as indicated by a received SID frame (either valid or invalid), and the SID frame that triggers the switch from ECU mode into pause mode is reliably emitted in RTP. Related: OS#6040 Depends: I3857be84bba12aaca0c2cca91458b7e13c5a642a (libosmocore) Change-Id: Iac577975c9ab50cb8ebbc035c661c1880e7cecec --- M src/common/l1sap.c M src/osmo-bts-trx/sched_lchan_tchf.c M src/osmo-bts-trx/sched_lchan_tchh.c 3 files changed, 79 insertions(+), 69 deletions(-) Approvals: fixeria: Looks good to me, approved pespin: Looks good to me, but someone else must approve Jenkins Builder: Verified diff --git a/src/common/l1sap.c b/src/common/l1sap.c index 981d3bc..7f61f2c 100644 --- a/src/common/l1sap.c +++ b/src/common/l1sap.c @@ -1813,6 +1813,51 @@ send_ul_rtp_packet(lchan, fn, msg->data, msg->len); } +/* A helper function for l1sap_tch_ind(): handling BFI + * + * Please note that we pass the msgb to this function, even though it is + * currently not used. This msgb passing is a provision for adding + * support for TRAU-UL-like RTP payload formats like TW-TS-001 that allow + * indicating BFI along with deemed-bad frame data bits, just like + * GSM 08.60 and 08.61 TRAU-UL frames. + */ +static void tch_ul_bfi_handler(struct gsm_lchan *lchan, + const struct gsm_time *g_time, struct msgb *msg) +{ + uint32_t fn = g_time->fn; + uint8_t ecu_out[GSM_FR_BYTES]; + int rc; + + /* Are we applying an ECU to this uplink, and are we in a state + * (not DTX pause) where we emit ECU output? */ + if (lchan->ecu_state && !osmo_ecu_is_dtx_pause(lchan->ecu_state)) { + rc = osmo_ecu_frame_out(lchan->ecu_state, ecu_out); + /* did it actually give us some output? */ + if (rc > 0) { + /* yes, send it out in RTP */ + send_ul_rtp_packet(lchan, fn, ecu_out, rc); + return; + } + } + + /* Are we in rtp continuous-streaming special mode? If so, send out + * a BFI packet as zero-length RTP payload. */ + if (lchan->ts->trx->bts->rtp_nogaps_mode) { + send_ul_rtp_packet(lchan, fn, NULL, 0); + return; + } + + /* Most classic form of BFI handling: generate an intentional gap + * in the outgoing RTP stream. */ + LOGPLCGT(lchan, g_time, DRTP, LOGL_DEBUG, + "Skipping RTP frame with lost payload\n"); + if (lchan->abis_ip.osmux.use) + lchan_osmux_skipped_frame(lchan, fn_ms_adj(fn, lchan)); + else if (lchan->abis_ip.rtp_socket) + osmo_rtp_skipped_frame(lchan->abis_ip.rtp_socket, fn_ms_adj(fn, lchan)); + lchan->rtp_tx_marker = true; +} + /* TCH received from bts model */ static int l1sap_tch_ind(struct gsm_bts_trx *trx, struct osmo_phsap_prim *l1sap, struct ph_tch_param *tch_ind) @@ -1851,6 +1896,9 @@ * available is expected as empty payload. We also check if quality is * good enough. */ if (msg->len && tch_ind->lqual_cb >= bts->min_qual_norm) { + /* feed the good frame to the ECU, if we are applying one */ + if (lchan->ecu_state) + osmo_ecu_frame_in(lchan->ecu_state, false, msg->data, msg->len); /* hand msg to RTP code for transmission */ if (bts->emit_hr_rfc5993 && lchan->type == GSM_LCHAN_TCH_H && lchan->tch_mode == GSM48_CMODE_SPEECH_V1) @@ -1865,19 +1913,7 @@ return 1; } } else { - /* Are we in rtp continuous-streaming special mode? If so, send - * out a BFI packet as zero-length RTP payload. */ - if (bts->rtp_nogaps_mode) { - send_ul_rtp_packet(lchan, fn, NULL, 0); - } else { - LOGPLCGT(lchan, &g_time, DRTP, LOGL_DEBUG, - "Skipping RTP frame with lost payload (chan_nr=0x%02x)\n", chan_nr); - if (lchan->abis_ip.osmux.use) - lchan_osmux_skipped_frame(lchan, fn_ms_adj(fn, lchan)); - else if (lchan->abis_ip.rtp_socket) - osmo_rtp_skipped_frame(lchan->abis_ip.rtp_socket, fn_ms_adj(fn, lchan)); - lchan->rtp_tx_marker = true; - } + tch_ul_bfi_handler(lchan, &g_time, msg); } lchan->tch.last_fn = fn; diff --git a/src/osmo-bts-trx/sched_lchan_tchf.c b/src/osmo-bts-trx/sched_lchan_tchf.c index 0b709bc..9acbf31 100644 --- a/src/osmo-bts-trx/sched_lchan_tchf.c +++ b/src/osmo-bts-trx/sched_lchan_tchf.c @@ -30,7 +30,6 @@ #include <osmocom/gsm/gsm0502.h> #include <osmocom/codec/codec.h> -#include <osmocom/codec/ecu.h> #include <osmocom/coding/gsm0503_coding.h> #include <osmocom/coding/gsm0503_amr_dtx.h> @@ -247,12 +246,9 @@ bfi_flag = true; } - if (rc != GSM_MACBLOCK_LEN && lchan->ecu_state) - osmo_ecu_frame_in(lchan->ecu_state, bfi_flag, tch_data, rc); - ber10k = compute_ber10k(n_bits_total, n_errors); if (bfi_flag) - goto bfi; + rc = 0; /* this is how we signal BFI to l1sap */ /* FACCH */ if (rc == GSM_MACBLOCK_LEN) { @@ -270,36 +266,13 @@ * the fake (BFI) TCH indication we set meas_avg.rssi to zero. * Doing so tells l1sap.c to ignore the measurement result. */ meas_avg.rssi = 0; - -bfi: - if (rsl_cmode == RSL_CMOD_SPD_SPEECH) { - /* indicate bad frame */ - if (lchan->tch.dtx.ul_sid) { - /* DTXu: pause in progress. Push empty payload to upper layers */ - rc = 0; - goto compose_l1sap; - } - - /* If there is an ECU active on this channel, use its output */ - if (lchan->ecu_state) { - rc = osmo_ecu_frame_out(lchan->ecu_state, tch_data); - if (rc >= 0) /* Otherwise we send a BFI */ - goto compose_l1sap; - } - - /* In order to signal BFI in our UL RTP output, we need - * to push an empty payload to l1sap. The upper layer - * will choose the correct RTP representation of this - * BFI based on model-independent vty config. */ - rc = 0; - } + rc = 0; } if (rsl_cmode != RSL_CMOD_SPD_SPEECH) return 0; /* TCH or BFI */ -compose_l1sap: return _sched_compose_tch_ind(l1ts, fn_begin, bi->chan, tch_data, rc, meas_avg.toa256, ber10k, meas_avg.rssi, meas_avg.ci_cb, is_sub); diff --git a/src/osmo-bts-trx/sched_lchan_tchh.c b/src/osmo-bts-trx/sched_lchan_tchh.c index c91569f..8d1aab3 100644 --- a/src/osmo-bts-trx/sched_lchan_tchh.c +++ b/src/osmo-bts-trx/sched_lchan_tchh.c @@ -30,7 +30,6 @@ #include <osmocom/gsm/gsm0502.h> #include <osmocom/codec/codec.h> -#include <osmocom/codec/ecu.h> #include <osmocom/coding/gsm0503_coding.h> #include <osmocom/coding/gsm0503_amr_dtx.h> @@ -285,11 +284,8 @@ bfi_flag = true; } - if (rc != GSM_MACBLOCK_LEN && lchan->ecu_state) - osmo_ecu_frame_in(lchan->ecu_state, bfi_flag, tch_data, rc); - if (bfi_flag) - goto bfi; + rc = 0; /* this is how we signal BFI to l1sap */ /* FACCH */ if (rc == GSM_MACBLOCK_LEN) { @@ -307,33 +303,12 @@ bfi: /* A FACCH/H frame replaces two speech frames, so we need to send two BFIs. * One is sent here, another will be sent two bursts later (see above). */ - if (rsl_cmode == RSL_CMOD_SPD_SPEECH) { - /* indicate bad frame */ - if (lchan->tch.dtx.ul_sid) { - /* DTXu: pause in progress. Push empty payload to upper layers */ - rc = 0; - goto compose_l1sap; - } - - /* If there is an ECU active on this channel, use its output */ - if (lchan->ecu_state) { - rc = osmo_ecu_frame_out(lchan->ecu_state, tch_data); - if (rc >= 0) /* Otherwise we send a BFI */ - goto compose_l1sap; - } - - /* In order to signal BFI in our UL RTP output, we need - * to push an empty payload to l1sap. The upper layer - * will choose the correct RTP representation of this - * BFI based on model-independent vty config. */ - rc = 0; - } + rc = 0; } if (rsl_cmode != RSL_CMOD_SPD_SPEECH) return 0; -compose_l1sap: /* TCH or BFI */ return _sched_compose_tch_ind(l1ts, fn_begin, bi->chan, tch_data, rc, meas_avg.toa256, ber10k, meas_avg.rssi, -- To view, visit https://gerrit.osmocom.org/c/osmo-bts/+/33471 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-bts Gerrit-Branch: master Gerrit-Change-Id: Iac577975c9ab50cb8ebbc035c661c1880e7cecec Gerrit-Change-Number: 33471 Gerrit-PatchSet: 3 Gerrit-Owner: falconia &lt;falcon(a)freecalypso.org&gt; Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: falconia &lt;falcon(a)freecalypso.org&gt; Gerrit-Reviewer: fixeria &lt;vyanitskiy(a)sysmocom.de&gt; Gerrit-Reviewer: pespin &lt;pespin(a)sysmocom.de&gt; Gerrit-MessageType: merged