Bug #5688
closedosmo-bts-sysmo and osmo-bts-trx use different GSM-HR RTP packet format
Added by dexter over 1 year ago. Updated 6 months ago.
100%
Description
When comparing the RTP packet format that is used by osmo-bts-sysmo and osmo-bts-trx it becomes obvious that osmo-bts-trx uses RFC5993 (35 byte payload), while osmo-bts-sysmo uses TS 101.318 (34 byte payload). The reason for this is that osmo-bts-trx is using the encoding function in libosmocore gsm0503_coding.c, while osmo-bts-sysmo uses the encoder and decoder functions of its DSP.
We should make sure that all osmo-bts versions always use the same format. For osmo-bts-sysmo that would mean that we need to convert to RFC5993. The conversion is simple, we just need to prepend one byte ToC at the beginning of each RTP packet we emit.
In the receiving path we should make sure that both formats are accepted. Since the BTS knows when GSM-HR is in use we could just check the length of the packets to distinguish which of the two formats we receive
Optionally it might also make the format configurable. Preferably using a proprietary RSL IE so that the config value can be in the osmo-bsc.cfg.
Related issues
Updated by dexter over 1 year ago
Note: The same could be done for AMR BWE aligned packets which. Those are dropped in l1sap.c. We could also just convert them.
Updated by dexter over 1 year ago
- % Done changed from 0 to 10
I have now submitted a patch that lets osmo-bts accept any of the two HR GSM formats:
https://gerrit.osmocom.org/c/osmo-bts/+/30523 l1sap: Accept RFC5993 and TS 101.318 HR GSM payload
The format conversion is done with minimal effort, so that no additional memcpy() or buffers are introduced. However, even if we now accept both formats we still may emit the wrong format. One idea to get around this would be to make the TX format adaptive so that we always send RTP packets in the same format as we receive, but that will be in a follow up patch.
Updated by dexter over 1 year ago
- % Done changed from 10 to 80
The problem on the receiving end of the BTS should now be solved since the BTS will now accept both formats. However, the receiving side is still a problem. We already have a various extra flags in the ASSIGNMENT COMMAND. I added one to signal the type of the HR GSM RTP audio. So we now can configure which format the BTS should use.
https://gerrit.osmocom.org/c/osmo-bts/+/30629 rsl: allow configuration of HR GSM RTP format via CHANNEL ACTIVATE [NEW]
https://gerrit.osmocom.org/c/osmo-bsc/+/30630 abis_rsl: signal HR GSM RTP format to BTS via RSL [NEW]
Updated by dexter over 1 year ago
We are currently discussing if it is such a good idea to configure the format via RSL, it may be better to have it directly in the osmo-bts.cfg: https://gerrit.osmocom.org/c/libosmocore/+/30724
Updated by dexter about 1 year ago
- % Done changed from 80 to 50
As discussed we now follow a more generic approach. The MGW will get the information about which of the two GSM HR format is used via SDP from the BSC. The BSC is fully aware of the BTS type and therefore also has the information which format is used.
There is a patch for the MGW and a related patch for the TTCN3 testsuite in gerrit:
https://gerrit.osmocom.org/c/osmo-mgw/+/31214
https://gerrit.osmocom.org/c/osmo-ttcn3-hacks/+/31205
Updated by dexter about 1 year ago
I have updated the patch for osmo-mgw so that it now also generates the FMTP string using the osmo-mgcp-client.
The patch for osmo-bts has also received an update. We still accept both formats but now we have to use BTS feature flag so that the info about the preferred Format is available to the BSC.
https://gerrit.osmocom.org/c/osmo-bts/+/31417
Updated by dexter about 1 year ago
- Status changed from In Progress to Stalled
This task is currently blocked by #5688 due to a conflict in the osmo-mgcp-client API.
Updated by falconia 11 months ago
- Related to Feature #6036: Implement ternary SID classification for HR1 uplink added
Updated by falconia 11 months ago
The way in which this bug has been handled so far has been rather disappointing. In my opinion, this bug ticket conflates two separate problems:
Issue 1: in the realm of interoperable interfaces between components from different vendors, there exist two different RTP encoding formats for HR1.
Issue 2: internally to OsmoBTS, libosmocoding functions used by osmo-bts-trx are poorly designed in that they natively implement a pseudo-RFC5993 format, causing osmo-bts-trx to treat RFC 5993 as its native.
These issues need to be addressed separately. For Issue 1 (interoperability), the best principle is "be conservative in what you send and liberal in what you accept": have OsmoBTS accept both formats in RTP input, and produce one format or the other on output per vty option, consistent across all models.
In previous comments on this ticket and related patch proposals, there is a recurring but misguided notion that different PHY implementations may have either TS 101 318 or RFC 5993 as their "native" format for HR1, and that higher-level design of OsmoBTS (and even supporting library functions like SID checks, ECUs etc) needs to dance around these two different HR1 formats percolating up and down the stack. I contest this idea:
- Thinking in philosophical terms, the "true" HR1 frame payload, as in a Platonic ideal or a Kantian thing-in-itself, is 112 bits or 14 octets, whereas TS 101 318 and RFC 5993 are two different external representations of this Platonic ideal. It just so happens that the "true internal" Platonic ideal form of HR1 and the TS 101 318 representation are bitwise identical - but philosophically they are separate.
- By the very nature of what a PHY needs to do (GSM 05.03 channel encoding and decoding), BTS PHYs should operate on the "Platonic ideal" form of HR1 frame payload (which just happens to be the same as TS 101 318), whereas external representation may be that or RFC 5993.
- gsm0503_tch_hr_decode() does not actually emit RFC 5993 output, instead it emits pseudo-5993 or bogo-5993: it emits one constant 0x00 octet followed by 14 octets of "Platonic ideal" HR1 payload. This output is NOT valid RFC 5993: in true RFC 5993 the FT field in the ToC octet needs to be set to the result of SID classification decision (see #6036), but gsm0503_tch_hr_decode() always writes 0. gsm0503_tch_hr_encode() should likewise be seen as accepting payloads consisting of one totally ignored octet (no ToC checks are done) followed by 14 octets of "true" HR1 payload.
- I propose this fix to libosmocoding: extend gsm0503_tch_hr_encode() so it will accept both 14-byte and 15-byte formats (the latter being only for backward compat with current bad design, with the extra octet ignored), and add gsm0503_tch_hr_decode2() function that will be exactly like current gsm0503_tch_hr_decode(), but without the extra bogon octet in the output.
- In the extremely unlikely case that we run into a third-party proprietary PHY that takes and/or emits HR1 payload in a form that is claimed to be RFC 5993, we'll need to experiment with it (behavioral reverse eng) to see what it actually does with the extra octet in the input, and what it puts into that extra octet in the output - and then decide from there on the most appropriate course of action.
The takeaway is that we should streamline OsmoBTS code to treat the 14-octet form of HR1 as canonical internally, and implement RFC 5993 at the point of external interfaces. Ironically, however, despite the internal form being the 14-octet one, the preferred external form should be RFC 5993, with TS 101 318 as the non-preferred alternative. Why? See #6036 for the rationale.
If the community is agreeable to the ideas I just outlined, I am willing to implement them in code, i.e., prepare and submit the necessary patches to libosmocodec, libosmocoding and osmo-bts, covering both the present ticket and #6036. However, I will only be able to begin this HR1 codec work after all of my currently outstanding patches for FR and EFR codecs have been merged.
Updated by falconia 11 months ago
- Status changed from Stalled to In Progress
- Assignee changed from dexter to falconia
The libosmocoding portion of my proposed solution has already been merged:
https://cgit.osmocom.org/libosmocore/commit/?id=8d8d5af957dd0abecc9f77d311f59a9076c2585a
https://cgit.osmocom.org/libosmocore/commit/?id=57a3b3a51fff40cff46c632285cd91e7458bf84e
In just a few hours from now I will have a clean patch to osmo-bts that makes all models accept both formats in RTP output and emit one or the other format per vty configuration, thereby fully solving the present bug ticket.
Updated by falconia 11 months ago
Brown paper bag moment:
https://gerrit.osmocom.org/c/libosmocore/+/32967
Please forgive your old 1980s UNIX gal, this whole DSO business is rather foreign to me and smells too much like Windows DLLs.
Updated by laforge 11 months ago
On Tue, May 23, 2023 at 07:11:07PM +0000, falconia wrote:
Please forgive your old 1980s UNIX gal, this whole DSO business is rather foreign to me and smells too much like Windows DLLs.
note that symbol visibility in ELF .so files is a 90s topic, I'm quite sure :P
I find it rather useful to be able to have symbols shared between files within a shared library,
but not expose those to the global symbol namespace of the application. Yes, usability with the .map
file sort-of sucks, but the goal justifies the means.
Updated by falconia 11 months ago
- Status changed from In Progress to Feedback
- % Done changed from 50 to 100
The following patch series implements all needed functionality:
https://gerrit.osmocom.org/c/osmo-bts/+/32968
https://gerrit.osmocom.org/c/osmo-bts/+/32969
https://gerrit.osmocom.org/c/osmo-bts/+/32970
Updated by dexter 11 months ago
- Status changed from Feedback to In Progress
Thank you very much for contributing. You are welcome to continue this ticket. I was approaching the problem from two sides:
1) The MGW currently converts HR audio by just converting it in the opposite format. This of course may lead to confusion. It works most setups and as far as I know we never had problems with this. But it definitely will become a problem when the BSC has to manage BTSs that have opposite formats.
There is a patch for osmo-mgw (https://gerrit.osmocom.org/c/osmo-mgw/+/31214) that introduces an fmtp string to signal the HR format to osmo-mgw. When this feature is used, the HR format is explicitly negotiated.
Unfortunately the patch is blocked because it does modifications to the osmo-mgcp-client API that has a design problem. We have decided to fix that design problem first (see https://osmocom.org/issues/5723, we also have decided how this has to be fixed, I will start doing this as soon there is time to do it).
Technically this is an entirely different location, but I wanted to make you aware of this part. (The progress is set to 100% now but since the MGW part is not yet done I was setting it to 50%)
2) Different osmo-bts implementation may use a different HR format. This is of course a problem which needs to be resolved. I was trying to solve this in two steps: First I wanted to make sure that no incompatible payload enters the BTS code, so my Idea was to filter non supported formats. In order to know which format is supported I intended to use BTS feature flags.
For convenience I intended to convert arriving RTP payloads that do not match the supported HR format. This is not strictly needed and one might to argue about this. After all payloads in the wrong format are the result of another problem and maybe it is better to have a completely silent call rather then a call that is silent in one direction only.
The following patches are currently in review, maybe you can re-use parts of it:
https://gerrit.osmocom.org/c/osmo-bts/+/31417 l1sap: Drop invalid HR GSM payload
https://gerrit.osmocom.org/c/osmo-bts/+/32630 l1sap: Accept RFC5993 and TS 101.318 HR GSM payload
https://gerrit.osmocom.org/c/osmo-bts/+/32752 l1sap: cosmetic: rename payload_len to rtp_pl_len
The next step would have been to add a way to configure the HR format that the BTS should transmit in the core network direction. We already discussed this and opted for a simple VTY option in osmo-bts. As I can see you already implemented this part.
From what I can see we are basically done on the BTS side, we only have to get those patches through review. The ticket is currently assigned to you. You can assign it back to me when the BTS side is done.
Updated by falconia 11 months ago
- % Done changed from 100 to 80
dexter - thanks for explaining the background, and what happened before I jumped in.
The MGW currently converts HR audio by just converting it in the opposite format. This of course may lead to confusion. It works most setups and as far as I know we never had problems with this.
I wonder about that "works most setups" part - does anyone among "real" network deployments (beyond Osmocom developers testing everything) actually use HRv1 codec, and if so, why? I thought I was the only person in the Osmocom community who is interested in deploying GSM networks specifically for providing service to vintage mobile phones, with all other "real" Osmocom network operators assuming that user handsets are reasonably "modern" - and AMR support in handsets goes back to at least 2005, or probably a few years earlier with manufs who didn't drag their feet. And if you have AMR support, why would a real operator run with HRv1 rather than AMR-HR?
I personally am not really interested in deploying this HR codec either - although I am doing full retro, I seek to mimic the practices of the existing commercial GSM network in USA that is about to be shut down - and as far as I know, they never used HR. I've experimented with sending different lists of supported speech codec versions in the Bearer cap IE from a test MS, and I can never get the network to give me TCH/HS, even if I construct a weird Bearer cap IE that says that the MS prefers HR over everything else. Instead this network always goes for AMR-HR if the MS supports it; if the MS lacks AMR support, then the network allocates TCH/F instead of TCH/H and selects EFR if the MS supports it, or FR otherwise. The specs require every MS to support FRv1, with all others being optional in any combination.
Back to the present Osmocom ticket, I took this issue up and I am pursuing my preferred solution (OsmoBTS part) in Gerrit because I have other patches that I care about for FR & EFR, the two non-AMR codecs I actually use in my network deployment - and because DTX specs are mostly consistent between FR/EFR and HR (the only major diff is the lack of fixed bit-counting rules for HR, see #6036), the work I am doing for FR/EFR also needs to extend to HR1. And if your "competing" osmo-bts patches for the present issue were to be merged, that solution would completely preclude any possibility of me extending my FR/EFR work to also cover HR the same way - hence me jumping in passionately with a counter-proposal of different design.
Updated by falconia 11 months ago
- Status changed from In Progress to Resolved
- % Done changed from 80 to 100
Applied in changeset osmo-bts|c18f9f256c905d845c0060f19f3cd46cc40a2fe5.
Updated by neels 6 months ago
- Related to Feature #6171: mgcp-client API: there should be a separate mgcp_codec_param for each codec in mgcp_msg.codecs[] added
Updated by neels 6 months ago
possible confusion: this ticket is resolved but related patches are still in heavy review?
https://gerrit.osmocom.org/c/osmo-mgw/+/31214
