rtpsession: don't use invalid times in RTCP timeouts
authorWim Taymans <wim.taymans@collabora.co.uk>
Tue, 23 Jul 2013 15:40:02 +0000 (17:40 +0200)
committerWim Taymans <wim.taymans@collabora.co.uk>
Tue, 23 Jul 2013 15:41:48 +0000 (17:41 +0200)
An invalid timeout can be calculated when we disabled RTCP by setting the
bandwidth to 0. Make sure all code can handle this case.

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

gst/rtpmanager/rtpsession.c

index c7ef33d350652d0a104dc3020f3428f7cd326d59..a2e90a31686fc1062e57665f2c3a3195348ebde4 100644 (file)
@@ -2067,7 +2067,8 @@ rtp_session_process_bye (RTPSession * sess, GstRTCPPacket * packet,
       /* some members went away since the previous timeout estimate.
        * Perform reverse reconsideration but only when we are not scheduling a
        * BYE ourselves. */
-      if (arrival->current_time < sess->next_rtcp_check_time) {
+      if (sess->next_rtcp_check_time != GST_CLOCK_TIME_NONE &&
+          arrival->current_time < sess->next_rtcp_check_time) {
         GstClockTime time_remaining;
 
         time_remaining = sess->next_rtcp_check_time - arrival->current_time;
@@ -2489,6 +2490,7 @@ add_bitrates (gpointer key, RTPSource * source, gdouble * bandwidth)
   *bandwidth += source->bitrate;
 }
 
+/* must be called with session lock */
 static GstClockTime
 calculate_rtcp_interval (RTPSession * sess, gboolean deterministic,
     gboolean first)
@@ -2569,7 +2571,11 @@ rtp_session_schedule_bye_locked (RTPSession * sess, const gchar * reason,
   /* reschedule transmission */
   sess->last_rtcp_send_time = current_time;
   interval = calculate_rtcp_interval (sess, FALSE, TRUE);
-  sess->next_rtcp_check_time = current_time + interval;
+
+  if (interval != GST_CLOCK_TIME_NONE)
+    sess->next_rtcp_check_time = current_time + interval;
+  else
+    sess->next_rtcp_check_time = GST_CLOCK_TIME_NONE;
 
   GST_DEBUG ("Schedule BYE for %" GST_TIME_FORMAT ", %" GST_TIME_FORMAT,
       GST_TIME_ARGS (interval), GST_TIME_ARGS (sess->next_rtcp_check_time));
@@ -2639,7 +2645,7 @@ rtp_session_next_timeout (RTPSession * sess, GstClockTime current_time)
       ", next time: %" GST_TIME_FORMAT,
       GST_TIME_ARGS (current_time), GST_TIME_ARGS (result));
 
-  if (result < current_time) {
+  if (result == GST_CLOCK_TIME_NONE || result < current_time) {
     GST_DEBUG ("take current time as base");
     /* our previous check time expired, start counting from the current time
      * again. */
@@ -2795,6 +2801,10 @@ session_cleanup (const gchar * key, RTPSource * source, ReportData * data)
   is_sender = RTP_SOURCE_IS_SENDER (source);
   is_active = RTP_SOURCE_IS_ACTIVE (source);
 
+  /* nothing to do when without RTCP */
+  if (data->interval == GST_CLOCK_TIME_NONE)
+    return;
+
   /* our own rtcp interval may have been forced low by secondary configuration,
    * while sender side may still operate with higher interval,
    * so do not just take our interval to decide on timing out sender,
@@ -2977,7 +2987,8 @@ is_rtcp_time (RTPSession * sess, GstClockTime current_time, ReportData * data)
     goto early;
 
   /* no need to check yet */
-  if (sess->next_rtcp_check_time > current_time) {
+  if (sess->next_rtcp_check_time == GST_CLOCK_TIME_NONE ||
+      sess->next_rtcp_check_time > current_time) {
     GST_DEBUG ("no check time yet, next %" GST_TIME_FORMAT " > now %"
         GST_TIME_FORMAT, GST_TIME_ARGS (sess->next_rtcp_check_time),
         GST_TIME_ARGS (current_time));
@@ -2987,16 +2998,20 @@ is_rtcp_time (RTPSession * sess, GstClockTime current_time, ReportData * data)
   /* get elapsed time since we last reported */
   elapsed = current_time - sess->last_rtcp_send_time;
 
+  new_send_time = data->interval;
   /* perform forward reconsideration */
-  new_send_time = rtp_stats_add_rtcp_jitter (&sess->stats, data->interval);
+  if (new_send_time != GST_CLOCK_TIME_NONE) {
+    new_send_time = rtp_stats_add_rtcp_jitter (&sess->stats, new_send_time);
 
-  GST_DEBUG ("forward reconsideration %" GST_TIME_FORMAT ", elapsed %"
-      GST_TIME_FORMAT, GST_TIME_ARGS (new_send_time), GST_TIME_ARGS (elapsed));
+    GST_DEBUG ("forward reconsideration %" GST_TIME_FORMAT ", elapsed %"
+        GST_TIME_FORMAT, GST_TIME_ARGS (new_send_time),
+        GST_TIME_ARGS (elapsed));
 
-  new_send_time += sess->last_rtcp_send_time;
+    new_send_time += sess->last_rtcp_send_time;
+  }
 
   /* check if reconsideration */
-  if (current_time < new_send_time) {
+  if (new_send_time == GST_CLOCK_TIME_NONE || current_time < new_send_time) {
     GST_DEBUG ("reconsider RTCP for %" GST_TIME_FORMAT,
         GST_TIME_ARGS (new_send_time));
     /* store new check time */
@@ -3010,25 +3025,29 @@ early:
 
   GST_DEBUG ("can send RTCP now, next interval %" GST_TIME_FORMAT,
       GST_TIME_ARGS (new_send_time));
-  sess->next_rtcp_check_time = current_time + new_send_time;
-
-  /* Apply the rules from RFC 4585 section 3.5.3 */
-  if (sess->stats.min_interval != 0 && !sess->first_rtcp) {
-    GstClockTimeDiff T_rr_current_interval = g_random_double_range (0.5, 1.5) *
-        sess->stats.min_interval;
-
-    /* This will caused the RTCP to be suppressed if no FB packets are added */
-    if (sess->last_rtcp_send_time + T_rr_current_interval >
-        sess->next_rtcp_check_time) {
-      GST_DEBUG ("RTCP packet could be suppressed min: %" GST_TIME_FORMAT
-          " last: %" GST_TIME_FORMAT
-          " + T_rr_current_interval: %" GST_TIME_FORMAT
-          " >  sess->next_rtcp_check_time: %" GST_TIME_FORMAT,
-          GST_TIME_ARGS (sess->stats.min_interval),
-          GST_TIME_ARGS (sess->last_rtcp_send_time),
-          GST_TIME_ARGS (T_rr_current_interval),
-          GST_TIME_ARGS (sess->next_rtcp_check_time));
-      data->may_suppress = TRUE;
+
+  sess->next_rtcp_check_time = new_send_time;
+  if (new_send_time != GST_CLOCK_TIME_NONE) {
+    sess->next_rtcp_check_time += current_time;
+
+    /* Apply the rules from RFC 4585 section 3.5.3 */
+    if (sess->stats.min_interval != 0 && !sess->first_rtcp) {
+      GstClockTimeDiff T_rr_current_interval =
+          g_random_double_range (0.5, 1.5) * sess->stats.min_interval;
+
+      /* This will caused the RTCP to be suppressed if no FB packets are added */
+      if (sess->last_rtcp_send_time + T_rr_current_interval >
+          sess->next_rtcp_check_time) {
+        GST_DEBUG ("RTCP packet could be suppressed min: %" GST_TIME_FORMAT
+            " last: %" GST_TIME_FORMAT
+            " + T_rr_current_interval: %" GST_TIME_FORMAT
+            " >  sess->next_rtcp_check_time: %" GST_TIME_FORMAT,
+            GST_TIME_ARGS (sess->stats.min_interval),
+            GST_TIME_ARGS (sess->last_rtcp_send_time),
+            GST_TIME_ARGS (T_rr_current_interval),
+            GST_TIME_ARGS (sess->next_rtcp_check_time));
+        data->may_suppress = TRUE;
+      }
     }
   }
 
@@ -3224,6 +3243,9 @@ rtp_session_request_early_rtcp (RTPSession * sess, GstClockTime current_time,
   if (GST_CLOCK_TIME_IS_VALID (sess->next_early_rtcp_time))
     goto dont_send;
 
+  if (!GST_CLOCK_TIME_IS_VALID (sess->next_rtcp_check_time))
+    goto dont_send;
+
   /* Ignore the request a scheduled packet will be in time anyway */
   if (current_time + max_delay > sess->next_rtcp_check_time)
     goto dont_send;