diff options
author | Mychaela N. Falconia <falcon@freecalypso.org> | 2023-05-19 05:40:04 +0000 |
---|---|---|
committer | falconia <falcon@freecalypso.org> | 2023-05-19 15:07:09 +0000 |
commit | 9cc85c1a6a89ca1027deb999f0937c5c84515683 (patch) | |
tree | a56fc1730d5978b98ee9b3d9da2cf6fb967ccb81 | |
parent | 2c511d5755dc195e0be49fe5312f3ee654a14748 (diff) |
trx: remove model-specific BFI packet formats
As detailed in OS#6033, osmo-bts-trx was emitting its own invented
packet formats to signal BFIs (bad frame indications), different
from the vty-configured ("rtp continuous-streaming" or not) BFI
signaling method implemented in l1sap and used by all other BTS models.
In the case of EFR codec, the made-up BFI format previously used by
osmo-bts-trx caused EFR operation to be broken: a spec-compliant EFR
decoder on the receiving end of the RTP stream (a network transcoder
or the MS on the other GSM call leg) would receive and decode garbage
EFR frames of 244 zero bits instead of invoking the spec-defined bad
frame handler, causing bad audio artifacts for the user. The same
situation would also happen for FR1 codec, but the application of
ECU masked this bug: with the ECU invoked in the UL output path,
the BFI emitting code for FR1 never executed.
In the case of AMR and HR1 codecs, the model-specific BFI packet
format of osmo-bts-trx is currently harmless, but:
* The extra code adds unnecessary complexity;
* BFI packet formats are inconsistent between osmo-bts-trx and
other OsmoBTS models;
* BFI format is even inconsistent within osmo-bts-trx itself, as
under certain conditions the common code in l1sap will override
the UL payload from the BTS model and emit its own form of BFI
instead.
Fix all of the above by removing trx model-specific BFI formats
for all codecs, and let l1sap handle all BFIs.
Related: OS#6033
Change-Id: I8f9fb5b8c5b2cad4b92ac693c0040779f811981a
-rw-r--r-- | src/osmo-bts-trx/sched_lchan_tchf.c | 34 | ||||
-rw-r--r-- | src/osmo-bts-trx/sched_lchan_tchh.c | 29 |
2 files changed, 10 insertions, 53 deletions
diff --git a/src/osmo-bts-trx/sched_lchan_tchf.c b/src/osmo-bts-trx/sched_lchan_tchf.c index 6d6fb0b1..43a37844 100644 --- a/src/osmo-bts-trx/sched_lchan_tchf.c +++ b/src/osmo-bts-trx/sched_lchan_tchf.c @@ -295,35 +295,11 @@ bfi: goto compose_l1sap; } - switch (tch_mode) { - case GSM48_CMODE_SPEECH_V1: /* FR */ - memset(tch_data, 0, GSM_FR_BYTES); - tch_data[0] = 0xd0; - rc = GSM_FR_BYTES; - break; - case GSM48_CMODE_SPEECH_EFR: /* EFR */ - memset(tch_data, 0, GSM_EFR_BYTES); - tch_data[0] = 0xc0; - rc = GSM_EFR_BYTES; - break; - case GSM48_CMODE_SPEECH_AMR: /* AMR */ - rc = osmo_amr_rtp_enc(tch_data, - chan_state->codec[chan_state->ul_cmr], - chan_state->codec[chan_state->ul_ft], - AMR_BAD); - if (rc < 2) { - LOGL1SB(DL1P, LOGL_ERROR, l1ts, bi, - "Failed to encode AMR_BAD frame (rc=%d), " - "not sending BFI\n", rc); - return -EINVAL; - } - memset(tch_data + sizeof(struct amr_hdr), 0, rc - sizeof(struct amr_hdr)); - break; - default: - LOGL1SB(DL1P, LOGL_ERROR, l1ts, bi, - "TCH mode %u invalid, please fix!\n", tch_mode); - return -EINVAL; - } + /* 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; } } diff --git a/src/osmo-bts-trx/sched_lchan_tchh.c b/src/osmo-bts-trx/sched_lchan_tchh.c index 41965c44..5872c33e 100644 --- a/src/osmo-bts-trx/sched_lchan_tchh.c +++ b/src/osmo-bts-trx/sched_lchan_tchh.c @@ -333,30 +333,11 @@ bfi: goto compose_l1sap; } - switch (tch_mode) { - case GSM48_CMODE_SPEECH_V1: /* HR */ - tch_data[0] = 0x70; /* F = 0, FT = 111 */ - memset(tch_data + 1, 0, 14); - rc = 15; - break; - case GSM48_CMODE_SPEECH_AMR: /* AMR */ - rc = osmo_amr_rtp_enc(tch_data, - chan_state->codec[chan_state->ul_cmr], - chan_state->codec[chan_state->ul_ft], - AMR_BAD); - if (rc < 2) { - LOGL1SB(DL1P, LOGL_ERROR, l1ts, bi, - "Failed to encode AMR_BAD frame (rc=%d), " - "not sending BFI\n", rc); - return -EINVAL; - } - memset(tch_data + sizeof(struct amr_hdr), 0, rc - sizeof(struct amr_hdr)); - break; - default: - LOGL1SB(DL1P, LOGL_ERROR, l1ts, bi, - "TCH mode %u invalid, please fix!\n", tch_mode); - return -EINVAL; - } + /* 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; } } |