----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3256/#review10937 -----------------------------------------------------------
/branches/11/res/res_rtp_asterisk.c <https://reviewboard.asterisk.org/r/3256/#comment20567> Agreed, and pjnath does not provide a mechanism to do just that without destroying/re-creating as Matt says. What would also be useful is to further look at the SDPs involved - are the candidates really changing? Do we really need to restart the ICE negotiation? - Joshua Colp On Feb. 24, 2014, 6:36 p.m., Jonathan Rose wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviewboard.asterisk.org/r/3256/ > ----------------------------------------------------------- > > (Updated Feb. 24, 2014, 6:36 p.m.) > > > Review request for Asterisk Developers, Joshua Colp, Kevin Harwell, and Matt > Jordan. > > > Bugs: ASTERISK-22911 and ASTERISK-23213 > https://issues.asterisk.org/jira/browse/ASTERISK-22911 > https://issues.asterisk.org/jira/browse/ASTERISK-23213 > > > Repository: Asterisk > > > Description > ------- > > Let me start by saying this is almost certainly not the complete answer to > solving these problems. This patch is simply an alternative to backing out > the patch from r405234 and leaving the existing aborts in place. I've written > a test to make sure the new patch (and likely a later patch which can resolve > these problems with ICE more comprehensively) does not crash Asterisk and > that can be view here: https://reviewboard.asterisk.org/r/3255/ > > In my reproduction of this regression, I noticed a few things. The first was > that when starting a call with ICE from SIPML5 that Asterisk would not be > able to full initialize the ICE session. Instead when creating the candidate > checklist via pj_ice_sess_create_check_list, PJNATH would be unable to > associate the srflx candidates with any host pairs when pruning the > checklist. The SRFLX candidates would have the addresses used internally on > my LAN while the addresses it would be matched up against would mirror those > of my external IP (in other words, they just didn't match by address). The > overall return from pj_ice_sess_create_check_list would be > PJNATH_EICENOHOSTCAND and while the ICE session wouldn't get flagged as > started, I still continued to receive two way audio on both ends in Asterisk > 11.7 > > Asterisk 11.8 however would clear the candidate lists at this point and I > would get one way audio instead. > > The crash in 11.7 from holding was caused because upon holding the call, the > SDP would contain the same list of ICE candidates, so Asterisk would attempt > to start the ICE session again. This time when it entered the create check > list function, the maximum ICE candidates in the session would be exhausted > causing PJNATH to assert and abort (which visibly appears more or less the > same as a crash). This work around patch checks to see if a create checklist > call will cause that value to expand above the maximum and if it would, we > abandon starting ICE back up and we clear the current candidate list on the > ICE session. In my own tests this seemed to work quite well (I had two way > audio, Asterisk didn't terminate suddenly, and I was able to hold and resume > the call while still maintaining two way audio and music on hold for all > expected ends). According to Vytis though, neither this patch nor the > original patch to resolve ASTERISK-22911 by kharwell actually fixed his audio > problems anyway . Kharwell's patch fixed the assertions that were occurring because the candidate list would be cleared on any error including the PJNATH_EICENOHOSTCAND which most of the people in ASTERISK-23213 were most likely experiencing as well. I draw that conclusion because when they used this patch they reported that it fixed their issues without introducing any new crashes. > > At this point, I'm not entirely sure that audio actually should be allowed to > work when building the ICE session check list fails like it has been in this > scenario. But we need to finalize a new Asterisk 11 release soon and we can't > have this apparent regression in when we do so. In the future we need to find > out more comprehensively why pj_ice_sess_create_check_list is failing and how > to fix that. > > > Props to Lott Caskey for providing some improvements to the patch that I > wrote. > > > Diffs > ----- > > /branches/11/res/res_rtp_asterisk.c 408284 > > Diff: https://reviewboard.asterisk.org/r/3256/diff/ > > > Testing > ------- > > Tested calls and holds with a SIPML5 call to a desk phone. Confirmed two way > audio and music on hold. > Requested that the reporter and participants of ASTERISK-23213 test the > patch. Everyone who tested it confirmed that it fixed their problems. > Ran https://reviewboard.asterisk.org/r/3255/ and checked the log files to see > what was happening > > > 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
