rtpsession: Handle first RTCP packet and early feedback correctly

According to RFC 4585 section 3.5.3 step 1 we are not allowed to send
an early RTCP packet for the very first one. It must be a regular one.

Also make sure to not use last_rtcp_send_time in any calculations until
we actually sent an RTCP packet already. In specific this means that we
must not use it for forward reconsideration of the current RTCP send time.
Instead we don't do any forward reconsideration for the first RTCP packet.

1 files changed, 43 insertions, 9 deletions

diff --git a/gst/rtpmanager/rtpsession.c b/gst/rtpmanager/rtpsession.c
index a93523aaf..23ab4e1c9 100644
--- a/gst/rtpmanager/rtpsession.c
+++ b/gst/rtpmanager/rtpsession.c
@@ -558,6 +558,7 @@ rtp_session_init (RTPSession * sess)
 
   sess->first_rtcp = TRUE;
   sess->next_rtcp_check_time = GST_CLOCK_TIME_NONE;
+  sess->last_rtcp_send_time = GST_CLOCK_TIME_NONE;
 
   sess->allow_early = TRUE;
   sess->next_early_rtcp_time = GST_CLOCK_TIME_NONE;
@@ -3467,7 +3468,7 @@ make_source_bye (RTPSession * sess, RTPSource * source, ReportData * data)
 static gboolean
 is_rtcp_time (RTPSession * sess, GstClockTime current_time, ReportData * data)
 {
-  GstClockTime new_send_time, elapsed;
+  GstClockTime new_send_time;
   GstClockTime interval;
   RTPSessionStats *stats;
 
@@ -3498,21 +3499,33 @@ is_rtcp_time (RTPSession * sess, GstClockTime current_time, ReportData * data)
   }
 
 early:
-  /* get elapsed time since we last reported */
-  elapsed = current_time - sess->last_rtcp_send_time;
 
   /* take interval and add jitter */
   interval = data->interval;
   if (interval != GST_CLOCK_TIME_NONE)
     interval = rtp_stats_add_rtcp_jitter (stats, interval);
 
-  /* perform forward reconsideration */
-  if (interval != GST_CLOCK_TIME_NONE) {
-    GST_DEBUG ("forward reconsideration %" GST_TIME_FORMAT ", elapsed %"
-        GST_TIME_FORMAT, GST_TIME_ARGS (interval), GST_TIME_ARGS (elapsed));
-    new_send_time = interval + sess->last_rtcp_send_time;
+  if (sess->last_rtcp_send_time != GST_CLOCK_TIME_NONE) {
+    /* perform forward reconsideration */
+    if (interval != GST_CLOCK_TIME_NONE) {
+      GstClockTime elapsed;
+
+      /* get elapsed time since we last reported */
+      elapsed = current_time - sess->last_rtcp_send_time;
+
+      GST_DEBUG ("forward reconsideration %" GST_TIME_FORMAT ", elapsed %"
+          GST_TIME_FORMAT, GST_TIME_ARGS (interval), GST_TIME_ARGS (elapsed));
+      new_send_time = interval + sess->last_rtcp_send_time;
+    } else {
+      new_send_time = sess->last_rtcp_send_time;
+    }
   } else {
-    new_send_time = sess->last_rtcp_send_time;
+    /* If this is the first RTCP packet, we can reconsider anything based
+     * on the last RTCP send time because there was none.
+     */
+    g_warn_if_fail (!data->is_early);
+    data->is_early = FALSE;
+    new_send_time = current_time;
   }
 
   if (!data->is_early) {
@@ -3832,6 +3845,27 @@ rtp_session_request_early_rtcp (RTPSession * sess, GstClockTime current_time,
     goto end;
   }
 
+  /* RFC 4585 section 3.5.3 step 1
+   * If no regular RTCP packet has been sent before, then a regular
+   * RTCP packet has to be scheduled first and FB messages might be
+   * included there
+   */
+  if (!GST_CLOCK_TIME_IS_VALID (sess->last_rtcp_send_time)) {
+    GST_LOG_OBJECT (sess, "no RTCP sent yet");
+
+    if (current_time + max_delay > sess->next_rtcp_check_time) {
+      GST_LOG_OBJECT (sess,
+          "next scheduled time is soon %" GST_TIME_FORMAT " + %" GST_TIME_FORMAT
+          " > %" GST_TIME_FORMAT, GST_TIME_ARGS (current_time),
+          GST_TIME_ARGS (max_delay),
+          GST_TIME_ARGS (sess->next_rtcp_check_time));
+      ret = TRUE;
+    } else {
+      ret = FALSE;
+    }
+    goto end;
+  }
+
   T_rr = sess->next_rtcp_check_time - sess->last_rtcp_send_time;
 
   /*  RFC 4585 section 3.5.2 step 2b */