rtpsession: Also send conflict event when sending packet
authorOlivier CrĂȘte <olivier.crete@collabora.com>
Wed, 23 Jan 2019 21:58:22 +0000 (16:58 -0500)
committerNicolas Dufresne <nicolas@ndufresne.ca>
Sat, 6 Jul 2019 14:23:20 +0000 (14:23 +0000)
If the conflict is detected when sending a packet, then also send an
upstream event to tell the source to reconfigure itself.

Also ignore the collision if we see more than one collision from the same
remote source to avoid problems on loops.

gst/rtpmanager/rtpsession.c
tests/check/elements/rtpsession.c

index 05dd6b5..081de50 100644 (file)
@@ -1579,6 +1579,27 @@ rtp_session_add_conflicting_address (RTPSession * sess,
       add_conflicting_address (sess->conflicting_addresses, address, time);
 }
 
+static void
+rtp_session_have_conflict (RTPSession * sess, RTPSource * source,
+    GSocketAddress * address, GstClockTime current_time)
+{
+  guint32 ssrc = rtp_source_get_ssrc (source);
+
+  /* Its a new collision, lets change our SSRC */
+  rtp_session_add_conflicting_address (sess, address, current_time);
+
+  /* mark the source BYE */
+  rtp_source_mark_bye (source, "SSRC Collision");
+  /* if we were suggesting this SSRC, change to something else */
+  if (sess->suggested_ssrc == ssrc) {
+    sess->suggested_ssrc = rtp_session_create_new_ssrc (sess);
+    sess->internal_ssrc_set = TRUE;
+  }
+
+  on_ssrc_collision (sess, source);
+
+  rtp_session_schedule_bye_locked (sess, current_time);
+}
 
 static gboolean
 check_collision (RTPSession * sess, RTPSource * source,
@@ -1672,22 +1693,11 @@ check_collision (RTPSession * sess, RTPSource * source,
        */
       GST_DEBUG ("Our packets are being looped back to us, dropping");
     } else {
-      /* Its a new collision, lets change our SSRC */
-      rtp_session_add_conflicting_address (sess, pinfo->address,
-          pinfo->current_time);
-
-      GST_DEBUG ("Collision for SSRC %x", ssrc);
-      /* mark the source BYE */
-      rtp_source_mark_bye (source, "SSRC Collision");
-      /* if we were suggesting this SSRC, change to something else */
-      if (sess->suggested_ssrc == ssrc) {
-        sess->suggested_ssrc = rtp_session_create_new_ssrc (sess);
-        sess->internal_ssrc_set = TRUE;
-      }
+      GST_DEBUG ("Collision for SSRC %x from new incoming packet,"
+          " change our sender ssrc", ssrc);
 
-      on_ssrc_collision (sess, source);
-
-      rtp_session_schedule_bye_locked (sess, pinfo->current_time);
+      rtp_session_have_conflict (sess, source, pinfo->address,
+          pinfo->current_time);
     }
   }
 
@@ -3115,9 +3125,31 @@ rtp_session_send_rtp (RTPSession * sess, gpointer data, gboolean is_list,
   if (created)
     on_new_sender_ssrc (sess, source);
 
-  if (!source->internal)
-    /* FIXME: Send GstRTPCollision upstream  */
-    goto collision;
+  if (!source->internal) {
+    GSocketAddress *from;
+
+    if (source->rtp_from)
+      from = source->rtp_from;
+    else
+      from = source->rtcp_from;
+    if (from) {
+      if (rtp_session_find_conflicting_address (sess, from, current_time)) {
+        /* Its a known conflict, its probably a loop, not a collision
+         * lets just drop the incoming packet
+         */
+        GST_LOG ("Our packets are being looped back to us, ignoring collision");
+      } else {
+        GST_DEBUG ("Collision for SSRC %x, change our sender ssrc", pinfo.ssrc);
+
+        rtp_session_have_conflict (sess, source, from, current_time);
+
+        goto collision;
+      }
+    } else {
+      GST_LOG ("Ignoring collision on sent SSRC %x because remote source"
+          " doesn't have an address", pinfo.ssrc);
+    }
+  }
 
   prevsender = RTP_SOURCE_IS_SENDER (source);
   oldrate = source->bitrate;
index ab9d36a..0e9f8cf 100644 (file)
@@ -949,22 +949,36 @@ add_rtcp_sdes_packet (GstBuffer * gstbuf, guint32 ssrc, const char *cname)
 GST_START_TEST (test_ssrc_collision_when_sending)
 {
   SessionHarness *h = session_harness_new ();
-  GstBuffer *buf = gst_rtcp_buffer_new (1400);
+  GstBuffer *buf;
+  GstEvent *ev;
+  GSocketAddress *saddr;
 
-/* Push SDES with identical SSRC as what we will use for sending RTP,
-   establishing this as a non-internal SSRC */
+  /* Push SDES with identical SSRC as what we will use for sending RTP,
+     establishing this as a non-internal SSRC */
+  buf = gst_rtcp_buffer_new (1400);
   add_rtcp_sdes_packet (buf, 0x12345678, "test@foo.bar");
+  saddr = g_inet_socket_address_new_from_string ("127.0.0.1", 8080);
+  gst_buffer_add_net_address_meta (buf, saddr);
+  g_object_unref (saddr);
   session_harness_recv_rtcp (h, buf);
 
+
   /* Push RTP buffer making our internal SSRC=0x12345678 */
-  fail_unless_equals_int (GST_FLOW_OK,
-      session_harness_send_rtp (h, generate_test_buffer (0, 0x12345678)));
+  buf = generate_test_buffer (0, 0x12345678);
+  fail_unless_equals_int (GST_FLOW_OK, session_harness_send_rtp (h, buf));
 
   /* Verify the packet we just sent is not being boomeranged back to us
      as a received packet! */
   fail_unless_equals_int (0, gst_harness_buffers_in_queue (h->recv_rtp_h));
 
-  /* FIXME: verify a Collision event coming upstream! */
+  while ((ev = gst_harness_try_pull_upstream_event (h->send_rtp_h)) != NULL) {
+    if (GST_EVENT_CUSTOM_UPSTREAM == GST_EVENT_TYPE (ev) &&
+        gst_event_has_name (ev, "GstRTPCollision"))
+      break;
+    gst_event_unref (ev);
+  }
+  fail_unless (ev != NULL);
+  gst_event_unref (ev);
 
   session_harness_free (h);
 }