rtpsession: Don't start the RTCP thread until it's needed

Always wait with starting the RTCP thread until either a RTP or RTCP
packet is sent or received. Special handling is needed to make sure the
RTCP thread is started when requesting an early RTCP packet.

We want to wait with starting the RTCP thread until it's needed in order
to not send RTCP packets for an inactive source.

https://bugzilla.gnome.org/show_bug.cgi?id=795139

diff --git a/gst/rtpmanager/gstrtpsession.c b/gst/rtpmanager/gstrtpsession.c
index 0c42fb7..6c1bba0 100644 (file)
--- a/gst/rtpmanager/gstrtpsession.c
+++ b/gst/rtpmanager/gstrtpsession.c
@@ -303,6 +303,8 @@ static GstClockTime gst_rtp_session_request_time (RTPSession * session,
 static void gst_rtp_session_notify_nack (RTPSession * sess,
     guint16 seqnum, guint16 blp, guint32 ssrc, gpointer user_data);
 static void gst_rtp_session_reconfigure (RTPSession * sess, gpointer user_data);
+static void gst_rtp_session_notify_early_rtcp (RTPSession * sess,
+    gpointer user_data);
 
 static RTPSessionCallbacks callbacks = {
   gst_rtp_session_process_rtp,
@@ -314,7 +316,8 @@ static RTPSessionCallbacks callbacks = {
   gst_rtp_session_request_key_unit,
   gst_rtp_session_request_time,
   gst_rtp_session_notify_nack,
-  gst_rtp_session_reconfigure
+  gst_rtp_session_reconfigure,
+  gst_rtp_session_notify_early_rtcp
 };
 
 /* GObject vmethods */
@@ -1239,8 +1242,7 @@ gst_rtp_session_change_state (GstElement * element, GstStateChange transition)
       break;
     case GST_STATE_CHANGE_READY_TO_PAUSED:
       GST_RTP_SESSION_LOCK (rtpsession);
-      if (rtpsession->send_rtp_src)
-        rtpsession->priv->wait_send = TRUE;
+      rtpsession->priv->wait_send = TRUE;
       GST_RTP_SESSION_UNLOCK (rtpsession);
       break;
     case GST_STATE_CHANGE_PAUSED_TO_PLAYING:
@@ -2713,3 +2715,15 @@ gst_rtp_session_reconfigure (RTPSession * sess, gpointer user_data)
     gst_object_unref (send_rtp_sink);
   }
 }
+
+static void
+gst_rtp_session_notify_early_rtcp (RTPSession * sess, gpointer user_data)
+{
+  GstRtpSession *rtpsession = GST_RTP_SESSION (user_data);
+
+  GST_DEBUG_OBJECT (rtpsession, "Notified of early RTCP");
+  /* with an early RTCP request, we might have to start the RTCP thread */
+  GST_RTP_SESSION_LOCK (rtpsession);
+  signal_waiting_rtcp_thread_unlocked (rtpsession);
+  GST_RTP_SESSION_UNLOCK (rtpsession);
+}

diff --git a/gst/rtpmanager/rtpsession.c b/gst/rtpmanager/rtpsession.c
index 8dda369..da27abd 100644 (file)
--- a/gst/rtpmanager/rtpsession.c
+++ b/gst/rtpmanager/rtpsession.c
@@ -1114,6 +1114,10 @@ rtp_session_set_callbacks (RTPSession * sess, RTPSessionCallbacks * callbacks,
     sess->callbacks.reconfigure = callbacks->reconfigure;
     sess->reconfigure_user_data = user_data;
   }
+  if (callbacks->notify_early_rtcp) {
+    sess->callbacks.notify_early_rtcp = callbacks->notify_early_rtcp;
+    sess->notify_early_rtcp_user_data = user_data;
+  }
 }
 
 /**
@@ -4371,6 +4375,10 @@ rtp_session_send_rtcp (RTPSession * sess, GstClockTime max_delay)
 
   now = sess->callbacks.request_time (sess, sess->request_time_user_data);
 
+  /* notify the application that we intend to send early RTCP */
+  if (sess->callbacks.notify_early_rtcp)
+    sess->callbacks.notify_early_rtcp (sess, sess->notify_early_rtcp_user_data);
+
   return rtp_session_request_early_rtcp (sess, now, max_delay);
 }
 

diff --git a/gst/rtpmanager/rtpsession.h b/gst/rtpmanager/rtpsession.h
index 99e5dd6..e849d81 100644 (file)
--- a/gst/rtpmanager/rtpsession.h
+++ b/gst/rtpmanager/rtpsession.h
@@ -167,6 +167,16 @@ typedef void (*RTPSessionNotifyNACK) (RTPSession *sess,
 typedef void (*RTPSessionReconfigure) (RTPSession *sess, gpointer user_data);
 
 /**
+ * RTPSessionNotifyEarlyRTCP:
+ * @sess: an #RTPSession
+ * @user_data: user data specified when registering
+ *
+ * Notifies of early RTCP being requested
+ */
+typedef void (*RTPSessionNotifyEarlyRTCP) (RTPSession *sess,
+    gpointer user_data);
+
+/**
  * RTPSessionCallbacks:
  * @RTPSessionProcessRTP: callback to process RTP packets
  * @RTPSessionSendRTP: callback for sending RTP packets
@@ -177,6 +187,7 @@ typedef void (*RTPSessionReconfigure) (RTPSession *sess, gpointer user_data);
  * @RTPSessionRequestTime: callback for requesting the current time
  * @RTPSessionNotifyNACK: callback for notifying NACK
  * @RTPSessionReconfigure: callback for requesting reconfiguration
+ * @RTPSessionNotifyEarlyRTCP: callback for notifying early RTCP
  *
  * These callbacks can be installed on the session manager to get notification
  * when RTP and RTCP packets are ready for further processing. These callbacks
@@ -193,6 +204,7 @@ typedef struct {
   RTPSessionRequestTime request_time;
   RTPSessionNotifyNACK  notify_nack;
   RTPSessionReconfigure reconfigure;
+  RTPSessionNotifyEarlyRTCP notify_early_rtcp;
 } RTPSessionCallbacks;
 
 /**
@@ -269,6 +281,7 @@ struct _RTPSession {
   gpointer              request_time_user_data;
   gpointer              notify_nack_user_data;
   gpointer              reconfigure_user_data;
+  gpointer              notify_early_rtcp_user_data;
 
   RTPSessionStats stats;
   RTPSessionStats bye_stats;

diff --git a/tests/check/elements/rtpsession.c b/tests/check/elements/rtpsession.c
index 73851b3..3b44f8e 100644 (file)
--- a/tests/check/elements/rtpsession.c
+++ b/tests/check/elements/rtpsession.c
@@ -699,43 +699,33 @@ stats_test_cb (GObject * object, GParamSpec * spec, gpointer data)
   guint num_sources = 0;
   gboolean *cb_called = data;
   g_assert (*cb_called == FALSE);
-  *cb_called = TRUE;
 
   /* We should be able to get a rtpsession property
      without introducing the deadlock */
   g_object_get (object, "num-sources", &num_sources, NULL);
+
+  *cb_called = TRUE;
 }
 
 GST_START_TEST (test_dont_lock_on_stats)
 {
-  GstHarness *h_rtcp;
-  GstHarness *h_send;
-  GstClock *clock = gst_test_clock_new ();
-  GstTestClock *testclock = GST_TEST_CLOCK (clock);
+  SessionHarness *h = session_harness_new ();
   gboolean cb_called = FALSE;
 
-  /* use testclock as the systemclock to capture the rtcp thread waits */
-  gst_system_clock_set_default (GST_CLOCK (testclock));
-
-  h_rtcp =
-      gst_harness_new_with_padnames ("rtpsession", "recv_rtcp_sink",
-      "send_rtcp_src");
-  h_send =
-      gst_harness_new_with_element (h_rtcp->element, "send_rtp_sink",
-      "send_rtp_src");
-
   /* connect to the stats-reporting */
-  g_signal_connect (h_rtcp->element, "notify::stats",
+  g_signal_connect (h->session, "notify::stats",
       G_CALLBACK (stats_test_cb), &cb_called);
 
-  /* "crank" and check the stats */
-  g_assert (gst_test_clock_crank (testclock));
-  gst_buffer_unref (gst_harness_pull (h_rtcp));
+  /* Push RTP buffer to make sure RTCP-thread have started */
+  fail_unless_equals_int (GST_FLOW_OK,
+      session_harness_send_rtp (h, generate_test_buffer (0, 0xDEADBEEF)));
+
+  /* crank the RTCP-thread and pull out rtcp, generating a stats-callback */
+  session_harness_crank_clock (h);
+  gst_buffer_unref (session_harness_pull_rtcp (h));
   fail_unless (cb_called);
 
-  gst_harness_teardown (h_send);
-  gst_harness_teardown (h_rtcp);
-  gst_object_unref (clock);
+  session_harness_free (h);
 }
 
 GST_END_TEST;
@@ -1205,6 +1195,45 @@ GST_START_TEST (test_feedback_rtcp_race)
 
 GST_END_TEST;
 
+GST_START_TEST (test_dont_send_rtcp_while_idle)
+{
+  SessionHarness *h = session_harness_new ();
+
+  /* verify the RTCP thread has not started */
+  fail_unless_equals_int (0, gst_test_clock_peek_id_count (h->testclock));
+  /* and that no RTCP has been pushed */
+  fail_unless_equals_int (0, gst_harness_buffers_in_queue (h->rtcp_h));
+
+  session_harness_free (h);
+}
+
+GST_END_TEST;
+
+GST_START_TEST (test_send_rtcp_when_signalled)
+{
+  SessionHarness *h = session_harness_new ();
+  gboolean ret;
+
+  /* verify the RTCP thread has not started */
+  fail_unless_equals_int (0, gst_test_clock_peek_id_count (h->testclock));
+  /* and that no RTCP has been pushed */
+  fail_unless_equals_int (0, gst_harness_buffers_in_queue (h->rtcp_h));
+
+  /* then ask explicitly to send RTCP */
+  g_signal_emit_by_name (h->internal_session,
+      "send-rtcp-full", GST_SECOND, &ret);
+  /* this is FALSE due to no next RTCP check time */
+  fail_unless (ret == FALSE);
+
+  /* "crank" and verify RTCP now was sent */
+  session_harness_crank_clock (h);
+  gst_buffer_unref (session_harness_pull_rtcp (h));
+
+  session_harness_free (h);
+}
+
+GST_END_TEST;
+
 static Suite *
 rtpsession_suite (void)
 {
@@ -1226,6 +1255,8 @@ rtpsession_suite (void)
   tcase_add_test (tc_chain, test_feedback_rtcp_race);
   tcase_add_test (tc_chain, test_receive_regular_pli);
   tcase_add_test (tc_chain, test_receive_pli_no_sender_ssrc);
+  tcase_add_test (tc_chain, test_dont_send_rtcp_while_idle);
+  tcase_add_test (tc_chain, test_send_rtcp_when_signalled);
   return s;
 }