----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3275/#review11002 -----------------------------------------------------------
/branches/11/res/res_rtp_asterisk.c <https://reviewboard.asterisk.org/r/3275/#comment20634> For the sake of readability, line up the comments using spaces /branches/11/res/res_rtp_asterisk.c <https://reviewboard.asterisk.org/r/3275/#comment20633> Although this was already named local_candidates, in order to keep the naming consistent with the other containers, you may want to consider renaming this to 'ice_local_candidates' as well. No worries if you don't want to however. /branches/11/res/res_rtp_asterisk.c <https://reviewboard.asterisk.org/r/3275/#comment20635> This function duplicates a lot of code in ast_rtp_new. I'd create a new function for the shared code of actually making the ICE session called 'ice_create'. That should actually return a -1/0 for failure/success, as pj_ice_sess_create and technically fail. If the ICE session creation fails, we should not set ice_started. /branches/11/res/res_rtp_asterisk.c <https://reviewboard.asterisk.org/r/3275/#comment20637> Once you've refactored this code into ice_create, assign rtp->ice_port on success. - Matt Jordan On Feb. 27, 2014, 3:53 p.m., Jonathan Rose wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviewboard.asterisk.org/r/3275/ > ----------------------------------------------------------- > > (Updated Feb. 27, 2014, 3:53 p.m.) > > > Review request for Asterisk Developers, Joshua Colp, Kevin Harwell, and Matt > Jordan. > > > Bugs: ASTERISK-22911 > https://issues.asterisk.org/jira/browse/ASTERISK-22911 > > > Repository: Asterisk > > > Description > ------- > > This patch provides a fix for the hold problem by doing the following: > > Once an ICE session is marked as started, we start adding any new remote > candidates into a separate list until we get another attempt to start the ICE > session. > Once a call to start the ice session is made, instead of immediately quitting > if the session is already started, we check for a difference in the two > candidates lists. If the lists are identical, we wipe out the new list and > keep the old one and just quit then going on with the current ICE session. If > the lists are changed, we toss the old list and adopt the new one and restart > the ICE session. > > > Diffs > ----- > > /branches/11/res/res_rtp_asterisk.c 409132 > > Diff: https://reviewboard.asterisk.org/r/3275/diff/ > > > Testing > ------- > > SIPML client to Asterisk to Desk Phone > SIPML calls desk phone > audio test, got two way audio > SIPML holds call > SIPML resumes call > audio test, got two way audio (previously this would cause one way audio from > the SIPML client to the desk phone) > > > Thanks, > > Jonathan Rose > >
-- _____________________________________________________________________ -- 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
