Cellular Network Infrastructure » OsmoMGW

This patch [1] in practice reverted a previous patch [2] which was submitted a for a reason.

The reason is that we want to match the exact AMR Payload Type in first place, in order to avoid AMR transcoding OA<->BE if not needed.

This can happen for instance if connection A in the endpoint announces through SDP that it supports only let's say:

• PT=100 AMR octet_aligned=1

And connection B, the other connection in the same endpoint, announces it supports both in SDP, let's say:

• PT=102 AMR octet_aligned=0
• PT=103 AMR octet_aligned=1

After [1] is merged, the code, when forwarding RTP connA->connB, will select the PT of first AMR codec found in conn regardless of the octet_aligned, that is PT=102, which means it will end up later on having to transocde OA->BE. The ideal thing to do in this situation would be to pick PT=103, so that transcoding can be avoided when sending packets on that direction, hence freeing CPU use.

In order to do so, it's quite easy, we just need to modify a bit patch [1] to incorporate the idea of [2]. So first we do a lookup as in [2], checking exact match, and if not found, do as in [1] (no AMR octet_aligned match).
This can be generalized as "first try a match not requiring transcoding, and if not found, try a match which requires transcoding".

It can be implemented with something like this:

diff --git a/src/libosmo-mgcp/mgcp_codec.c b/src/libosmo-mgcp/mgcp_codec.c
index 38aa0a79f..f6dfbd66b 100644
--- a/src/libosmo-mgcp/mgcp_codec.c
+++ b/src/libosmo-mgcp/mgcp_codec.c
@@ -366,7 +366,7 @@ bool mgcp_codec_amr_is_octet_aligned(const struct mgcp_rtp_codec *codec)

 /* Compare two codecs, all parameters must match up, except for the payload type
  * number. */
-static bool codecs_same(struct mgcp_rtp_codec *codec_a, struct mgcp_rtp_codec *codec_b)
+static bool codecs_same(struct mgcp_rtp_codec *codec_a, struct mgcp_rtp_codec *codec_b, bool allow_transcoding)
 {
        if (codec_a->rate != codec_b->rate)
                return false;
@@ -378,10 +378,10 @@ static bool codecs_same(struct mgcp_rtp_codec *codec_a, struct mgcp_rtp_codec *c
                return false;
        if (strcmp(codec_a->subtype_name, codec_b->subtype_name))
                return false;
-
-       /* Note: AMR allows to set the RTP payload format to octet-aligned or bandwith-efficient (octet-aligned=0)
-        * via SDP. This difference concerns payload format only, but not the actual codec. It is not a difference
-        * within the meaning of this function. */
+       if (!strcmp(codec_a->subtype_name, "AMR")) {
+               if (mgcp_codec_amr_is_octet_aligned(codec_a) != mgcp_codec_amr_is_octet_aligned(codec_b))
+                       return allow_transcoding; /* we support transcoding OA<->BE */
+       }

        return true;
 }
@@ -421,16 +421,19 @@ int mgcp_codec_pt_translate(struct mgcp_conn_rtp *conn_src, struct mgcp_conn_rtp
         * equivalent of it on the destination side */
        codecs_assigned = rtp_dst->codecs_assigned;
        OSMO_ASSERT(codecs_assigned <= MGCP_MAX_CODECS);
+       /* First attempt exact match, without transcoding: */
        for (i = 0; i < codecs_assigned; i++) {
-               if (codecs_same(codec_src, &rtp_dst->codecs[i])) {
-                       codec_dst = &rtp_dst->codecs[i];
-                       break;
-               }
+               if (codecs_same(codec_src, &rtp_dst->codecs[i], false);
+                       return rtp_dst->codecs[i]->payload_type;
+       }
+       /* No exact match, try again with transcoding enabled: */
+       for (i = 0; i < codecs_assigned; i++) {
+               if (codecs_same(codec_src, &rtp_dst->codecs[i], true);
+                       return rtp_dst->codecs[i]->payload_type;
        }
-       if (!codec_dst)
-               return -EINVAL;

-       return codec_dst->payload_type;
+       /* No matching codec found: */
+               return -EINVAL;
 }

We should add TTCN3/unit tests validating scenarios like the one described above, and make sure we do the intelligent thing.

[1] https://gerrit.osmocom.org/c/osmo-mgw/+/27223
[2] https://gerrit.osmocom.org/c/osmo-mgw/+/15136