[asterisk-dev] [Code Review] ICE, STUN, and TURN Support

[Mark Michelson]

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


Made a pass through and found a few things. I'll look deeper on my next one.


/trunk/channels/chan_sip.c
<https://reviewboard.asterisk.org/r/1891/#comment11256>

    address and relay_address should be larger since an IPv6 address may require more than 23 characters.



/trunk/res/res_rtp_asterisk.c
<https://reviewboard.asterisk.org/r/1891/#comment11261>

    A comment explaining this would be fantastic.



/trunk/res/res_rtp_asterisk.c
<https://reviewboard.asterisk.org/r/1891/#comment11263>

    I'm a bit confused about pj_thread_register_check(). The idea is that you have to register your thread with pjlib before calling any of pjlib's functions, or else the library will assert and you'll feel very silly.
    
    The problem here is that you've used pj_str() twice before calling pj_thread_register_check(). Is that potentially problematic?



/trunk/res/res_rtp_asterisk.c
<https://reviewboard.asterisk.org/r/1891/#comment11265>

    red blob



/trunk/res/res_rtp_asterisk.c
<https://reviewboard.asterisk.org/r/1891/#comment11266>

    Another case of using pj_str() before pj_thread_register_check()


- Mark


On April 30, 2012, 1:50 p.m., Joshua Colp wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/1891/
> -----------------------------------------------------------
> 
> (Updated April 30, 2012, 1:50 p.m.)
> 
> 
> Review request for Asterisk Developers.
> 
> 
> Summary
> -------
> 
> This patch adds support for the following:
> 
> ICE attribute parsing and generation in chan_sip
> Usage of the ICE interface in chan_sip
> ICE support within res_rtp_asterisk
> STUN support within res_rtp_asterisk for getting the server reflexive address
> TURN support within res_rtp_asterisk for relaying traffic when needed
> Additional configuration options for the above
> 
> One area which could use some feedback is the pjproject integration. Should we try to use their build system as I have done to integrate things, or try to roll their stuff into ours further?
> 
> If you would like some background on ICE and the actual support here take a gander at the wiki page located at https://wiki.asterisk.org/wiki/display/~jcolp/ICE%2C+STUN%2C+and+TURN+Support - hopefully it proves useful to you.
> 
> 
> Diffs
> -----
> 
>   /trunk/channels/chan_sip.c 361952 
>   /trunk/configs/rtp.conf.sample 361952 
>   /trunk/include/asterisk/rtp_engine.h 361952 
>   /trunk/main/rtp_engine.c 361952 
>   /trunk/res/Makefile 361952 
>   /trunk/res/res_rtp_asterisk.c 361952 
> 
> Diff: https://reviewboard.asterisk.org/r/1891/diff
> 
> 
> Testing
> -------
> 
> Tested various scenarios using Bria and X-Lite behind multiple NATs, different NATs, different internet connections but locally reachable, etc. It works well.
> 
> 
> Thanks,
> 
> Joshua
> 
>

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