[asterisk-dev] [Code Review]: chan_ooh323 direct rtp (remote bridging) support
may213
reviewboard at asterisk.org
Thu Jan 5 09:09:24 CST 2012
> On Jan. 3, 2012, 11:29 a.m., Paul Belanger wrote:
> > /trunk/addons/chan_ooh323.c, line 4317
> > <https://reviewboard.asterisk.org/r/1607/diff/2/?file=22178#file22178line4317>
> >
> > invert logic to avoided nested logic.
> >
> > if (!changed) {
> > return;
> > )
I think it is more visible what this part is executed if there is change of rtp peer detected and
we must not just return if nothing changed but unlock mutex and free allocated callToken as per end of
function.
> On Jan. 3, 2012, 11:29 a.m., Paul Belanger wrote:
> > /trunk/addons/chan_ooh323.c, line 4505
> > <https://reviewboard.asterisk.org/r/1607/diff/2/?file=22178#file22178line4505>
> >
> > space
fixed
> On Jan. 3, 2012, 11:29 a.m., Paul Belanger wrote:
> > /trunk/addons/chan_ooh323.c, line 4535
> > <https://reviewboard.asterisk.org/r/1607/diff/2/?file=22178#file22178line4535>
> >
> > braces
fixed
> On Jan. 3, 2012, 11:29 a.m., Paul Belanger wrote:
> > /trunk/addons/chan_ooh323.c, lines 4659-4661
> > <https://reviewboard.asterisk.org/r/1607/diff/2/?file=22178#file22178line4659>
> >
> > same, since we are here
fixed
> On Jan. 3, 2012, 11:29 a.m., Paul Belanger wrote:
> > /trunk/addons/ooh323c/src/ooCmdChannel.c, lines 417-419
> > <https://reviewboard.asterisk.org/r/1607/diff/2/?file=22180#file22180line417>
> >
> > spacing and blobs
fixed
> On Jan. 3, 2012, 11:29 a.m., Paul Belanger wrote:
> > /trunk/addons/ooh323c/src/ooLogChan.c, lines 266-267
> > <https://reviewboard.asterisk.org/r/1607/diff/2/?file=22182#file22182line266>
> >
> > while (...) {
> >
> > }
fixed
> On Jan. 3, 2012, 11:29 a.m., Paul Belanger wrote:
> > /trunk/addons/ooh323c/src/ooLogChan.c, line 269
> > <https://reviewboard.asterisk.org/r/1607/diff/2/?file=22182#file22182line269>
> >
> > blob
fixed
> On Jan. 3, 2012, 11:29 a.m., Paul Belanger wrote:
> > /trunk/addons/ooh323c/src/ooLogChan.c, lines 271-275
> > <https://reviewboard.asterisk.org/r/1607/diff/2/?file=22182#file22182line271>
> >
> > if (...) {
> >
> > } else {
> >
> > }
fixed
> On Jan. 3, 2012, 11:29 a.m., Paul Belanger wrote:
> > /trunk/addons/ooh323c/src/ooStackCmds.c, lines 553-556
> > <https://reviewboard.asterisk.org/r/1607/diff/2/?file=22184#file22184line553>
> >
> > if (...) {
> >
> > }
fixed
> On Jan. 3, 2012, 11:29 a.m., Paul Belanger wrote:
> > /trunk/addons/ooh323c/src/ooStackCmds.c, lines 558-560
> > <https://reviewboard.asterisk.org/r/1607/diff/2/?file=22184#file22184line558>
> >
> > same
fixed
> On Jan. 3, 2012, 11:29 a.m., Paul Belanger wrote:
> > /trunk/addons/ooh323c/src/ooStackCmds.c, lines 566-570
> > <https://reviewboard.asterisk.org/r/1607/diff/2/?file=22184#file22184line566>
> >
> > same
fixed
> On Jan. 3, 2012, 11:29 a.m., Paul Belanger wrote:
> > /trunk/addons/ooh323c/src/ooStackCmds.c, lines 578-579
> > <https://reviewboard.asterisk.org/r/1607/diff/2/?file=22184#file22184line578>
> >
> > same
fixed
> On Jan. 3, 2012, 11:29 a.m., Paul Belanger wrote:
> > /trunk/addons/ooh323c/src/ooStackCmds.c, lines 580-582
> > <https://reviewboard.asterisk.org/r/1607/diff/2/?file=22184#file22184line580>
> >
> > same
fixed
> On Jan. 3, 2012, 11:29 a.m., Paul Belanger wrote:
> > /trunk/addons/ooh323c/src/ooStackCmds.c, line 590
> > <https://reviewboard.asterisk.org/r/1607/diff/2/?file=22184#file22184line590>
> >
> > spacing
fixed
> On Jan. 3, 2012, 11:29 a.m., Paul Belanger wrote:
> > /trunk/addons/ooh323c/src/ooStackCmds.c, line 591
> > <https://reviewboard.asterisk.org/r/1607/diff/2/?file=22184#file22184line591>
> >
> > blob
fixed
> On Jan. 3, 2012, 11:29 a.m., Paul Belanger wrote:
> > /trunk/addons/ooh323c/src/ooStackCmds.c, lines 592-598
> > <https://reviewboard.asterisk.org/r/1607/diff/2/?file=22184#file22184line592>
> >
> > same
fixed
> On Jan. 3, 2012, 11:29 a.m., Paul Belanger wrote:
> > /trunk/addons/ooh323cDriver.c, lines 240-241
> > <https://reviewboard.asterisk.org/r/1607/diff/2/?file=22191#file22191line240>
> >
> > same
fixed
> On Jan. 3, 2012, 11:29 a.m., Paul Belanger wrote:
> > /trunk/addons/ooh323cDriver.c, lines 250-251
> > <https://reviewboard.asterisk.org/r/1607/diff/2/?file=22191#file22191line250>
> >
> > same
fixed
- may213
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviewboard.asterisk.org/r/1607/#review5101
-----------------------------------------------------------
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/20120105/32de5a79/attachment-0001.htm>
More information about the asterisk-dev
mailing list