neels has uploaded this change for review. ( 
https://gerrit.osmocom.org/c/osmo-msc/+/34412?usp=email )


Change subject: fix codecs in internal call bridge
......................................................................

fix codecs in internal call bridge

This is a fixup for the patch
'3G: decapsulate IuUP to AMR at the MGW; allow 3G<-AMR->2G'
I386a6a426c318040b019ab5541689c67e94672a1

After this patch, osmo-msc intelligently decides which codecs to run on
which legs of the RTP streams. In the meantime, the necessary matching
changes to call_leg_local_bridge() have been lost in patch reviews.

Testing 3G to 3G voice now, I noticed that call_leg_local_bridge()
overwrites the intelligent choices made earlier.

The history of an MGW endpoint that should convert from IUFP to plain
AMR, extracted from a pcap, looks like this:

    - CRCX None None
    - CRCX-OK audio 4050 RTP/AVP 112 None
    - MDCX audio 4056 RTP/AVP 112 AMR
    - MDCX-OK audio 4050 RTP/AVP 112 AMR
    - MDCX audio 4056 RTP/AVP 96 VND.3GPP.IUFP
    - MDCX-OK audio 4050 RTP/AVP 96 VND.3GPP.IUFP

So after call_leg_local_bridge(), there is an extra MDCX + MDCX-OK that
switches the codec from 112 AMR back to 96 IUFP.

That is because call_leg_local_bridge() copies the *RAN* side's codec to
both CN sides, which used to be ok when RAN and CN codecs were always
identical.

Instead, adjust only the CN sides of the MGW endpoints, and adjust them
so that both CN sides are identical. osmo-mgw should then be able to
trivially translate the codecs appropriately.

Change-Id: I130bcd77ec57e332370c487a11b0b973b6e1089d
---
M src/libmsc/call_leg.c
1 file changed, 73 insertions(+), 11 deletions(-)



  git pull ssh://gerrit.osmocom.org:29418/osmo-msc refs/changes/12/34412/1

diff --git a/src/libmsc/call_leg.c b/src/libmsc/call_leg.c
index d0dc642..9c3b8a0 100644
--- a/src/libmsc/call_leg.c
+++ b/src/libmsc/call_leg.c
@@ -352,25 +352,46 @@
 int call_leg_local_bridge(struct call_leg *cl1, uint32_t call_id1, struct 
gsm_trans *trans1,
                          struct call_leg *cl2, uint32_t call_id2, struct 
gsm_trans *trans2)
 {
-       struct sdp_audio_codecs *codecs;
+       struct sdp_audio_codecs *cn_codecs = NULL;

        cl1->local_bridge = cl2;
        cl2->local_bridge = cl1;

-       /* We may just copy the codec info we have for the RAN side of the 
first leg to the CN side of both legs. This
-        * also means that if both legs use different codecs the MGW must 
perform transcoding on the second leg. */
-       if (!cl1->rtp[RTP_TO_RAN] || !cl1->rtp[RTP_TO_RAN]->codecs_known) {
-               LOG_CALL_LEG(cl1, LOGL_ERROR, "RAN-side RTP stream codec is not 
known, not ready for bridging\n");
+       /* Marry the two CN sides of the call legs. Call establishment should 
have made all efforts for these to be
+        * compatible. However, for local bridging, the codecs and payload type 
numbers must be exactly identical on
+        * both sides. Both sides may so far have different payload type 
numbers or slightly differing codecs, but it
+        * will only work when the SDP on the RTP_TO_CN sides of the call legs 
talk the same payload type numbers.
+        * So, simply take the SDP from one RTP_TO_CN side, and overwrite the 
other RTP_TO_CN side's SDP with it.
+        * If all goes to plan, the codecs will be identical, or possibly the 
MGW will do a conversion like AMR-BE to
+        * AMR-OA. In the worst case, the other call leg cannot transcode, and 
the call fails -- because codec
+        * negotiation did not do a good enough job.
+        *
+        * When codecs match:
+        *
+        *     call leg 1         call leg 2
+        *     ---MGW-ep-------   ---MGW-ep-------
+        *     RAN      CN        CN       RAN
+        *     AMR:112  AMR:112   AMR:96   AMR:96
+        *                 |
+        *                 +-------+
+        *                         |
+        *                         V
+        *     AMR:112  AMR:112   AMR:112  AMR:96
+        *                               ^MGW-endpoint converts payload type 
numbers between 112 and 96.
+        */
+       if (cl1->rtp[RTP_TO_CN] && cl1->rtp[RTP_TO_CN]->codecs_known)
+               cn_codecs = &cl1->rtp[RTP_TO_CN]->codecs;
+       else if (cl2->rtp[RTP_TO_CN] && cl2->rtp[RTP_TO_CN]->codecs_known)
+               cn_codecs = &cl2->rtp[RTP_TO_CN]->codecs;
+       if (!cn_codecs) {
+               LOG_CALL_LEG(cl1, LOGL_ERROR, "RAN-side CN stream codec is not 
known, not ready for bridging\n");
+               LOG_CALL_LEG(cl2, LOGL_ERROR, "RAN-side CN stream codec is not 
known, not ready for bridging\n");
                return -EINVAL;
        }
-       codecs = &cl1->rtp[RTP_TO_RAN]->codecs;
-
-       if (!cl1->rtp[RTP_TO_CN] || !cl2->rtp[RTP_TO_CN])
-               return -ENOTCONN;

        call_leg_ensure_ci(cl1, RTP_TO_CN, call_id1, trans1,
-                          codecs, &cl2->rtp[RTP_TO_CN]->local);
+                          cn_codecs, &cl2->rtp[RTP_TO_CN]->local);
        call_leg_ensure_ci(cl2, RTP_TO_CN, call_id2, trans2,
-                          codecs, &cl1->rtp[RTP_TO_CN]->local);
+                          cn_codecs, &cl1->rtp[RTP_TO_CN]->local);
        return 0;
 }

--
To view, visit https://gerrit.osmocom.org/c/osmo-msc/+/34412?usp=email
To unsubscribe, or for help writing mail filters, visit 
https://gerrit.osmocom.org/settings

Gerrit-Project: osmo-msc
Gerrit-Branch: master
Gerrit-Change-Id: I130bcd77ec57e332370c487a11b0b973b6e1089d
Gerrit-Change-Number: 34412
Gerrit-PatchSet: 1
Gerrit-Owner: neels <nhofm...@sysmocom.de>
Gerrit-MessageType: newchange

Reply via email to