[asterisk-dev] [Code Review] reworked chan_ooh323

Jeff Peeler jpeeler at digium.com
Wed Nov 4 12:45:15 CST 2009 


> On 2009-11-02 15:30:31, Jeff Peeler wrote:
> > /trunk/addons/chan_ooh323.c, line 1715
> > <https://reviewboard.asterisk.org/r/324/diff/2/?file=7214#file7214line1715>
> >
> >     why was this change made?
> 
>  wrote:
>     p->exten must be null terminated string.

ast_copy_string ensures the destination is null terminated though


> On 2009-11-02 15:30:31, Jeff Peeler wrote:
> > /trunk/addons/chan_ooh323.c, line 968
> > <https://reviewboard.asterisk.org/r/324/diff/2/?file=7214#file7214line968>
> >
> >     can't wait for the monitor call thread in this fashion - waiting on a pthread condition may be required
> 
>  wrote:
>     it's simplest method and it work. if call thread can't start in 500 ms above, call will be terminated.
>     I think it is not a better, but enhancements will be not easy.
>     
>     for using cond variable we must create additional sync points because if cond var will be in OOH323CallData
>     structure we can't use it before call thread is started, call data stucture may be not created even when 
>     ooh323_call is called (heavy load on main stack thread), also if we will place cond var in ooh323_pvt 
>     structure we can have deleted pvt structure already when call thread start (call is dropped already from 
>     asterisk side)
>

If this code must stay then at least document where the magic numbers came from until a better solution can be found.


> On 2009-11-02 15:30:31, Jeff Peeler wrote:
> > /trunk/addons/chan_ooh323.c, line 1345
> > <https://reviewboard.asterisk.org/r/324/diff/2/?file=7214#file7214line1345>
> >
> >     %abled
> 
>  wrote:
>     %s - is formatting symbol ;)

I don't know what I was thinking...


- Jeff


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


On 2009-11-03 18:27:02, may213 wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/324/
> -----------------------------------------------------------
> 
> (Updated 2009-11-03 18:27:02)
> 
> 
> Review request for Asterisk Developers.
> 
> 
> Summary
> -------
> 
> There is reworked chan_ooh323. 
> 
> Architecture changes are:
> multi-thread instead of single-thread original version. One thread is for commands from asterisk call thread to h323 stack, one thread is for accepting endpoint connection and talking with gatekeeper, one thread per call is for call processing.
> separate memory context from one endpoint context to many contexts of one per call. mutexes for locking critical data are introduced.
> 
> Protocol changes are:
> fast start response is generated based on our capability instead of return copy of remote FS OLC
> fast start response is generated one time per call
> capability adaptation to remote side framing
> improved gatekeeper support
> non-standart codec (G726 and G726r32 from Cisco, AMB NB)  support
> t.38 and mode changes (request mode/request mode ack) support
> 
> T.38 support, which implemented in two modes. Transparent mode is for non-t38 aware channels where translation from t38 packet to sound made by chan_ooh323 himself and faxgw mode which is t.38 pass-through and compatible with xFax applications and chan_sip.
> 
> and many small and cosmetic changes.
> 
> 
> Diffs
> -----
> 
>   /trunk/addons/Makefile 227508 
>   /trunk/addons/chan_ooh323.h 227508 
>   /trunk/addons/chan_ooh323.c 227508 
>   /trunk/addons/ooh323c/src/context.c 227508 
>   /trunk/addons/ooh323c/src/decode.c 227508 
>   /trunk/addons/ooh323c/src/dlist.c 227508 
>   /trunk/addons/ooh323c/src/encode.c 227508 
>   /trunk/addons/ooh323c/src/errmgmt.c 227508 
>   /trunk/addons/ooh323c/src/eventHandler.h 227508 
>   /trunk/addons/ooh323c/src/eventHandler.c 227508 
>   /trunk/addons/ooh323c/src/h323/H235-SECURITY-MESSAGESDec.c 227508 
>   /trunk/addons/ooh323c/src/h323/H323-MESSAGESDec.c 227508 
>   /trunk/addons/ooh323c/src/h323/MULTIMEDIA-SYSTEM-CONTROLDec.c 227508 
>   /trunk/addons/ooh323c/src/memheap.h 227508 
>   /trunk/addons/ooh323c/src/memheap.c 227508 
>   /trunk/addons/ooh323c/src/ooCalls.h 227508 
>   /trunk/addons/ooh323c/src/ooCalls.c 227508 
>   /trunk/addons/ooh323c/src/ooCapability.h 227508 
>   /trunk/addons/ooh323c/src/ooCapability.c 227508 
>   /trunk/addons/ooh323c/src/ooCmdChannel.h 227508 
>   /trunk/addons/ooh323c/src/ooCmdChannel.c 227508 
>   /trunk/addons/ooh323c/src/ooCommon.h 227508 
>   /trunk/addons/ooh323c/src/ooDateTime.h 227508 
>   /trunk/addons/ooh323c/src/ooDateTime.c 227508 
>   /trunk/addons/ooh323c/src/ooGkClient.h 227508 
>   /trunk/addons/ooh323c/src/ooGkClient.c 227508 
>   /trunk/addons/ooh323c/src/ooLogChan.h 227508 
>   /trunk/addons/ooh323c/src/ooLogChan.c 227508 
>   /trunk/addons/ooh323c/src/ooSocket.h 227508 
>   /trunk/addons/ooh323c/src/ooSocket.c 227508 
>   /trunk/addons/ooh323c/src/ooStackCmds.h 227508 
>   /trunk/addons/ooh323c/src/ooStackCmds.c 227508 
>   /trunk/addons/ooh323c/src/ooTimer.c 227508 
>   /trunk/addons/ooh323c/src/ooUtils.c 227508 
>   /trunk/addons/ooh323c/src/ooasn1.h 227508 
>   /trunk/addons/ooh323c/src/oochannels.h 227508 
>   /trunk/addons/ooh323c/src/oochannels.c 227508 
>   /trunk/addons/ooh323c/src/ooh245.h 227508 
>   /trunk/addons/ooh323c/src/ooh245.c 227508 
>   /trunk/addons/ooh323c/src/ooh323.h 227508 
>   /trunk/addons/ooh323c/src/ooh323.c 227508 
>   /trunk/addons/ooh323c/src/ooh323ep.h 227508 
>   /trunk/addons/ooh323c/src/ooh323ep.c 227508 
>   /trunk/addons/ooh323c/src/ooports.c 227508 
>   /trunk/addons/ooh323c/src/ooq931.h 227508 
>   /trunk/addons/ooh323c/src/ooq931.c 227508 
>   /trunk/addons/ooh323c/src/ootrace.h 227508 
>   /trunk/addons/ooh323c/src/ootrace.c 227508 
>   /trunk/addons/ooh323c/src/ootypes.h 227508 
>   /trunk/addons/ooh323c/src/perutil.c 227508 
>   /trunk/addons/ooh323c/src/printHandler.h 227508 
>   /trunk/addons/ooh323c/src/printHandler.c 227508 
>   /trunk/addons/ooh323c/src/rtctype.c 227508 
>   /trunk/addons/ooh323cDriver.h 227508 
>   /trunk/addons/ooh323cDriver.c 227508 
> 
> Diff: https://reviewboard.asterisk.org/r/324/diff
> 
> 
> Testing
> -------
> 
> Tested on heavily-loaded h323 transit server with call rate up to 8 per sec and up to 200k per day. asterisk restarted only for changing module. No memory leaks.
> 
> T.38 tested in both mode. It work in transparent mode for sending and receiving faxes, work in faxgw mode with Send/ReceiveFax applications. Faxing with chan_sip is not tested because i don't have sip terminal with t.38 support.
> 
> 
> Thanks,
> 
> may213
> 
>

More information about the asterisk-dev mailing list