----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3982/#review13261 -----------------------------------------------------------
/branches/13/res/res_rtp_asterisk.c <https://reviewboard.asterisk.org/r/3982/#comment23735> This was clearly in this code prior to this patch, but it'd be nice if component were typed to ast_rtp_ice_component_type. That would make it less likely that an arbitrary integer is passed to the function and going beyond the bounds of the array. /branches/13/res/res_rtp_asterisk.c <https://reviewboard.asterisk.org/r/3982/#comment23736> Nitpick: no space between (int) and status. /branches/13/res/res_rtp_asterisk.c <https://reviewboard.asterisk.org/r/3982/#comment23737> Naming nitpick: thread is a bit too close to a keyword (it certainly gets bolded in reviewboard). I'd rename it to something that won't be quite as confusing in review tools/IDEs/things that like markup. - Matt Jordan 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
