webrtcbin: only start gathering on local descriptions
authorMatthew Waters <matthew@centricular.com>
Wed, 29 Apr 2020 12:01:32 +0000 (22:01 +1000)
committerGStreamer Merge Bot <gitlab-merge-bot@gstreamer-foundation.org>
Thu, 30 Apr 2020 14:47:55 +0000 (14:47 +0000)
If we are in a state where we are answering, we would start gathering
when the offer is set which is incorrect for at least two reasons.

1. Sending ICE candidates before sending an answer is a hard error in
   all of the major browsers and will fail the negotiation.
2. If libnice ever adds the username fragment to the candidate for
   ice-restart hardening, the ice username and fragment would be
   incorrect.

JSEP also hints that the right call flow is to only start gathering when
a local description is set in 4.1.9 setLocalDescription

"This API indirectly controls the candidate gathering process."

as well as hints throughout other sections.

Part-of: <https://gitlab.freedesktop.org/gstreamer/gst-plugins-bad/-/merge_requests/1226>

ext/webrtc/gstwebrtcbin.c

index fd6e812..986cfb7 100644 (file)
@@ -4448,21 +4448,24 @@ _set_description_task (GstWebRTCBin * webrtc, struct set_description *sd)
 
     _get_ice_credentials_from_sdp_media (sd->sdp->sdp, i, &ufrag, &pwd);
 
-    if (sd->source == SDP_LOCAL)
+    if (sd->source == SDP_LOCAL) {
       gst_webrtc_ice_set_local_credentials (webrtc->priv->ice,
           item->stream, ufrag, pwd);
-    else
+    } else {
       gst_webrtc_ice_set_remote_credentials (webrtc->priv->ice,
           item->stream, ufrag, pwd);
+    }
     g_free (ufrag);
     g_free (pwd);
   }
 
-  for (i = 0; i < webrtc->priv->ice_stream_map->len; i++) {
-    IceStreamItem *item =
-        &g_array_index (webrtc->priv->ice_stream_map, IceStreamItem, i);
+  if (sd->source == SDP_LOCAL) {
+    for (i = 0; i < webrtc->priv->ice_stream_map->len; i++) {
+      IceStreamItem *item =
+          &g_array_index (webrtc->priv->ice_stream_map, IceStreamItem, i);
 
-    gst_webrtc_ice_gather_candidates (webrtc->priv->ice, item->stream);
+      gst_webrtc_ice_gather_candidates (webrtc->priv->ice, item->stream);
+    }
   }
 
   /* Add any pending trickle ICE candidates if we have both offer and answer */