From dece8413ef338c257283267482517b723878e5a4 Mon Sep 17 00:00:00 2001 From: Wim Taymans Date: Tue, 23 Jul 2013 17:40:02 +0200 Subject: [PATCH] rtpsession: don't use invalid times in RTCP timeouts 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 | 74 ++++++++++++++++++++++++------------- 1 file changed, 48 insertions(+), 26 deletions(-) diff --git a/gst/rtpmanager/rtpsession.c b/gst/rtpmanager/rtpsession.c index c7ef33d350..a2e90a3168 100644 --- a/gst/rtpmanager/rtpsession.c +++ b/gst/rtpmanager/rtpsession.c @@ -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; + sess->next_rtcp_check_time = new_send_time; + if (new_send_time != GST_CLOCK_TIME_NONE) { + sess->next_rtcp_check_time += current_time; - /* 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; + /* 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;