<p>Jenkins2 <strong>merged</strong> this change.</p><p><a href="https://gerrit.asterisk.org/5917">View Change</a></p><div style="white-space:pre-wrap">Approvals:
  Kevin Harwell: Looks good to me, but someone else must approve
  George Joseph: Looks good to me, approved
  Jenkins2: Approved for Submit

</div><pre style="font-family: monospace,monospace; white-space: pre-wrap;">res_rtp_asterisk: Fix issues with ICE renegotiation.<br><br>When re-inviting to add more streams it is possible for<br>the role of existing ICE sessions to be changed to the<br>incorrect value. This results in subsequent refreshes<br>within the sessions getting a role conflict and the ICE<br>session breaking down. This change only sets the role to<br>be the new value if an ICE renegotiation is actually<br>going to happen, otherwise the existing role is preserved.<br><br>As well if we encounter a situation where a unidirectional<br>ICE negotiation happens and the other side does not send us<br>candidates we will not store any information for sending<br>traffic, even though we know where they are reachable. This<br>change fixes this by using the source of the ICE traffic<br>itself as the target if no candidates are known and we<br>receive some ICE traffic.<br><br>ASTERISK-27088<br><br>Change-Id: I71228181e358917fcefc3100fad21b2fc02a59a9<br>---<br>M res/res_rtp_asterisk.c<br>1 file changed, 18 insertions(+), 7 deletions(-)<br><br></pre><pre style="font-family: monospace,monospace; white-space: pre-wrap;">diff --git a/res/res_rtp_asterisk.c b/res/res_rtp_asterisk.c<br>index f6317f5..6714073 100644<br>--- a/res/res_rtp_asterisk.c<br>+++ b/res/res_rtp_asterisk.c<br>@@ -319,6 +319,7 @@<br>     ast_cond_t cond;            /*!< ICE/TURN condition for signaling */<br> <br>    struct ice_wrap *ice;       /*!< ao2 wrapped ICE session */<br>+       enum ast_rtp_ice_role role; /*!< Our role in ICE negotiation */<br>    pj_turn_sock *turn_rtp;     /*!< RTP TURN relay */<br>         pj_turn_sock *turn_rtcp;    /*!< RTCP TURN relay */<br>        pj_turn_state_t turn_state; /*!< Current state of the TURN relay session */<br>@@ -681,7 +682,6 @@<br> static int ice_reset_session(struct ast_rtp_instance *instance)<br> {<br>     struct ast_rtp *rtp = ast_rtp_instance_get_data(instance);<br>-   pj_ice_sess_role role = rtp->ice->real_ice->role;<br>    int res;<br> <br>   ast_debug(3, "Resetting ICE for RTP instance '%p'\n", instance);<br>@@ -693,8 +693,9 @@<br>       ast_debug(3, "Recreating ICE session %s (%d) for RTP instance '%p'\n", ast_sockaddr_stringify(&rtp->ice_original_rtp_addr), rtp->ice_port, instance);<br>     res = ice_create(instance, &rtp->ice_original_rtp_addr, rtp->ice_port, 1);<br>  if (!res) {<br>-          /* Preserve the role that the old ICE session used */<br>-                pj_ice_sess_change_role(rtp->ice->real_ice, role);<br>+             /* Use the current expected role for the ICE session */<br>+              pj_ice_sess_change_role(rtp->ice->real_ice, rtp->role == AST_RTP_ICE_ROLE_CONTROLLED ?<br>+                      PJ_ICE_SESS_ROLE_CONTROLLED : PJ_ICE_SESS_ROLE_CONTROLLING);<br>  }<br> <br>  /* If we only have one component now, and we previously set up TURN for RTCP,<br>@@ -765,6 +766,8 @@<br>            ast_debug(3, "Proposed == active candidates for RTP instance '%p'\n", instance);<br>            ao2_cleanup(rtp->ice_proposed_remote_candidates);<br>          rtp->ice_proposed_remote_candidates = NULL;<br>+               /* If this ICE session is being preserved then go back to the role it currently is */<br>+                rtp->role = rtp->ice->real_ice->role;<br>             return;<br>       }<br> <br>@@ -938,10 +941,7 @@<br>            return;<br>       }<br> <br>- pj_thread_register_check();<br>-<br>-       pj_ice_sess_change_role(rtp->ice->real_ice, role == AST_RTP_ICE_ROLE_CONTROLLED ?<br>-              PJ_ICE_SESS_ROLE_CONTROLLED : PJ_ICE_SESS_ROLE_CONTROLLING);<br>+ rtp->role = role;<br> }<br> <br> /*! \pre instance is locked */<br>@@ -2522,6 +2522,17 @@<br>                  return -1;<br>            }<br>             if (!rtp->passthrough) {<br>+                  /* If a unidirectional ICE negotiation occurs then lock on to the source of the<br>+                       * ICE traffic and use it as the target. This will occur if the remote side only<br>+                      * wants to receive media but never send to us.<br>+                       */<br>+                  if (!rtp->ice_active_remote_candidates && !rtp->ice_proposed_remote_candidates) {<br>+                              if (rtcp) {<br>+                                  ast_sockaddr_copy(&rtp->rtcp->them, sa);<br>+                           } else {<br>+                                     ast_rtp_instance_set_remote_address(instance, sa);<br>+                           }<br>+                    }<br>                     return 0;<br>             }<br>             rtp->passthrough = 0;<br></pre><p>To view, visit <a href="https://gerrit.asterisk.org/5917">change 5917</a>. To unsubscribe, visit <a href="https://gerrit.asterisk.org/settings">settings</a>.</p><div itemscope itemtype="http://schema.org/EmailMessage"><div itemscope itemprop="action" itemtype="http://schema.org/ViewAction"><link itemprop="url" href="https://gerrit.asterisk.org/5917"/><meta itemprop="name" content="View Change"/></div></div>

<div style="display:none"> Gerrit-Project: asterisk </div>
<div style="display:none"> Gerrit-Branch: 14 </div>
<div style="display:none"> Gerrit-MessageType: merged </div>
<div style="display:none"> Gerrit-Change-Id: I71228181e358917fcefc3100fad21b2fc02a59a9 </div>
<div style="display:none"> Gerrit-Change-Number: 5917 </div>
<div style="display:none"> Gerrit-PatchSet: 2 </div>
<div style="display:none"> Gerrit-Owner: Joshua Colp <jcolp@digium.com> </div>
<div style="display:none"> Gerrit-Reviewer: George Joseph <gjoseph@digium.com> </div>
<div style="display:none"> Gerrit-Reviewer: Jenkins2 </div>
<div style="display:none"> Gerrit-Reviewer: Joshua Colp <jcolp@digium.com> </div>
<div style="display:none"> Gerrit-Reviewer: Kevin Harwell <kharwell@digium.com> </div>