[asterisk-dev] [Code Review] Modularized RTP stack support

[Joshua Colp]

> On 2009-04-01 09:38:48, Russell Bryant wrote:
> > /trunk/include/asterisk/rtp_engine.h, lines 366-368
> > <http://reviewboard.digium.com/r/209/diff/4/?file=4013#file4013line366>
> >
> >     This is declared twice in this file.

Fixed.


> On 2009-04-01 09:38:48, Russell Bryant wrote:
> > /trunk/include/asterisk/rtp_engine.h, lines 864-887
> > <http://reviewboard.digium.com/r/209/diff/4/?file=4013#file4013line864>
> >
> >     Fix capitalization of arguments here

Fixed.


> On 2009-04-01 09:38:48, Russell Bryant wrote:
> > /trunk/include/asterisk/rtp_engine.h, lines 909-912
> > <http://reviewboard.digium.com/r/209/diff/4/?file=4013#file4013line909>
> >
> >     capitalization

Fixed.


> On 2009-04-01 09:38:48, Russell Bryant wrote:
> > /trunk/res/res_rtp_asterisk.c, lines 2511-2514
> > <http://reviewboard.digium.com/r/209/diff/4/?file=4021#file4021line2511>
> >
> >     nochecksums = ast_false(s) ? 1 : 0;

Done.


> On 2009-04-01 09:38:48, Russell Bryant wrote:
> > /trunk/res/res_rtp_asterisk.c, lines 2488-2491
> > <http://reviewboard.digium.com/r/209/diff/4/?file=4021#file4021line2488>
> >
> >     These min/max values could go into a global constant as well

Done.


> On 2009-04-01 09:38:48, Russell Bryant wrote:
> > /trunk/res/res_rtp_asterisk.c, lines 2481-2482
> > <http://reviewboard.digium.com/r/209/diff/4/?file=4021#file4021line2481>
> >
> >     Can you put these default values in a global constant?  They are referenced in a couple of places.

Done.


> On 2009-04-01 09:38:48, Russell Bryant wrote:
> > /trunk/include/asterisk/stun.h, lines 67-68
> > <http://reviewboard.digium.com/r/209/diff/4/?file=4014#file4014line67>
> >
> >     This should probably go in include/asterisk/_private.h with the rest of the _init() functions

Done.


- Joshua


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://reviewboard.digium.com/r/209/#review666
-----------------------------------------------------------


On 2009-04-01 09:21:47, Joshua Colp wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://reviewboard.digium.com/r/209/
> -----------------------------------------------------------
> 
> (Updated 2009-04-01 09:21:47)
> 
> 
> Review request for Asterisk Developers.
> 
> 
> Summary
> -------
> 
> This patch provides a common API (known as the RTP engine API) so that RTP stacks can be easily plugged into Asterisk. Functionality wise this patch should be equal to the current capabilities of our in-core RTP stack. The API is documented in the rtp_engine.h header file and the in-core RTP stack has been broken out into a module called res_rtp_asterisk.
> 
> 
> Diffs
> -----
> 
>   /trunk/UPGRADE.txt 185780 
>   /trunk/apps/app_dial.c 185780 
>   /trunk/channels/chan_agent.c 185780 
>   /trunk/channels/chan_bridge.c 185780 
>   /trunk/channels/chan_gtalk.c 185780 
>   /trunk/channels/chan_h323.c 185780 
>   /trunk/channels/chan_jingle.c 185780 
>   /trunk/channels/chan_local.c 185780 
>   /trunk/channels/chan_mgcp.c 185780 
>   /trunk/channels/chan_sip.c 185780 
>   /trunk/channels/chan_skinny.c 185780 
>   /trunk/channels/chan_unistim.c 185780 
>   /trunk/configs/sip.conf.sample 185780 
>   /trunk/include/asterisk/rtp.h 185780 
>   /trunk/include/asterisk/rtp_engine.h PRE-CREATION 
>   /trunk/include/asterisk/stun.h PRE-CREATION 
>   /trunk/main/Makefile 185780 
>   /trunk/main/asterisk.c 185780 
>   /trunk/main/loader.c 185780 
>   /trunk/main/rtp.c 185780 
>   /trunk/main/rtp_engine.c PRE-CREATION 
>   /trunk/main/stun.c PRE-CREATION 
>   /trunk/res/res_rtp_asterisk.c PRE-CREATION 
> 
> Diff: http://reviewboard.digium.com/r/209/diff
> 
> 
> Testing
> -------
> 
> I've tested using the most complex channel driver that uses the RTP stack, chan_sip. I've confirmed calls of various scenarios work but would like further testing with additional channel drivers that I am unable to test.
> 
> 
> Thanks,
> 
> Joshua
> 
>