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

[Paul Belanger]

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


There is still a lot of work to bring this module up to specs with the CODING-GUIDELINES.  Still a lot of issue with if statements, tab indentation, red blobs, etc.  I believe now is the time to resolve these issues.


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

    invert logic to avoided nested logic. 
    
    if (!changed) {
        return;
    )



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

    space



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

    braces



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

    same, since we are here



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

    spacing and blobs



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

    while (...) {
    
    }



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

    blob



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

    if (...) {
    
    } else {
    
    }



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

    if (...) {
    
    }



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

    same



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

    same



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

    same



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

    same



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

    spacing



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

    blob



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

    same



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

    space and braces



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

    same



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

    same


- Paul


On Dec. 9, 2011, 1:03 p.m., may213 wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/1607/
> -----------------------------------------------------------
> 
> (Updated Dec. 9, 2011, 1:03 p.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 347865 
>   /trunk/addons/chan_ooh323.h 347865 
>   /trunk/addons/chan_ooh323.c 347865 
>   /trunk/addons/ooh323c/src/ooCalls.h 347865 
>   /trunk/addons/ooh323c/src/ooCmdChannel.c 347865 
>   /trunk/addons/ooh323c/src/ooLogChan.h 347865 
>   /trunk/addons/ooh323c/src/ooLogChan.c 347865 
>   /trunk/addons/ooh323c/src/ooStackCmds.h 347865 
>   /trunk/addons/ooh323c/src/ooStackCmds.c 347865 
>   /trunk/addons/ooh323c/src/ooh245.h 347865 
>   /trunk/addons/ooh323c/src/ooh245.c 347865 
>   /trunk/addons/ooh323c/src/ooh323.c 347865 
>   /trunk/addons/ooh323c/src/ooh323ep.c 347865 
>   /trunk/addons/ooh323c/src/ooq931.h 347865 
>   /trunk/addons/ooh323c/src/ooq931.c 347865 
>   /trunk/addons/ooh323cDriver.c 347865 
> 
> 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/20120103/9ca66208/attachment-0001.htm>