Attention is currently required from: laforge, neels, pespin.

View Change

6 comments:

• Patchset:

  • Patch Set #2:

    > and also the BSC has to then be aware of the HR formats in use. […]

    I have so many objections to the argument raised by @nhofmeyr@sysmocom.de that I am not sure where to begin... Just a few thoughts that come to mind:

    • We all collectively, in our role as Osmocom community, got shot in the foot pretty badly when the decision was made, ages ago and long before I joined the project, to use MGCP as the control interface to OsmoMGW, as opposed to a completely custom, ad hoc and private interface. (For an example of the latter, see the control i/f to my tw-border-mgw.) Yes, I know the history behind this design decision: OsmoBSC was interfacing to ip.access proprietary MSC whose proprietary, non-3GPP flavor of A-over-IP interface (known around here as SCCPlite) includes MGCP with endpoints that function like rtpbridge in OsmoMGW. But this historical detail about the way some Osmocom-related commercial business once ran some years ago does nothing to help those of us who are 100% non-profit and FOSS-based (no proprietary MSCs kicking around, no commercial operations) - from my PoV, and from the standpoint of anyone else in a position similar to mine, the use of MGCP (and SDP) in Osmocom is an obstacle, a bone in the throat, an intrusion of IETF-isms into what is otherwise a clean world of GSM governed by 3GPP specs.
    • In terms of functionality, the best RTP format for HRv1 codec is TW-TS-002. For those who cannot or do not wish to use this extended format, the next best choice is RFC 5993 - and the latter is strictly, unambiguously stipulated by 3GPP for use at AoIP interface. The "raw" format of TS 101 318 is quite useful in internal interfaces (low-level library APIs), but use of this raw format in external RTP (external to any given single process) should be phased out.
    • The whole mechanism of on-the-fly HRv1 format conversion in OsmoMGW looks like a solution in search of a problem. What is wrong with simply configuring every OsmoBTS instance to use `rtp hr-format rfc5993` and thus match the requirement of AoIP interface? Is it about supporting ip.access nanoBTS that are stuck with TS 101 318 format? Or another pointless beauty contest, expending extra work just so that existing installations of osmo-bts-sysmo etc that default to `rtp hr-format ts101318` will convert to the right AoIP format "automagically"?
    • The idea keeps being tossed around that OsmoBSC should know which HR format each BTS uses and can therefore pass this info to OsmoMGW via an invented fmtp parameter. The part about inventing an fmtp parameter is trivial - I already did plenty of that for TW-TS-* - instead the problem is with BSC knowledge part. Let us all remember that the HR format emitted by OsmoBTS (all variants) is **configured by vty** on the BTS. How in the world would OsmoBSC know which format is configured by vty on each BTS?

  • Patch Set #2:

    I'd still want the unresolved comments to be adressed somehow.

    There exists a concept called "pick your battles", or "choose your battles wisely". Back in March I tried to do "the right thing" and prepare this patch for Osmocom, even though the feature in question is utterly useless - HRv1 codec has absolutely no use outside of just-for-fun retronetworking experiments, to the point where even American 2G Cooperative, a dedicated operator specifically for users of retro phones and probably the most retronetworking-oriented GSM operator anywhere in the world, won't be using HRv1 in production. And if someone does wish to play with HRv1 just for fun, there are much easier ways to accomplish that exercise, either by configuring `rtp hr-format rfc5993` in OsmoBTS or by enabling TW-TS-002 from MSC/CN side, which propagates down to the BTS and overrides the local `hr-format` setting.

    Therefore, I am driven to conclude that the present issue (OS#6059) and the whole debate around it is not about useful functionality, but more of a beauty contest, a question of which solution would be most aesthetically pleasing to @nhofmeyr@sysmocom.de, @pespin@sysmocom.de and other senior developers whose opinion ranks higher than mine. Therefore, unless those with opposing views soften their stance, I cannot justify spending more of my scarce volunteer time on this issue.

• File include/osmocom/mgcp/mgcp_rtp_end.h:

  • Patch Set #2, Line 40: bool hr_ts101318_detected;

    I would prefer better layer separation for these: […]

    Multipart response, similar to the other comment where deep philosophical issues are involved:

    • Your proposal puts TW-TS-002 "on equals" with TS 101 318 and RFC 5993 in the enum, suggesting that you envision somehow converting between TW-TS-002 and those other formats. Such conversion is not possible and shows a fundamental misunderstanding: TW-TS-002 is not just a different representation format for the same information content, instead there is additional information content with TW-TS-002 that simply does not exist with either of the two "standard" formats. Hence there is no possibility of conversion.
    • The only correct way to handle TW-TS-002 is the way I handle it in my patch: treat it as a request from OsmoBSC that basically says "dear OsmoMGW, please stand down and disable all of your smart conversions, just pass the payload through and leave it alone".
    • The rest of your enum proposal I simply don't understand, and as already explained, I am running into a limit of how much volunteer work I can put into this issue that is purely for the heck of it, with no practical use that I can see.

• File src/libosmo-mgcp/mgcp_network.c:

  • Patch Set #2, Line 715: static int rfc5993_hr_convert(struct msgb *msg, struct mgcp_conn_rtp *dst)

    It is an artifact of how I developed this code in stages. […]

    Update 7 months later: I'll spin an updated patch with these arguments swapped.

  • Patch Set #2, Line 1577: Th

    s/This internal function examines/Examine/

    What language do you then propose for the "it is invoked when..." part?

  • Patch Set #2, Line 1658: Autodetect

    s/between // ?

    Removing "between" would make the imperative sentence ungrammatical.

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