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 &lt;nhofmeyr(a)sysmocom.de&gt; Gerrit-MessageType: newchange