Bug #5974
closedWhen TCH UL frames are stolen for FACCH, RTP stream from sysmoBTS is messed up
100%
Description
This bug is specific to sysmoBTS platform and osmo-bts-sysmo. As I understand it, when a TCH/F is active, in every 20 ms time window the PHY sends either a GsmL1_Sapi_TchF message or a GsmL1_Sapi_FacchF message to the unit's Linux processor, but not both. In other words, if a TCH UL frame was stolen for FACCH, there will be a GsmL1_Sapi_FacchF in that time window, but no GsmL1_Sapi_TchF. However, the code path that eventually leads to RTP output or at least an RTP timestamp increment is invoked only for GsmL1_Sapi_TchF, and as a result the 20 ms time window with FACCH is missed altogether for RTP purposes. The resulting effect in the emitted RTP stream is that the sequence number increments by 1 and the timestamp increments by 160 between two successive packets, but the time interval between them is 40 ms instead of 20 ms - in other words, the timestamp increment fails to reflect the actual timing reality.
I have only tested with FR1 and EFR, but I reason that the same behavior most likely occurs with AMR and HR1 too.
Files
Related issues
Updated by laforge about 1 year ago
- Status changed from New to In Progress
- Assignee set to laforge
Updated by laforge about 1 year ago
- Status changed from In Progress to Feedback
- Assignee changed from laforge to falconia
- % Done changed from 0 to 30
proposed patch in https://gerrit.osmocom.org/c/osmo-bts/+/32082 - if you're interested, please give it a try.
Updated by falconia about 1 year ago
- File os5974-themwi.patch os5974-themwi.patch added
- Status changed from Feedback to Stalled
- Parent task set to #5975
laforge wrote in #note-2:
proposed patch in https://gerrit.osmocom.org/c/osmo-bts/+/32082 - if you're interested, please give it a try.
I won't be able to actually test the above, or any other proposed patch that addresses only the present issue by itself, without also addressing #5975, because I currently run with local patches that handle #5975 (plus the present issue) and my environment can't work without that other issue being addressed. However, looking at the proposed patch in Gerrit, I see that the code in l1if_tch_rx() will dereference an uninitialized automatic var pointer data_ind (when calling add_l1sap_header()) when it is called with (l1p_msg == NULL).
Please look at os5974-themwi.patch for an example of how I currently handle that issue. I won't be pushing that patch to Gerrit as-is (that's why I attached it here instead), but I am already working on a Gerrit-able patch for #5975, and then I will push a patch series to Gerrit that will address #5975 and then the present issue in this order.
Updated by falconia about 1 year ago
- Related to Feature #5975: Provide a configurable option to emit a continuous RTP stream without gaps added
Updated by laforge about 1 year ago
- Status changed from Stalled to Resolved
- % Done changed from 30 to 100
Applied in changeset osmo-bts|89155afa3fe7a1c9458eedf7791546b98b19035c.
Updated by falconia about 1 year ago
- Status changed from Resolved to In Progress
laforge wrote in #note-6:
Applied in changeset osmo-bts|89155afa3fe7a1c9458eedf7791546b98b19035c.
The above commit does not constitute a working fix: this code will most likely segfault when it dereferences the data_ind pointer which remains uninitialized in the case when l1if_tch_rx() is called with a NULL l1p_msg argument.
Updated by falconia about 1 year ago
- % Done changed from 100 to 80
I will be submitting an actually working patch for this bug to Gerrit in probably no more than a few hours from now, as part of a patch series that will also address #5975.
Updated by falconia about 1 year ago
Updated by falconia about 1 year ago
- Status changed from In Progress to Resolved
- % Done changed from 80 to 100
The bug is fixed for TCH/F with this commit in master:
https://cgit.osmocom.org/osmo-bts/commit/?id=61e7fdbb3fac93e0b15a9820b0d81dc4b5f20864
Because the problem is solved for TCH/F, I am closing this bug ticket as resolved. If the problem still exists for TCH/H (it would have to be tested, we shouldn't simply assume), then whoever cares about TCH/H will need to open a separate ticket for it.
