[asterisk-dev] [Code Review] chan_ooh323 direct rtp (remote bridging) support

Thu Jan 5 09:39:30 CST 2012

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


more comments


/trunk/addons/chan_ooh323.c
<https://reviewboard.asterisk.org/r/1607/#comment9441>

    spacing


/trunk/addons/chan_ooh323.c
<https://reviewboard.asterisk.org/r/1607/#comment9442>

    same


/trunk/addons/chan_ooh323.c
<https://reviewboard.asterisk.org/r/1607/#comment9443>

    same


/trunk/addons/chan_ooh323.c
<https://reviewboard.asterisk.org/r/1607/#comment9444>

    remove?


/trunk/addons/chan_ooh323.c
<https://reviewboard.asterisk.org/r/1607/#comment9504>

    { }


/trunk/addons/ooh323c/src/ooCmdChannel.c
<https://reviewboard.asterisk.org/r/1607/#comment9481>

    spacing


/trunk/addons/ooh323c/src/ooLogChan.c
<https://reviewboard.asterisk.org/r/1607/#comment9479>

    spacing


/trunk/addons/ooh323c/src/ooLogChan.c
<https://reviewboard.asterisk.org/r/1607/#comment9480>

    spacing


/trunk/addons/ooh323c/src/ooLogChan.c
<https://reviewboard.asterisk.org/r/1607/#comment9478>

    spacing


/trunk/addons/ooh323c/src/ooStackCmds.c
<https://reviewboard.asterisk.org/r/1607/#comment9477>

    if (...) {
    
    }


/trunk/addons/ooh323c/src/ooStackCmds.c
<https://reviewboard.asterisk.org/r/1607/#comment9476>

    spacing


/trunk/addons/ooh323c/src/ooStackCmds.c
<https://reviewboard.asterisk.org/r/1607/#comment9475>

    spacing


/trunk/addons/ooh323c/src/ooh245.c
<https://reviewboard.asterisk.org/r/1607/#comment9474>

    spacing


/trunk/addons/ooh323c/src/ooh245.c
<https://reviewboard.asterisk.org/r/1607/#comment9473>

    blobs


/trunk/addons/ooh323c/src/ooh245.c
<https://reviewboard.asterisk.org/r/1607/#comment9472>

    single line, spacing


/trunk/addons/ooh323c/src/ooh245.c
<https://reviewboard.asterisk.org/r/1607/#comment9471>

    spacing, single line


/trunk/addons/ooh323c/src/ooh245.c
<https://reviewboard.asterisk.org/r/1607/#comment9470>

    blob


/trunk/addons/ooh323c/src/ooh245.c
<https://reviewboard.asterisk.org/r/1607/#comment9469>

    blob


/trunk/addons/ooh323c/src/ooh245.c
<https://reviewboard.asterisk.org/r/1607/#comment9468>

    blob


/trunk/addons/ooh323c/src/ooh245.c
<https://reviewboard.asterisk.org/r/1607/#comment9465>

    remove?


/trunk/addons/ooh323c/src/ooh245.c
<https://reviewboard.asterisk.org/r/1607/#comment9467>

    blob


/trunk/addons/ooh323c/src/ooh245.c
<https://reviewboard.asterisk.org/r/1607/#comment9466>

    spacing, single line


/trunk/addons/ooh323c/src/ooh245.c
<https://reviewboard.asterisk.org/r/1607/#comment9464>

    blob


/trunk/addons/ooh323c/src/ooh245.c
<https://reviewboard.asterisk.org/r/1607/#comment9463>

    spacing, single line


/trunk/addons/ooh323c/src/ooh245.c
<https://reviewboard.asterisk.org/r/1607/#comment9462>

    spacing


/trunk/addons/ooh323c/src/ooh245.c
<https://reviewboard.asterisk.org/r/1607/#comment9461>

    spacing, single line


/trunk/addons/ooh323c/src/ooh245.c
<https://reviewboard.asterisk.org/r/1607/#comment9460>

    single line


/trunk/addons/ooh323c/src/ooh245.c
<https://reviewboard.asterisk.org/r/1607/#comment9458>

    if (...) {


/trunk/addons/ooh323c/src/ooh245.c
<https://reviewboard.asterisk.org/r/1607/#comment9459>

    spacing and move { up


/trunk/addons/ooh323c/src/ooh245.c
<https://reviewboard.asterisk.org/r/1607/#comment9457>

    remove?


/trunk/addons/ooh323c/src/ooh245.c
<https://reviewboard.asterisk.org/r/1607/#comment9456>

    remove?


/trunk/addons/ooh323c/src/ooh245.c
<https://reviewboard.asterisk.org/r/1607/#comment9454>

    blob


/trunk/addons/ooh323c/src/ooh245.c
<https://reviewboard.asterisk.org/r/1607/#comment9455>

    if (...) {
    
    }


/trunk/addons/ooh323c/src/ooh245.c
<https://reviewboard.asterisk.org/r/1607/#comment9453>

    blob


/trunk/addons/ooh323c/src/ooh245.c
<https://reviewboard.asterisk.org/r/1607/#comment9452>

    spacing, red blob


/trunk/addons/ooh323c/src/ooh245.c
<https://reviewboard.asterisk.org/r/1607/#comment9451>

    remove? and blob


/trunk/addons/ooh323c/src/ooh245.c
<https://reviewboard.asterisk.org/r/1607/#comment9450>

    blob


/trunk/addons/ooh323c/src/ooh245.c
<https://reviewboard.asterisk.org/r/1607/#comment9449>

    blob


/trunk/addons/ooh323c/src/ooh245.c
<https://reviewboard.asterisk.org/r/1607/#comment9448>

    spacing


/trunk/addons/ooh323c/src/ooh323.c
<https://reviewboard.asterisk.org/r/1607/#comment9447>

    red blobs since you are here


/trunk/addons/ooh323c/src/ooq931.c
<https://reviewboard.asterisk.org/r/1607/#comment9446>

    more coding guidelines formatting required


/trunk/addons/ooh323c/src/ooq931.c
<https://reviewboard.asterisk.org/r/1607/#comment9445>

    list block needs some formatting love.  Red blobs, spacing, if statements.


/trunk/addons/ooh323cDriver.c
<https://reviewboard.asterisk.org/r/1607/#comment9503>

    spacing, { }


/trunk/addons/ooh323cDriver.c
<https://reviewboard.asterisk.org/r/1607/#comment9502>

    spacing, { }


/trunk/addons/ooh323cDriver.c
<https://reviewboard.asterisk.org/r/1607/#comment9501>

    spacing, { }


/trunk/addons/ooh323cDriver.c
<https://reviewboard.asterisk.org/r/1607/#comment9500>

    spacing, { }


/trunk/addons/ooh323cDriver.c
<https://reviewboard.asterisk.org/r/1607/#comment9499>

    spacing, { }


/trunk/addons/ooh323cDriver.c
<https://reviewboard.asterisk.org/r/1607/#comment9498>

    spacing, { }


/trunk/addons/ooh323cDriver.c
<https://reviewboard.asterisk.org/r/1607/#comment9497>

    spacing, { }


/trunk/addons/ooh323cDriver.c
<https://reviewboard.asterisk.org/r/1607/#comment9496>

    spacing, { }


/trunk/addons/ooh323cDriver.c
<https://reviewboard.asterisk.org/r/1607/#comment9495>

    spacing, { }


/trunk/addons/ooh323cDriver.c
<https://reviewboard.asterisk.org/r/1607/#comment9494>

    spacing, { } and blob


/trunk/addons/ooh323cDriver.c
<https://reviewboard.asterisk.org/r/1607/#comment9493>

    spacing, { } and blob


/trunk/addons/ooh323cDriver.c
<https://reviewboard.asterisk.org/r/1607/#comment9492>

    spacing, { } and blob


/trunk/addons/ooh323cDriver.c
<https://reviewboard.asterisk.org/r/1607/#comment9491>

    spacing, { } and blob


/trunk/addons/ooh323cDriver.c
<https://reviewboard.asterisk.org/r/1607/#comment9490>

    spacing, { } and blob


/trunk/addons/ooh323cDriver.c
<https://reviewboard.asterisk.org/r/1607/#comment9489>

    spacing, { } and blob


/trunk/addons/ooh323cDriver.c
<https://reviewboard.asterisk.org/r/1607/#comment9488>

    spacing, { } and blob


/trunk/addons/ooh323cDriver.c
<https://reviewboard.asterisk.org/r/1607/#comment9487>

    spacing, { } and blob


/trunk/addons/ooh323cDriver.c
<https://reviewboard.asterisk.org/r/1607/#comment9486>

    spacing, { } and blob


/trunk/addons/ooh323cDriver.c
<https://reviewboard.asterisk.org/r/1607/#comment9485>

    spacing, { } and blob


/trunk/addons/ooh323cDriver.c
<https://reviewboard.asterisk.org/r/1607/#comment9484>

    spacing, { } and blob


/trunk/addons/ooh323cDriver.c
<https://reviewboard.asterisk.org/r/1607/#comment9483>

    spacing, { } and blob


/trunk/addons/ooh323cDriver.c
<https://reviewboard.asterisk.org/r/1607/#comment9482>

    spacing, { } and blob


- Paul


On Jan. 5, 2012, 9:11 a.m., may213 wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/1607/
> -----------------------------------------------------------
> 
> (Updated Jan. 5, 2012, 9:11 a.m.)
> 
> 
> Review request for Asterisk Developers.
> 
> 
> Summary
> -------
> 
> There is direct rtp support for chan_ooh323 channel driver.
> It work like chan_sip direct rtp support based on directmedia (directrtp) option that can be global or per peer/user.
> Also there is earlydirect (directrtpsetup) option that allow setup direct rtp connection for early media.
> 
> On h.323 side there can be two modes to enable direct rtp - with or without renegotiation media channels.
> If there are opened media channels (logical channels in h.323) then renegotiation must be done and it caused by
> sending empty terminalcapabilities set to opposite side.  If empty tcs is arrived transmit logical channels
> will be closed then reopen with new rtp addresses.
> if there are no opened channels then new rtp addresses come in the new fast start proposal or just saved 
> for further use in the channel open negotiation (it is for fast start disabled case).
> 
> 
> Diffs
> -----
> 
>   /trunk/addons/Makefile 349671 
>   /trunk/addons/chan_ooh323.h 349671 
>   /trunk/addons/chan_ooh323.c 349671 
>   /trunk/addons/ooh323c/src/ooCalls.h 349671 
>   /trunk/addons/ooh323c/src/ooCmdChannel.c 349671 
>   /trunk/addons/ooh323c/src/ooLogChan.h 349671 
>   /trunk/addons/ooh323c/src/ooLogChan.c 349671 
>   /trunk/addons/ooh323c/src/ooStackCmds.h 349671 
>   /trunk/addons/ooh323c/src/ooStackCmds.c 349671 
>   /trunk/addons/ooh323c/src/ooh245.h 349671 
>   /trunk/addons/ooh323c/src/ooh245.c 349671 
>   /trunk/addons/ooh323c/src/ooh323.c 349671 
>   /trunk/addons/ooh323c/src/ooh323ep.c 349671 
>   /trunk/addons/ooh323c/src/ooq931.h 349671 
>   /trunk/addons/ooh323c/src/ooq931.c 349671 
>   /trunk/addons/ooh323cDriver.c 349671 
> 
> Diff: https://reviewboard.asterisk.org/r/1607/diff
> 
> 
> Testing
> -------
> 
> h.323->h.323 (fast start enabled & disabled), h.323->sip, sip->h.323 remote bridging are tested and work well.
> Another rtp based channels (sccp, jingle) are not tested but must work.
> 
> 
> Thanks,
> 
> may213
> 
>

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