> On Sept. 8, 2014, 1:35 p.m., Matt Jordan wrote:
> > /branches/13/res/res_rtp_asterisk.c, line 850
> > <https://reviewboard.asterisk.org/r/3982/diff/1/?file=67291#file67291line850>
> >
> > Nitpick: no space between (int) and status.
>
> rmudgett wrote:
> I prefer
> (int) status,
> over
> (int)status,
>
> Matt Jordan wrote:
> Generally, the coding guidelines would suggest that you not do that
> however:
>
> {quote}
> -nprs: no space after parentheses
> {quote}
>
> I'll grant nothing calls that out specifically with casting, and the
> convention in the code is all over the map on this one. I'd guess based on
> some quick grepping that we tend to use (int)foo more than (int) foo, but
> that's just a quick gut check.
That option refers to spaces between parentheses "if ( x )" vs "if (x)" and
"func( x )" vs "func(x)".
{quote}
-nprs, --no-space-after-parentheses
Do not put a space after every ’(’ and before every ’)’.
See STATEMENTS.
{quote}
This is the one referring to casts and it says to put one there.
{quote}
-cs, --space-after-cast
Put a space after a cast operator.
See STATEMENTS.
{quote}
But this is getting into fine hair splitting. :)
- rmudgett
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviewboard.asterisk.org/r/3982/#review13261
-----------------------------------------------------------
On Sept. 7, 2014, 10:07 a.m., Joshua Colp wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/3982/
> -----------------------------------------------------------
>
> (Updated Sept. 7, 2014, 10:07 a.m.)
>
>
> Review request for Asterisk Developers.
>
>
> Bugs: ASTERISK-23577 and ASTERISK-23634
> https://issues.asterisk.org/jira/browse/ASTERISK-23577
> https://issues.asterisk.org/jira/browse/ASTERISK-23634
>
>
> Repository: Asterisk
>
>
> Description
> -------
>
> The TURN support in res_rtp_asterisk has been exercised by only a few, which
> has uncovered a slew of issues.
>
> 1. The number of file descriptors that an ioqueue instance in pjlib can
> support is a fixed number. Exceeding this causes an assertion. The code will
> now dynamically create/terminate threads as needed to ensure that this limit
> is not exceeded on ioqueue instances.
> 2. The API did not allow users of the TURN code to explicitly request a TURN
> session with details. This has been added.
> 3. The ICE code has a fixed size array of 4 for transports. As our transport
> identifiers started at 1 we were exceeding this causing an assertion. Our
> identifiers now start at 0.
> 4. The TURN client did not set up client binding causing needless bandwidth
> usage. Upon ICE completion if TURN is used channel binding is now established.
> 5. The code will no longer update address information on every sent packet.
> The remote address is now updated only once upon ICE completion to the target
> address.
> 6. When relaying was in use STUN traffic was getting looped back to
> res_rtp_asterisk due to it being handled on the normal socket. It is now
> handled in the TURN session callback instead.
> 7. Logging when a TURN relay is in use now uses the IP address that the TURN
> relay is sending/receiving to/from on our behalf.
> 8. Synchronization between the TURN session and the RTP instance now ensures
> that the session has been fully created or fully destroyed before continuing.
> 9. Some cleanup has occurred when tearing down the pj environment.
>
>
> Diffs
> -----
>
> /branches/13/res/res_rtp_asterisk.c 422835
> /branches/13/include/asterisk/rtp_engine.h 422835
>
> Diff: https://reviewboard.asterisk.org/r/3982/diff/
>
>
> Testing
> -------
>
> Sabotaged the code so the only candidates I sent were of the relay type.
> Confirmed bidirectional media flow using the TURN relay.
>
>
> Thanks,
>
> Joshua Colp
>
>
--
_____________________________________________________________________
-- Bandwidth and Colocation Provided by http://www.api-digital.com --
asterisk-dev mailing list
To UNSUBSCRIBE or update options visit:
http://lists.digium.com/mailman/listinfo/asterisk-dev
