rtpsession: Only schedule a timer when we actually have to send RTCP

Otherwise we will have 10s-100s of thread wakeups in feedback profiles, create
RTCP packets, etc. just to suppress them in 99% of the cases (i.e. if no
feedback is actually pending and no regular RTCP has to be sent).

This improves CPU usage and battery life quite a lot.

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

2 files changed, 113 insertions, 60 deletions

diff --git a/gst/rtpmanager/rtpsession.c b/gst/rtpmanager/rtpsession.c
index 2e6588e88..c27006d15 100644
--- a/gst/rtpmanager/rtpsession.c
+++ b/gst/rtpmanager/rtpsession.c
@@ -574,8 +574,8 @@ rtp_session_init (RTPSession * sess)
   sess->next_rtcp_check_time = GST_CLOCK_TIME_NONE;
   sess->last_rtcp_check_time = GST_CLOCK_TIME_NONE;
   sess->last_rtcp_send_time = GST_CLOCK_TIME_NONE;
+  sess->last_rtcp_interval = GST_CLOCK_TIME_NONE;
 
-  sess->allow_early = TRUE;
   sess->next_early_rtcp_time = GST_CLOCK_TIME_NONE;
   sess->rtcp_feedback_retention_window = DEFAULT_RTCP_FEEDBACK_RETENTION_WINDOW;
   sess->rtcp_immediate_feedback_threshold =
@@ -2326,15 +2326,31 @@ rtp_session_process_bye (RTPSession * sess, GstRTCPPacket * packet,
           pinfo->current_time < sess->next_rtcp_check_time) {
         GstClockTime time_remaining;
 
-        time_remaining = sess->next_rtcp_check_time - pinfo->current_time;
-        sess->next_rtcp_check_time =
-            gst_util_uint64_scale (time_remaining, members, pmembers);
+        /* Scale our next RTCP check time according to the change of numbers
+         * of members. But only if a) this is the first RTCP, or b) this is not
+         * a feedback session, or c) this is a feedback session but we schedule
+         * for every RTCP interval (aka no t-rr-interval set).
+         *
+         * FIXME: a) and b) are not great as we will possibly go below Tmin
+         * for non-feedback profiles and in case of a) below
+         * Tmin/t-rr-interval in any case.
+         */
+        if (sess->last_rtcp_send_time == GST_CLOCK_TIME_NONE ||
+            !(sess->rtp_profile == GST_RTP_PROFILE_AVPF
+                || sess->rtp_profile == GST_RTP_PROFILE_SAVPF) ||
+            sess->next_rtcp_check_time - sess->last_rtcp_send_time ==
+            sess->last_rtcp_interval) {
+          time_remaining = sess->next_rtcp_check_time - pinfo->current_time;
+          sess->next_rtcp_check_time =
+              gst_util_uint64_scale (time_remaining, members, pmembers);
+          sess->next_rtcp_check_time += pinfo->current_time;
+        }
+        sess->last_rtcp_interval =
+            gst_util_uint64_scale (sess->last_rtcp_interval, members, pmembers);
 
         GST_DEBUG ("reverse reconsideration %" GST_TIME_FORMAT,
             GST_TIME_ARGS (sess->next_rtcp_check_time));
 
-        sess->next_rtcp_check_time += pinfo->current_time;
-
         /* mark pending reconsider. We only want to signal the reconsideration
          * once after we handled all the source in the bye packet */
         reconsider = TRUE;
@@ -2905,7 +2921,6 @@ rtp_session_schedule_bye_locked (RTPSession * sess, GstClockTime current_time)
   INIT_AVG (sess->bye_stats.avg_rtcp_packet_size, 100);
   sess->bye_stats.bye_members = 1;
   sess->first_rtcp = TRUE;
-  sess->allow_early = TRUE;
 
   /* reschedule transmission */
   sess->last_rtcp_send_time = current_time;
@@ -2916,6 +2931,7 @@ rtp_session_schedule_bye_locked (RTPSession * sess, GstClockTime current_time)
     sess->next_rtcp_check_time = current_time + interval;
   else
     sess->next_rtcp_check_time = GST_CLOCK_TIME_NONE;
+  sess->last_rtcp_interval = interval;
 
   GST_DEBUG ("Schedule BYE for %" GST_TIME_FORMAT ", %" GST_TIME_FORMAT,
       GST_TIME_ARGS (interval), GST_TIME_ARGS (sess->next_rtcp_check_time));
@@ -2996,16 +3012,36 @@ rtp_session_next_timeout (RTPSession * sess, GstClockTime current_time)
       GST_DEBUG ("reconsider BYE, more than 50 sources");
       /* reconsider BYE if members >= 50 */
       interval = calculate_rtcp_interval (sess, FALSE, TRUE);
+      sess->last_rtcp_interval = interval;
     }
   } else {
     if (sess->first_rtcp) {
       GST_DEBUG ("first RTCP packet");
       /* we are called for the first time */
       interval = calculate_rtcp_interval (sess, FALSE, TRUE);
+      sess->last_rtcp_interval = interval;
     } else if (sess->next_rtcp_check_time < current_time) {
       GST_DEBUG ("old check time expired, getting new timeout");
       /* get a new timeout when we need to */
       interval = calculate_rtcp_interval (sess, FALSE, FALSE);
+      sess->last_rtcp_interval = interval;
+
+      if ((sess->rtp_profile == GST_RTP_PROFILE_AVPF
+              || sess->rtp_profile == GST_RTP_PROFILE_SAVPF)
+          && interval != GST_CLOCK_TIME_NONE) {
+        /* Apply the rules from RFC 4585 section 3.5.3 */
+        if (sess->stats.min_interval != 0) {
+          GstClockTime T_rr_current_interval = g_random_double_range (0.5,
+              1.5) * sess->stats.min_interval * GST_SECOND;
+
+          if (T_rr_current_interval > interval) {
+            GST_DEBUG ("Adjusting interval for t-rr-interval: %" GST_TIME_FORMAT
+                " > %" GST_TIME_FORMAT, GST_TIME_ARGS (T_rr_current_interval),
+                GST_TIME_ARGS (interval));
+            interval = T_rr_current_interval;
+          }
+        }
+      }
     }
   }
 
@@ -3462,7 +3498,7 @@ session_sdes (RTPSession * sess, ReportData * data)
     type = gst_rtcp_sdes_name_to_type (field);
 
     /* Early packets are minimal and only include the CNAME */
-    if (data->may_suppress && type != GST_RTCP_SDES_CNAME)
+    if (data->is_early && type != GST_RTCP_SDES_CNAME)
       continue;
 
     if (type > GST_RTCP_SDES_END && type < GST_RTCP_SDES_PRIV) {
@@ -3579,36 +3615,33 @@ is_rtcp_time (RTPSession * sess, GstClockTime current_time, ReportData * data)
           GST_TIME_ARGS (new_send_time));
       /* store new check time */
       sess->next_rtcp_check_time = new_send_time;
+      sess->last_rtcp_interval = interval;
       return FALSE;
     }
-    sess->next_rtcp_check_time = current_time + interval;
-  }
 
-  if ((sess->rtp_profile == GST_RTP_PROFILE_AVPF
-          || sess->rtp_profile == GST_RTP_PROFILE_SAVPF)
-      && interval != GST_CLOCK_TIME_NONE) {
-    /* Apply the rules from RFC 4585 section 3.5.3 */
-    if (stats->min_interval != 0 && !sess->first_rtcp) {
-      GstClockTime T_rr_current_interval =
-          g_random_double_range (0.5, 1.5) * stats->min_interval * GST_SECOND;
-
-      /* This will caused the RTCP to be suppressed if no FB packets are added */
-      if (sess->last_rtcp_send_time + T_rr_current_interval > new_send_time) {
-        GST_DEBUG ("RTCP packet could be suppressed min: %" GST_TIME_FORMAT
-            " last: %" GST_TIME_FORMAT
-            " + T_rr_current_interval: %" GST_TIME_FORMAT
-            " >  new_send_time: %" GST_TIME_FORMAT,
-            GST_TIME_ARGS (stats->min_interval),
-            GST_TIME_ARGS (sess->last_rtcp_send_time),
-            GST_TIME_ARGS (T_rr_current_interval),
-            GST_TIME_ARGS (new_send_time));
-        data->may_suppress = TRUE;
+    sess->last_rtcp_interval = interval;
+    if ((sess->rtp_profile == GST_RTP_PROFILE_AVPF
+            || sess->rtp_profile == GST_RTP_PROFILE_SAVPF)
+        && interval != GST_CLOCK_TIME_NONE) {
+      /* Apply the rules from RFC 4585 section 3.5.3 */
+      if (stats->min_interval != 0 && !sess->first_rtcp) {
+        GstClockTime T_rr_current_interval =
+            g_random_double_range (0.5, 1.5) * stats->min_interval * GST_SECOND;
+
+        if (T_rr_current_interval > interval) {
+          GST_DEBUG ("Adjusting interval for t-rr-interval: %" GST_TIME_FORMAT
+              " > %" GST_TIME_FORMAT, GST_TIME_ARGS (T_rr_current_interval),
+              GST_TIME_ARGS (interval));
+          interval = T_rr_current_interval;
+        }
       }
     }
+    sess->next_rtcp_check_time = current_time + interval;
   }
 
-  GST_DEBUG ("can send RTCP now, next interval %" GST_TIME_FORMAT,
-      GST_TIME_ARGS (new_send_time));
+
+  GST_DEBUG ("can send RTCP now, next %" GST_TIME_FORMAT,
+      GST_TIME_ARGS (sess->next_rtcp_check_time));
 
   return TRUE;
 }
@@ -3660,7 +3693,7 @@ generate_rtcp (const gchar * key, RTPSource * source, ReportData * data)
     /* send BYE */
     make_source_bye (sess, source, data);
     is_bye = TRUE;
-  } else if (!data->may_suppress) {
+  } else if (!data->is_early) {
     /* loop over all known sources and add report blocks. If we are early, we
      * just make a minimal RTCP packet and skip this step */
     g_hash_table_foreach (sess->ssrcs[sess->mask_idx],
@@ -3738,7 +3771,6 @@ rtp_session_on_timeout (RTPSession * sess, GstClockTime current_time,
   ReportData data = { GST_RTCP_BUFFER_INIT };
   GHashTable *table_copy;
   ReportOutput *output;
-  gboolean must_not_suppress;
 
   g_return_val_if_fail (RTP_IS_SESSION (sess), GST_FLOW_ERROR);
 
@@ -3797,9 +3829,6 @@ rtp_session_on_timeout (RTPSession * sess, GstClockTime current_time,
   if (!is_rtcp_time (sess, current_time, &data))
     goto done;
 
-  /* We need to send a full RTCP packet */
-  must_not_suppress = !data.is_early && !data.may_suppress;
-
   GST_DEBUG
       ("doing RTCP generation %u for %u sources, early %d, may suppress %d",
       sess->generation, data.num_to_report, data.is_early, data.may_suppress);
@@ -3814,19 +3843,24 @@ rtp_session_on_timeout (RTPSession * sess, GstClockTime current_time,
 
   /* we keep track of the last report time in order to timeout inactive
    * receivers or senders */
-  if (must_not_suppress)
+  if (!data.is_early) {
+    GST_DEBUG ("Time since last regular RTCP: %" GST_TIME_FORMAT " - %"
+        GST_TIME_FORMAT " = %" GST_TIME_FORMAT,
+        GST_TIME_ARGS (data.current_time),
+        GST_TIME_ARGS (sess->last_rtcp_send_time),
+        GST_TIME_ARGS (data.current_time - sess->last_rtcp_send_time));
     sess->last_rtcp_send_time = data.current_time;
+  }
 
+  GST_DEBUG ("Time since last RTCP: %" GST_TIME_FORMAT " - %" GST_TIME_FORMAT
+      " = %" GST_TIME_FORMAT, GST_TIME_ARGS (data.current_time),
+      GST_TIME_ARGS (sess->last_rtcp_send_time),
+      GST_TIME_ARGS (data.current_time - sess->last_rtcp_check_time));
   sess->last_rtcp_check_time = data.current_time;
   sess->first_rtcp = FALSE;
   sess->next_early_rtcp_time = GST_CLOCK_TIME_NONE;
   sess->scheduled_bye = FALSE;
 
-  /*  RFC 4585 section 3.5.2 step 6 */
-  if (!data.is_early) {
-    sess->allow_early = TRUE;
-  }
-
 done:
   RTP_SESSION_UNLOCK (sess);
 
@@ -3854,8 +3888,8 @@ done:
       sess->stats.nacks_sent += data.nacked_seqnums;
     } else {
       GST_DEBUG ("freeing packet callback: %p"
-          " do_not_suppress: %d may_suppress: %d",
-          sess->callbacks.send_rtcp, do_not_suppress, data.may_suppress);
+          " do_not_suppress: %d may_suppress: %d", sess->callbacks.send_rtcp,
+          do_not_suppress, data.may_suppress);
       sess->stats.nacks_dropped += data.nacked_seqnums;
       gst_buffer_unref (buffer);
     }
@@ -3879,8 +3913,9 @@ gboolean
 rtp_session_request_early_rtcp (RTPSession * sess, GstClockTime current_time,
     GstClockTime max_delay)
 {
-  GstClockTime T_dither_max, T_rr;
+  GstClockTime T_dither_max, T_rr, offset = 0;
   gboolean ret;
+  gboolean allow_early;
 
   /* Implements the algorithm described in RFC 4585 section 3.5.2 */
 
@@ -3930,7 +3965,7 @@ rtp_session_request_early_rtcp (RTPSession * sess, GstClockTime current_time,
     goto end;
   }
 
-  T_rr = sess->next_rtcp_check_time - sess->last_rtcp_check_time;
+  T_rr = sess->last_rtcp_interval;
 
   /*  RFC 4585 section 3.5.2 step 2b */
   /* If the total sources is <=2, then there is only us and one peer */
@@ -3948,16 +3983,41 @@ rtp_session_request_early_rtcp (RTPSession * sess, GstClockTime current_time,
   /*  RFC 4585 section 3.5.2 step 3 */
   if (current_time + T_dither_max > sess->next_rtcp_check_time) {
     GST_LOG_OBJECT (sess,
-        "don't send because of dither, next scheduled time is soon %"
+        "don't send because of dither, next scheduled time is too soon %"
         GST_TIME_FORMAT " + %" GST_TIME_FORMAT " > %" GST_TIME_FORMAT,
         GST_TIME_ARGS (current_time), GST_TIME_ARGS (T_dither_max),
         GST_TIME_ARGS (sess->next_rtcp_check_time));
-    ret = TRUE;
+    ret = T_dither_max <= max_delay;
     goto end;
   }
 
-  /*  RFC 4585 section 3.5.2 step 4a */
-  if (!sess->allow_early) {
+  /*  RFC 4585 section 3.5.2 step 4a and
+   *  RFC 4585 section 3.5.2 step 6 */
+  allow_early = FALSE;
+  if (sess->last_rtcp_check_time == sess->last_rtcp_send_time) {
+    /* Last time we sent a full RTCP packet, we can now immediately
+     * send an early one as allow_early was reset to TRUE */
+    allow_early = TRUE;
+  } else if (sess->last_rtcp_check_time + T_rr <= current_time + max_delay) {
+    /* Last packet we sent was an early RTCP packet and more than
+     * T_rr has passed since then, meaning we would have suppressed
+     * a regular RTCP packet already and reset allow_early to TRUE */
+    allow_early = TRUE;
+
+    /* We have to offset a bit as T_rr has not passed yet, but will before
+     * max_delay */
+    if (sess->last_rtcp_check_time + T_rr > current_time)
+      offset = (sess->last_rtcp_check_time + T_rr) - current_time;
+  } else {
+    GST_DEBUG_OBJECT (sess,
+        "can't allow early RTCP yet: last regular %" GST_TIME_FORMAT ", %"
+        GST_TIME_FORMAT " + %" GST_TIME_FORMAT " > %" GST_TIME_FORMAT " + %"
+        GST_TIME_FORMAT, GST_TIME_ARGS (sess->last_rtcp_send_time),
+        GST_TIME_ARGS (sess->last_rtcp_check_time), GST_TIME_ARGS (T_rr),
+        GST_TIME_ARGS (current_time), GST_TIME_ARGS (max_delay));
+  }
+
+  if (!allow_early) {
     /* Ignore the request a scheduled packet will be in time anyway */
     if (current_time + max_delay > sess->next_rtcp_check_time) {
       GST_LOG_OBJECT (sess,
@@ -3981,20 +4041,12 @@ rtp_session_request_early_rtcp (RTPSession * sess, GstClockTime current_time,
   if (T_dither_max) {
     /* Schedule an early transmission later */
     sess->next_early_rtcp_time = g_random_double () * T_dither_max +
-        current_time;
+        current_time + offset;
   } else {
     /* If no dithering, schedule it for NOW */
-    sess->next_early_rtcp_time = current_time;
+    sess->next_early_rtcp_time = current_time + offset;
   }
 
-  /*  RFC 4585 section 3.5.2 step 6 */
-  sess->allow_early = FALSE;
-  /* Delay next regular RTCP packet to not exceed the short-term
-   * RTCP bandwidth when using early feedback as compared to
-   * without */
-  sess->next_rtcp_check_time = sess->last_rtcp_check_time + 2 * T_rr;
-  sess->last_rtcp_check_time += T_rr;
-
   GST_LOG_OBJECT (sess, "next early RTCP time %" GST_TIME_FORMAT
       ", next regular RTCP time %" GST_TIME_FORMAT,
       GST_TIME_ARGS (sess->next_early_rtcp_time),
diff --git a/gst/rtpmanager/rtpsession.h b/gst/rtpmanager/rtpsession.h
index be0d9097e..7f7fe6992 100644
--- a/gst/rtpmanager/rtpsession.h
+++ b/gst/rtpmanager/rtpsession.h
@@ -242,6 +242,7 @@ struct _RTPSession {
   GstClockTime  next_rtcp_check_time; /* tn */
   GstClockTime  last_rtcp_check_time; /* tp */
   GstClockTime  last_rtcp_send_time;  /* t_rr_last */
+  GstClockTime  last_rtcp_interval;   /* T_rr */
   GstClockTime  start_time;
   gboolean      first_rtcp;
   gboolean      allow_early;