[asterisk-dev] Guidance required

Fri Dec 6 01:43:04 CST 2013

Hi Matthew,

Thanks for Your response.

On 05 Dec 2013, at 19:56, Matthew Jordan <mjordan at digium.com> wrote:

> 
> On Thu, Dec 5, 2013 at 6:14 AM, Vytis Valentinavičius <xytis2000 at gmail.com> wrote:
> Hello,
> 
> I am trying to resolve asterisk bug https://issues.asterisk.org/jira/browse/ASTERISK-22911, but I have some questions regarding existing code:
> 
> [ I am working on 11.5.1 version ]
> 
> Hey Vytis -
> 
> For the patch that you're developing, feel free to attach it as a diff on the actual issue. You'll need to sign a license contributor agreement, but having the code on the issue would be the appropriate next step. I commented on the issue to that extent as well.
> 
> 
> Currently ICE session is initialised on first SDP INVITE, during initialisation it pairs received remote candidates with known local candidates and effectively establishes a route from asterisk to requesting client.
> When WebRTC client issues unhold reinvite, ICE session is not altered, thus rendering connection from asterisk to client broken.
> 
> How should I alter the ICE session?
> Things I already tried:
> 1. Force asterisk to provide PJICE with completely fresh copy of remote candidates (not a concat of newly received and old candidates) (I think this is a separate bug.)
> 2. Force ICE session to recheck all available candidates by either reseting the check container or by removing them altogether. Both of these approaches failed, and I suspect the failures are related to async STUN checking, that use previously registered checks. I could not find any documentation/methods/help within PJSIP.
> 
> Hm. Reading Section 9.1.1.1 of RFC 5245:
> 
> These rules imply that setting the IP address in the c line to
>    0.0.0.0 will cause an ICE restart.  Consequently, ICE implementations
>    MUST NOT utilize this mechanism for call hold, and instead MUST use
>    a=inactive and a=sendonly as described in [RFC3264].
> 
> When you get a call hold, how is it indicating the media restriction?
> 
> In general, looking through section 9.1.2, I would suspect that any serious modification of the media streams would require ICE to be restarted completely. 
> 

I checked the logs of SipMl5, to issue the hold it sends an INVITE with a=sendonly and receives 200 OK with a=recvonly.
To resume it negotiates a=sendrecv with m= line containing the new port.
Imho Asterisk correctly negotiates the resume AND calls ast_rtp_remote_address_set() with new address.
So I just need to restart the ice session, but ...

> 
> Things I would like to try, yet may not find a logical approach to them:
> 1. Scrap the whole ICE session and reinitiate a new one. That was my first idea yet I can not determine where the ICE session was created in the first place. All the places I suspected were not executed over the whole lifetime of asterisk application.
> 2. Just forget the session when receiving and SDP reinvite and build a new one. This seems logical, yet I can not determine will it work for all cases, since I know only my own little case.
> 
> Please help me, I am struggling for over a week...
> 
> So, the rtp_engine already has methods to start/stop the RTP instance in certain situations. This should include when chan_sip (or other channel drivers) need to hold the media. See ast_rtp_start/ast_rtp_stop in res_rtp_asterisk, which is what the virtual table for res_rtp_asterisk will use when the start/stop methods are called from the engine.
> 
> In those methods, have you tried stopping ICE and starting it using ast_rtp_ice_stop/ast_rtp_ice_start? 

It would be great if ast_rtp_start existed. I checked both 11.5.1 and trunk versions. Neither of them has a field in ast_rtp_engine which at least resembles ‘start’ method. Closest one is ast_rtp_activate, but it seems unconnected to ice sessions. This is one of the reasons I could not determine the location where should I restart ICE.

Another reason is that ast_rtp_ice_start does not actually create an ice session, where ast_rtp_ice_stop destroys one. Thus this pair should not be used for restarting as far as I can tell. (Unless ice session is spawned in other code, which is guaranteed to be run in between of those two calls. But then again — I have no proof for that.)

That being said — what I wrote is more of a hack than a solution.
1. I force ast_rtp_ice_start to provide only the newest candidates to pj_ice_sess_create_check_list by discarding existing list after it was given to pj.
2. I force pj_ice_sess_create_check_list to reset all existing checks when called with new candidate list.

I am unsure about correctness of my patch, since I am not really fond of neither asterisk nor pj internal architectures and I just took a guess how should I dispose the containers in both sides, thus I may have created a memory leak in asterisk. PJ seems to use static arrays for checklists, so that kind of makes it easier, yet another thread might be using the checklist thus when I reset the counter I force the threads to race on checks array.

I will attach the patch to the issue.

> 
> -- 
> Matthew Jordan
> Digium, Inc. | Engineering Manager
> 445 Jan Davis Drive NW - Huntsville, AL 35806 - USA
> Check us out at: http://digium.com & http://asterisk.org
> -- 
> _____________________________________________________________________
> -- 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

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.digium.com/pipermail/asterisk-dev/attachments/20131206/3255cd09/attachment-0001.html>