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

[Joshua Colp]

> On May 3, 2012, 4:42 p.m., Mark Michelson wrote:
> > /trunk/channels/chan_sip.c, line 9837
> > <https://reviewboard.asterisk.org/r/1891/diff/1/?file=27601#file27601line9837>
> >
> >     address and relay_address should be larger since an IPv6 address may require more than 23 characters.

Fixed.


> On May 3, 2012, 4:42 p.m., Mark Michelson wrote:
> > /trunk/res/res_rtp_asterisk.c, lines 43-44
> > <https://reviewboard.asterisk.org/r/1891/diff/1/?file=27606#file27606line43>
> >
> >     A comment explaining this would be fantastic.

Done.


> On May 3, 2012, 4:42 p.m., Mark Michelson wrote:
> > /trunk/res/res_rtp_asterisk.c, line 663
> > <https://reviewboard.asterisk.org/r/1891/diff/1/?file=27606#file27606line663>
> >
> >     red blob

Fixed.


> On May 3, 2012, 4:42 p.m., Mark Michelson wrote:
> > /trunk/res/res_rtp_asterisk.c, lines 805-809
> > <https://reviewboard.asterisk.org/r/1891/diff/1/?file=27606#file27606line805>
> >
> >     Another case of using pj_str() before pj_thread_register_check()

See comment above.


> On May 3, 2012, 4:42 p.m., Mark Michelson wrote:
> > /trunk/res/res_rtp_asterisk.c, lines 422-433
> > <https://reviewboard.asterisk.org/r/1891/diff/1/?file=27606#file27606line422>
> >
> >     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?

No, actually. The usage of pj_str as seen simply sets the pointer and length within the pj_str_t structure itself. It does not allocate memory or do anything with pjlib. If I made a copy of the string using a memory pool I would not be able to until after the call to pj_thread_register_check.


- Joshua


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


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/20120509/074c2730/attachment-0001.htm>