Bug 1258753: Base candidate pair priority on controlling/controlled. r=drno
authorByron Campen [:bwc] <docfaraday@gmail.com>
Tue, 22 Mar 2016 12:22:56 -0500
changeset 291144 21a8c1db64fa61200e5e17f6140589407478c911
parent 291143 8772f2293eaba94a6890eadb525711e5d11ebf63
child 291145 f1cfe4b34e8da1b44551bff44827812a43d2864c
push id19656
push usergwagner@mozilla.com
push dateMon, 04 Apr 2016 13:43:23 +0000
treeherderb2g-inbound@e99061fde28a [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersdrno
bugs1258753
milestone48.0a1
Bug 1258753: Base candidate pair priority on controlling/controlled. r=drno MozReview-Commit-ID: 6RAFaAtBbJq
media/mtransport/third_party/nICEr/src/ice/ice_candidate_pair.c
media/mtransport/third_party/nICEr/src/ice/ice_media_stream.c
--- a/media/mtransport/third_party/nICEr/src/ice/ice_candidate_pair.c
+++ b/media/mtransport/third_party/nICEr/src/ice/ice_candidate_pair.c
@@ -44,20 +44,37 @@ static char *RCSSTRING __UNUSED__="$Id: 
 #include "stun.h"
 
 static char *nr_ice_cand_pair_states[]={"UNKNOWN","FROZEN","WAITING","IN_PROGRESS","FAILED","SUCCEEDED","CANCELLED"};
 
 static void nr_ice_candidate_pair_restart_stun_role_change_cb(NR_SOCKET s, int how, void *cb_arg);
 static void nr_ice_candidate_pair_compute_codeword(nr_ice_cand_pair *pair,
   nr_ice_candidate *lcand, nr_ice_candidate *rcand);
 
+static void nr_ice_candidate_pair_set_priority(nr_ice_cand_pair *pair)
+  {
+    /* Priority computation S 5.7.2 */
+    UINT8 controlling_priority, controlled_priority;
+    if(pair->pctx->controlling)
+    {
+      controlling_priority=pair->local->priority;
+      controlled_priority=pair->remote->priority;
+    }
+    else{
+      controlling_priority=pair->remote->priority;
+      controlled_priority=pair->local->priority;
+    }
+    pair->priority=(MIN(controlling_priority, controlled_priority))<<32 |
+      (MAX(controlling_priority, controlled_priority))<<1 |
+      (controlled_priority > controlling_priority?0:1);
+  }
+
 int nr_ice_candidate_pair_create(nr_ice_peer_ctx *pctx, nr_ice_candidate *lcand,nr_ice_candidate *rcand,nr_ice_cand_pair **pairp)
   {
     nr_ice_cand_pair *pair=0;
-    UINT8 o_priority, a_priority;
     int r,_status;
     UINT4 RTO;
     nr_ice_candidate tmpcand;
     UINT8 t_priority;
 
     if(!(pair=RCALLOC(sizeof(nr_ice_cand_pair))))
       ABORT(R_NO_MEMORY);
 
@@ -68,30 +85,17 @@ int nr_ice_candidate_pair_create(nr_ice_
     if(r=nr_concat_strings(&pair->as_string,pair->codeword,"|",lcand->addr.as_string,"|",
         rcand->addr.as_string,"(",lcand->label,"|",rcand->label,")", NULL))
       ABORT(r);
 
     nr_ice_candidate_pair_set_state(pctx,pair,NR_ICE_PAIR_STATE_FROZEN);
     pair->local=lcand;
     pair->remote=rcand;
 
-    /* Priority computation S 5.7.2 */
-    if(pctx->ctx->flags & NR_ICE_CTX_FLAGS_OFFERER)
-    {
-      assert(!(pctx->ctx->flags & NR_ICE_CTX_FLAGS_ANSWERER));
-
-      o_priority=lcand->priority;
-      a_priority=rcand->priority;
-    }
-    else{
-      o_priority=rcand->priority;
-      a_priority=lcand->priority;
-    }
-    pair->priority=(MIN(o_priority, a_priority))<<32 |
-      (MAX(o_priority, a_priority))<<1 | (o_priority > a_priority?0:1);
+    nr_ice_candidate_pair_set_priority(pair);
 
     /*
        TODO(bcampen@mozilla.com): Would be nice to log why this candidate was
        created (initial pair generation, triggered check, and new trickle
        candidate seem to be the possibilities here).
     */
     r_log(LOG_ICE,LOG_INFO,"ICE(%s)/CAND-PAIR(%s): Pairing candidate %s (%x):%s (%x) priority=%llu (%llx)",pctx->ctx->label,pair->codeword,lcand->addr.as_string,lcand->priority,rcand->addr.as_string,rcand->priority,pair->priority,pair->priority);
 
@@ -639,16 +643,17 @@ static void nr_ice_candidate_pair_restar
     r_log(LOG_ICE,LOG_INFO,"ICE-PEER(%s)/STREAM(%s)/CAND-PAIR(%s):COMP(%d): Restarting pair as %s: %s",pair->pctx->label,pair->local->stream->label,pair->codeword,pair->remote->component->component_id,pair->pctx->controlling ? "CONTROLLING" : "CONTROLLED",pair->as_string);
 
     nr_ice_candidate_pair_restart(pair->pctx, pair);
   }
 
 void nr_ice_candidate_pair_role_change(nr_ice_cand_pair *pair)
   {
     pair->stun_client->params.ice_binding_request.control = pair->pctx->controlling ? NR_ICE_CONTROLLING : NR_ICE_CONTROLLED;
+    nr_ice_candidate_pair_set_priority(pair);
 
     if(pair->state == NR_ICE_PAIR_STATE_IN_PROGRESS) {
       /* We could try only restarting in-progress pairs when they receive their
        * 487, but this ends up being simpler, because any extra 487 are dropped.
        */
       if(!pair->restart_role_change_cb_timer)
         NR_ASYNC_TIMER_SET(0,nr_ice_candidate_pair_restart_stun_role_change_cb,pair,&pair->restart_role_change_cb_timer);
     }
--- a/media/mtransport/third_party/nICEr/src/ice/ice_media_stream.c
+++ b/media/mtransport/third_party/nICEr/src/ice/ice_media_stream.c
@@ -866,18 +866,23 @@ int nr_ice_media_stream_disable_componen
 
     _status=0;
   abort:
     return(_status);
   }
 
 void nr_ice_media_stream_role_change(nr_ice_media_stream *stream)
   {
-    nr_ice_cand_pair *pair;
+    nr_ice_cand_pair *pair,*temp_pair;
+    /* Changing role causes candidate pair priority to change, which requires
+     * re-sorting the check list. */
+    nr_ice_cand_pair_head old_checklist=stream->check_list;
+    TAILQ_INIT(&stream->check_list);
+
     assert(stream->ice_state != NR_ICE_MEDIA_STREAM_UNPAIRED);
 
-    pair=TAILQ_FIRST(&stream->check_list);
-    while(pair){
+    TAILQ_FOREACH_SAFE(pair,&old_checklist,check_queue_entry,temp_pair) {
+      TAILQ_REMOVE(&old_checklist,pair,check_queue_entry);
       nr_ice_candidate_pair_role_change(pair);
-      pair=TAILQ_NEXT(pair,check_queue_entry);
+      nr_ice_candidate_pair_insert(&stream->check_list,pair);
     }
   }