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

falconia has posted comments on this change by falconia. ( https://gerrit.osmocom.org/c/osmo-mgw/+/39869?usp=email )

Change subject: mgw: rtp-patch rfc5993hr: convert to each end's respective format ......................................................................

Patch Set 2:

(6 comments)

Patchset:

PS2:

and also the BSC has to then be aware of the HR formats in use.

This part would be the biggest difficulty in practice. How would the BSC know which BTS is configured for TS 101 318 vs which is configured for RFC 5993, when it is a vty option in OsmoBTS?

File src/libosmo-mgcp/mgcp_network.c:

https://gerrit.osmocom.org/c/osmo-mgw/+/39869/comment/43c79097_a22c5d4b?usp=... : PS2, Line 691: * consider the full RTP chain of a single call:

...

(my idea so far, can be in a future patch, is to not send any HR payload to a peer until we have r […]

The comment by @laforge@osmocom.org is correct: I also initially thought about sending nothing to all to each peer until that peer sent its own first packet, but then I quickly realized that the result would be no traffic passing at all, ever. Referring to the ASCII art diagram in the detailed comment in my patch, if the policy proposed by @nhofmeyr@sysmocom.de were implemented, each of BSC-MGW-A and BSC-MGW-B would receive packets from BTS-A and BTS-B, but because neither BSC-associated MGW will be the first to send a packet to the CN, neither MSC-MGW-A nor MSC-MGW-B will ever see even one packet.

https://gerrit.osmocom.org/c/osmo-mgw/+/39869/comment/15786ea3_af80d169?usp=... : PS2, Line 698: will always be RFC 5993

On AoIP, there is no discussion. It is always RFC5993 as that is what (likely, quoting from memory) 3GPP TS 48.103 mandates.

Correct: TS 48.103 is the spec, and it unambiguously specifies RFC 5993 format for HR codec.

https://gerrit.osmocom.org/c/osmo-mgw/+/39869/comment/d7bff7ec_09cde6bb?usp=... : PS2, Line 715: static int rfc5993_hr_convert(struct msgb *msg, struct mgcp_conn_rtp *dst)

(kind of weird to swap the msgb argument position for no reason)

It is an artifact of how I developed this code in stages. I can flip this argument order in the next patch iteration, no big deal.

https://gerrit.osmocom.org/c/osmo-mgw/+/39869/comment/bc231742_388288f0?usp=... : PS2, Line 1660: strcmp(conn_src->end.cset.codec->subtype_name, "GSM-HR-08") == 0 && conn_src->end.rfc5993_hr_convert) {

I'd say better split this line into 2.

I originally had it split, but the overzealous linter forced me to join into this longer line! See the diff between patchset 1 and patchset 2, where patchset 1 was rejected (V-1) by the linter.

https://gerrit.osmocom.org/c/osmo-mgw/+/39869/comment/72bff7db_64557059?usp=... : PS2, Line 1661: int rc = examine_rtp_rx_gsmhr(msg, conn_src);

Is this called always on GSM-HR-08 or only once, until we successfully detected the HR format? ... […]

It is called for every received packet. If we were to perform format detection only on the very first packet, what would happen if a BSC-level handover occurs between two BTS that use (or are configured to use) different HR formats?