laforge submitted this change.

View Change

Approvals: Jenkins Builder: Verified fixeria: Looks good to me, but someone else must approve osmith: Looks good to me, approved
osmo_trau_frame_encode(): reduce space requirement in common case

Since its introduction in 2020, osmo_trau_frame_encode() API has had
the ability to emit TRAU-DL frames that are either lengthened or
shortened for timing adjustment, like real TRAUs do. However, this
capability was never exercised in OsmoMGW-E1, nor is it used in the
new tw-e1abis-mgw: given the constraint of having to submit 160-byte
blocks to the E1 stack every 20 ms, it is not really feasible for
our current sw-only implementation to achieve the same 125 us timing
optimizations that are possible and desirable in a hardware or FPGA
implementation.

Since all current users of osmo_trau_frame_encode() API call it with
fr->dl_ta_usec set to 0, the doubled output space requirement imposed
by the function becomes an unnecessary burden and an API design bug
in need of fixing. Solution: check fr->dir and fr->dl_ta_usec to see
if frame output lengthening is actually possible for each given
invocation of this function, and require doubled output space only
in those cases.

Change-Id: I9f3541b0618ebef26e53b69041e5b74abefd3b85
---
M src/trau/trau_frame.c
1 file changed, 14 insertions(+), 11 deletions(-)

diff --git a/src/trau/trau_frame.c b/src/trau/trau_frame.c
index ad7a102..d0cf3d1 100644
--- a/src/trau/trau_frame.c
+++ b/src/trau/trau_frame.c
@@ -1268,6 +1268,8 @@
*/
int osmo_trau_frame_encode(ubit_t *bits, size_t n_bits, const struct osmo_trau_frame *fr)
{
+ size_t space_req;
+
/* check for sufficient space provided by caller in output buffer */
switch (fr->type) {
case OSMO_TRAU16_FT_FR:
@@ -1276,35 +1278,37 @@
case OSMO_TRAU16_FT_AMR:
case OSMO_TRAU16_FT_IDLE:
/* timing alignment may happen: increased space requirement */
- if (n_bits < 2 * 40 * 8 - 1)
- return -ENOSPC;
+ if (fr->dir == OSMO_TRAU_DIR_DL && fr->dl_ta_usec > 0)
+ space_req = 2 * 40 * 8 - 1;
+ else
+ space_req = 1 * 40 * 8;
break;
case OSMO_TRAU16_FT_OAM:
case OSMO_TRAU16_FT_DATA_HR:
case OSMO_TRAU16_FT_DATA:
case OSMO_TRAU16_FT_D145_SYNC:
case OSMO_TRAU16_FT_EDATA:
- if (n_bits < 1 * 40 * 8)
- return -ENOSPC;
+ space_req = 1 * 40 * 8;
break;
case OSMO_TRAU8_SPEECH:
case OSMO_TRAU8_AMR_LOW:
case OSMO_TRAU8_AMR_6k7:
case OSMO_TRAU8_AMR_7k4:
/* timing alignment may happen: increased space requirement */
- if (n_bits < 2 * 20 * 8 - 1)
- return -ENOSPC;
+ if (fr->dir == OSMO_TRAU_DIR_DL && fr->dl_ta_usec > 0)
+ space_req = 2 * 20 * 8 - 1;
+ else
+ space_req = 1 * 20 * 8;
break;
case OSMO_TRAU8_DATA:
case OSMO_TRAU8_OAM:
- if (n_bits < 1 * 20 * 8)
- return -ENOSPC;
- break;
- case OSMO_TRAU_FT_NONE:
+ space_req = 1 * 20 * 8;
break;
default:
return -EINVAL;
}
+ if (n_bits < space_req)
+ return -ENOSPC;

switch (fr->type) {
case OSMO_TRAU16_FT_FR:
@@ -1338,7 +1342,6 @@
return encode8_amr_67(bits, fr);
case OSMO_TRAU8_AMR_7k4:
return encode8_amr_74(bits, fr);
- case OSMO_TRAU_FT_NONE:
default:
return -EINVAL;
}

To view, visit change 41209. To unsubscribe, or for help writing mail filters, visit settings.

Gerrit-MessageType: merged
Gerrit-Project: libosmo-abis
Gerrit-Branch: master
Gerrit-Change-Id: I9f3541b0618ebef26e53b69041e5b74abefd3b85
Gerrit-Change-Number: 41209
Gerrit-PatchSet: 1
Gerrit-Owner: falconia <falcon@freecalypso.org>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: fixeria <vyanitskiy@sysmocom.de>
Gerrit-Reviewer: laforge <laforge@osmocom.org>
Gerrit-Reviewer: osmith <osmith@sysmocom.de>