[asterisk-dev] [Code Review]: Send udptl packets directly between both call legs without extrac them

[Joshua Colp]

> On Feb. 8, 2012, 12:06 p.m., Mark Michelson wrote:
> > This approach is not going to work for a few reasons:
> > 
> > 1. This change assumes that both channels are SIP channels. Besides the fact that the assumption is made in a dangerous way, H.323 and Skinny also support UDPTL and would not see the benefits of this change.
> > 2. This change places bridging logic into a read callback. Having the reading channel peek across the bridge to get the other channel's private data should be avoided if at all possible.
> > 3. This change does not do any sort of compatibility checks between the bridged channels.
> > 
> > I did some digging in the Asterisk trunk code and found that there is an ast_udptl_bridge() function already defined in udptl.c. It appears to do what the goal of this patch is, plus it adds necessary checks for compatibility and other possible reasons why the bridge might fail. The problem is that there is no code that actually uses ast_udptl_bridge() in Asterisk trunk. I think the proper approach to this change is to add appropriate calls to ast_udptl_bridge() where they should go. I think the way to go is to change the .bridge callbacks in UDPTL-capable channel drivers to point to a bridge function which can then call into ast_rtp_instance_bridge() or into ast_udptl_bridge() depending on the type of media being bridged.
> 
> Joshua Colp wrote:
>     Yes, things have to be *exactly* right to be able to do this. For example if the negotiated maximum IFP size differs (and in a lot of cases it does) you can't send the packet directly to the other side, you have to go through the UDPTL stack so it can resize things. After going through doing the T.38 support and seeing the real world implementations I personally just never explored direct bridging them due to this. There just aren't as many cases where they would be compatible as you would hope. Additionally the number of packets involved in comparison to RTP is small, so while you would get a performance gain with a large number at smaller numbers you just wouldn't get that much.
>     
>     I don't want to deter you from doing this of course so the approach Mark has outlined above is the correct way. You will need to add compatibility checks for error correction, max ifp, and max datagram.
> 
> vrban wrote:
>     >1. This change assumes that both channels are SIP channels. Besides the fact that the assumption is made in a dangerous way, H.323 and Skinny also support UDPTL and would not see the benefits of this change.
>     
>     Yes, it is a SIP only feature. If one channels is not SIP, then the UDPTL pakets get processed in the current fashion.
>     
>     >2. This change places bridging logic into a read callback. Having the reading channel peek across the bridge to get the other channel's private data should be avoided if at all possible.
>     
>     Counterproposal? The idea was to forward the udptl packages, as soon as the arrive to have the smallest possible delay which is feasible.
>     
>     >3. This change does not do any sort of compatibility checks between the bridged channels.
>     
>     The idea is not to do _any_ compatibility checks. The idea is to do "real" pass through. So i will rewrite it so, that all t.38 parameter from the sdp from both channels will be passed. e.g. The T.38 re-INVITE doing channel send us T38FaxMaxDatagram:1400 and T38FaxUdpEC:t38UDPRedundancy, we will fordward to the second channel T38FaxMaxDatagram:1400 and T38FaxUdpEC:t38UDPRedundancy. Same for the 200 OK coming back then.

You can't just pass them through because some endpoints are broken and their max datagram has to be overridden in the sip.conf configuration entry. You would want to use that value. I still don't know how well this will work in the real world, our UDPTL stack really seems to help with compatibility.


- Joshua


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviewboard.asterisk.org/r/1716/#review5434
-----------------------------------------------------------


On Feb. 8, 2012, 9:43 a.m., vrban wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/1716/
> -----------------------------------------------------------
> 
> (Updated Feb. 8, 2012, 9:43 a.m.)
> 
> 
> Review request for Asterisk Developers.
> 
> 
> Summary
> -------
> 
> New feature to send udptl packets directly between both call legs without extrac them. Like p2p for RTP.
> 
> 
> Diffs
> -----
> 
>   trunk/channels/chan_sip.c 354394 
>   trunk/channels/sip/include/sip.h 354394 
>   trunk/include/asterisk/udptl.h 354394 
>   trunk/main/udptl.c 354394 
> 
> Diff: https://reviewboard.asterisk.org/r/1716/diff
> 
> 
> Testing
> -------
> 
> Send a T.38 fax across two SIP channels. And the incoming udptl packets from first call leg where forwarded to the T.38 target at the second call leg and fax was received.
> 
> SIP Carrier -> T.38 -> asterisk with this new feature -> T.38 -> asterisk 1.8 with res_fax
> 
> 
> Thanks,
> 
> vrban
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.digium.com/pipermail/asterisk-dev/attachments/20120216/2bfdcfc4/attachment.htm>