mirror of
https://gitea.osmocom.org/cellular-infrastructure/osmo-mgw.git
synced 2025-10-23 08:12:01 +00:00
Update patch set 2
Patch Set 2: (6 comments) Patch-set: 2 Attention: {"person_ident":"Gerrit User 1000230 \u003c1000230@035e6965-6537-41bd-912c-053f3cf69326\u003e","operation":"REMOVE","reason":"\u003cGERRIT_ACCOUNT_1000230\u003e replied on the change"} Attention: {"person_ident":"Gerrit User 1000004 \u003c1000004@035e6965-6537-41bd-912c-053f3cf69326\u003e","operation":"ADD","reason":"\u003cGERRIT_ACCOUNT_1000230\u003e replied on the change"}
This commit is contained in:
committed by
Gerrit Code Review
parent
60882f2161
commit
61188fedd6
@@ -70,6 +70,42 @@
|
|||||||
"revId": "bf44c8c782b83deb8c1dba7e6bc91f628313f76f",
|
"revId": "bf44c8c782b83deb8c1dba7e6bc91f628313f76f",
|
||||||
"serverId": "035e6965-6537-41bd-912c-053f3cf69326"
|
"serverId": "035e6965-6537-41bd-912c-053f3cf69326"
|
||||||
},
|
},
|
||||||
|
{
|
||||||
|
"unresolved": true,
|
||||||
|
"key": {
|
||||||
|
"uuid": "47cfabc0_4116232d",
|
||||||
|
"filename": "/PATCHSET_LEVEL",
|
||||||
|
"patchSetId": 2
|
||||||
|
},
|
||||||
|
"lineNbr": 0,
|
||||||
|
"author": {
|
||||||
|
"id": 1000230
|
||||||
|
},
|
||||||
|
"writtenOn": "2025-10-22T04:00:46Z",
|
||||||
|
"side": 1,
|
||||||
|
"message": "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\u0027t 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.\n\nTherefore, 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.",
|
||||||
|
"parentUuid": "22d57bdb_3501e12c",
|
||||||
|
"revId": "bf44c8c782b83deb8c1dba7e6bc91f628313f76f",
|
||||||
|
"serverId": "035e6965-6537-41bd-912c-053f3cf69326"
|
||||||
|
},
|
||||||
|
{
|
||||||
|
"unresolved": true,
|
||||||
|
"key": {
|
||||||
|
"uuid": "36108426_0b2678b6",
|
||||||
|
"filename": "/PATCHSET_LEVEL",
|
||||||
|
"patchSetId": 2
|
||||||
|
},
|
||||||
|
"lineNbr": 0,
|
||||||
|
"author": {
|
||||||
|
"id": 1000230
|
||||||
|
},
|
||||||
|
"writtenOn": "2025-10-22T04:00:46Z",
|
||||||
|
"side": 1,
|
||||||
|
"message": "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:\n\n* 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.\n\n* 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.\n\n* 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\"?\n\n* 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?",
|
||||||
|
"parentUuid": "a309e13b_f5b14638",
|
||||||
|
"revId": "bf44c8c782b83deb8c1dba7e6bc91f628313f76f",
|
||||||
|
"serverId": "035e6965-6537-41bd-912c-053f3cf69326"
|
||||||
|
},
|
||||||
{
|
{
|
||||||
"unresolved": true,
|
"unresolved": true,
|
||||||
"key": {
|
"key": {
|
||||||
@@ -87,6 +123,24 @@
|
|||||||
"revId": "bf44c8c782b83deb8c1dba7e6bc91f628313f76f",
|
"revId": "bf44c8c782b83deb8c1dba7e6bc91f628313f76f",
|
||||||
"serverId": "035e6965-6537-41bd-912c-053f3cf69326"
|
"serverId": "035e6965-6537-41bd-912c-053f3cf69326"
|
||||||
},
|
},
|
||||||
|
{
|
||||||
|
"unresolved": true,
|
||||||
|
"key": {
|
||||||
|
"uuid": "435bc440_72a4b6c2",
|
||||||
|
"filename": "include/osmocom/mgcp/mgcp_rtp_end.h",
|
||||||
|
"patchSetId": 2
|
||||||
|
},
|
||||||
|
"lineNbr": 40,
|
||||||
|
"author": {
|
||||||
|
"id": 1000230
|
||||||
|
},
|
||||||
|
"writtenOn": "2025-10-22T04:00:46Z",
|
||||||
|
"side": 1,
|
||||||
|
"message": "Multipart response, similar to the other comment where deep philosophical issues are involved:\n\n* 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.\n\n* 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\".\n\n* The rest of your enum proposal I simply don\u0027t 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.",
|
||||||
|
"parentUuid": "54a61fd4_fe289753",
|
||||||
|
"revId": "bf44c8c782b83deb8c1dba7e6bc91f628313f76f",
|
||||||
|
"serverId": "035e6965-6537-41bd-912c-053f3cf69326"
|
||||||
|
},
|
||||||
{
|
{
|
||||||
"unresolved": true,
|
"unresolved": true,
|
||||||
"key": {
|
"key": {
|
||||||
@@ -335,6 +389,24 @@
|
|||||||
"revId": "bf44c8c782b83deb8c1dba7e6bc91f628313f76f",
|
"revId": "bf44c8c782b83deb8c1dba7e6bc91f628313f76f",
|
||||||
"serverId": "035e6965-6537-41bd-912c-053f3cf69326"
|
"serverId": "035e6965-6537-41bd-912c-053f3cf69326"
|
||||||
},
|
},
|
||||||
|
{
|
||||||
|
"unresolved": true,
|
||||||
|
"key": {
|
||||||
|
"uuid": "3a7150e2_dcc0c572",
|
||||||
|
"filename": "src/libosmo-mgcp/mgcp_network.c",
|
||||||
|
"patchSetId": 2
|
||||||
|
},
|
||||||
|
"lineNbr": 715,
|
||||||
|
"author": {
|
||||||
|
"id": 1000230
|
||||||
|
},
|
||||||
|
"writtenOn": "2025-10-22T04:00:46Z",
|
||||||
|
"side": 1,
|
||||||
|
"message": "Update 7 months later: I\u0027ll spin an updated patch with these arguments swapped.",
|
||||||
|
"parentUuid": "d7bff7ec_09cde6bb",
|
||||||
|
"revId": "bf44c8c782b83deb8c1dba7e6bc91f628313f76f",
|
||||||
|
"serverId": "035e6965-6537-41bd-912c-053f3cf69326"
|
||||||
|
},
|
||||||
{
|
{
|
||||||
"unresolved": true,
|
"unresolved": true,
|
||||||
"key": {
|
"key": {
|
||||||
@@ -358,6 +430,30 @@
|
|||||||
"revId": "bf44c8c782b83deb8c1dba7e6bc91f628313f76f",
|
"revId": "bf44c8c782b83deb8c1dba7e6bc91f628313f76f",
|
||||||
"serverId": "035e6965-6537-41bd-912c-053f3cf69326"
|
"serverId": "035e6965-6537-41bd-912c-053f3cf69326"
|
||||||
},
|
},
|
||||||
|
{
|
||||||
|
"unresolved": true,
|
||||||
|
"key": {
|
||||||
|
"uuid": "751287c5_c1ae1a13",
|
||||||
|
"filename": "src/libosmo-mgcp/mgcp_network.c",
|
||||||
|
"patchSetId": 2
|
||||||
|
},
|
||||||
|
"lineNbr": 1577,
|
||||||
|
"author": {
|
||||||
|
"id": 1000230
|
||||||
|
},
|
||||||
|
"writtenOn": "2025-10-22T04:00:46Z",
|
||||||
|
"side": 1,
|
||||||
|
"message": "What language do you then propose for the \"it is invoked when...\" part?",
|
||||||
|
"parentUuid": "06d1d3bf_051fdcbe",
|
||||||
|
"range": {
|
||||||
|
"startLine": 1577,
|
||||||
|
"startChar": 3,
|
||||||
|
"endLine": 1577,
|
||||||
|
"endChar": 5
|
||||||
|
},
|
||||||
|
"revId": "bf44c8c782b83deb8c1dba7e6bc91f628313f76f",
|
||||||
|
"serverId": "035e6965-6537-41bd-912c-053f3cf69326"
|
||||||
|
},
|
||||||
{
|
{
|
||||||
"unresolved": true,
|
"unresolved": true,
|
||||||
"key": {
|
"key": {
|
||||||
@@ -381,6 +477,30 @@
|
|||||||
"revId": "bf44c8c782b83deb8c1dba7e6bc91f628313f76f",
|
"revId": "bf44c8c782b83deb8c1dba7e6bc91f628313f76f",
|
||||||
"serverId": "035e6965-6537-41bd-912c-053f3cf69326"
|
"serverId": "035e6965-6537-41bd-912c-053f3cf69326"
|
||||||
},
|
},
|
||||||
|
{
|
||||||
|
"unresolved": true,
|
||||||
|
"key": {
|
||||||
|
"uuid": "255bde41_ba255622",
|
||||||
|
"filename": "src/libosmo-mgcp/mgcp_network.c",
|
||||||
|
"patchSetId": 2
|
||||||
|
},
|
||||||
|
"lineNbr": 1658,
|
||||||
|
"author": {
|
||||||
|
"id": 1000230
|
||||||
|
},
|
||||||
|
"writtenOn": "2025-10-22T04:00:46Z",
|
||||||
|
"side": 1,
|
||||||
|
"message": "Removing \"between\" would make the imperative sentence ungrammatical.",
|
||||||
|
"parentUuid": "6386809e_87758d28",
|
||||||
|
"range": {
|
||||||
|
"startLine": 1658,
|
||||||
|
"startChar": 4,
|
||||||
|
"endLine": 1658,
|
||||||
|
"endChar": 14
|
||||||
|
},
|
||||||
|
"revId": "bf44c8c782b83deb8c1dba7e6bc91f628313f76f",
|
||||||
|
"serverId": "035e6965-6537-41bd-912c-053f3cf69326"
|
||||||
|
},
|
||||||
{
|
{
|
||||||
"unresolved": true,
|
"unresolved": true,
|
||||||
"key": {
|
"key": {
|
||||||
|
Reference in New Issue
Block a user