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 4d8922a..847522c 100644
--- a/gst/rtpmanager/gstrtpsession.c
+++ b/gst/rtpmanager/gstrtpsession.c
@@ -306,6 +306,8 @@
 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,
@@ -317,7 +319,8 @@
   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 */
@@ -1243,8 +1246,7 @@
       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:
@@ -2717,3 +2719,15 @@
     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 7d76703..3b8bd77 100644
--- a/gst/rtpmanager/rtpsession.c
+++ b/gst/rtpmanager/rtpsession.c
@@ -1115,6 +1115,10 @@
     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;
+  }
 }
 
 /**
@@ -4355,6 +4359,10 @@
 
   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 5ca8a82..a378e8d 100644
--- a/gst/rtpmanager/rtpsession.h
+++ b/gst/rtpmanager/rtpsession.h
@@ -167,6 +167,16 @@
 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 @@
  * @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 @@
   RTPSessionRequestTime request_time;
   RTPSessionNotifyNACK  notify_nack;
   RTPSessionReconfigure reconfigure;
+  RTPSessionNotifyEarlyRTCP notify_early_rtcp;
 } RTPSessionCallbacks;
 
 /**
@@ -269,6 +281,7 @@
   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 9cf1ae8..8c19ce2 100644
--- a/tests/check/elements/rtpsession.c
+++ b/tests/check/elements/rtpsession.c
@@ -556,43 +556,33 @@
   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;
@@ -787,6 +777,45 @@
 
 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)
 {
@@ -802,6 +831,8 @@
   tcase_add_test (tc_chain, test_ignore_suspicious_bye);
   tcase_add_test (tc_chain, test_illegal_rtcp_fb_packet);
   tcase_add_test (tc_chain, test_feedback_rtcp_race);
+  tcase_add_test (tc_chain, test_dont_send_rtcp_while_idle);
+  tcase_add_test (tc_chain, test_send_rtcp_when_signalled);
   return s;
 }