From e6504e3a65e2c99a7722bf3f396cc1b41917d65c Mon Sep 17 00:00:00 2001 From: =?utf8?q?Miguel=20Par=C3=ADs=20D=C3=ADaz?= Date: Fri, 10 Oct 2014 19:50:06 +0200 Subject: [PATCH] rtpsession: fix Early Feedback Transmission In early retransmission we are allowed to schedule 1 regular RTCP packet at an earlier time. When we do that, we need to set allow_early to FALSE and ignore/drop (or merge) all future requests for early transmission. We now first check if we can schedule an early RTCP and if we can, actually prepare the data for the next RTCP interval. After we send the next regular RTCP after the early RTCP, we set allow_early to TRUE again to allow more early requests. Remove the condition for the immediate feedback for now. Fixes https://bugzilla.gnome.org/show_bug.cgi?id=738319 --- gst/rtpmanager/rtpsession.c | 89 ++++++++++++++++++++++++++++----------------- gst/rtpmanager/rtpsession.h | 4 +- 2 files changed, 57 insertions(+), 36 deletions(-) diff --git a/gst/rtpmanager/rtpsession.c b/gst/rtpmanager/rtpsession.c index 4d40d90..3aa76ba 100644 --- a/gst/rtpmanager/rtpsession.c +++ b/gst/rtpmanager/rtpsession.c @@ -107,7 +107,7 @@ static void rtp_session_set_property (GObject * object, guint prop_id, static void rtp_session_get_property (GObject * object, guint prop_id, GValue * value, GParamSpec * pspec); -static void rtp_session_send_rtcp (RTPSession * sess, GstClockTime max_delay); +static gboolean rtp_session_send_rtcp (RTPSession * sess, GstClockTime max_delay); static guint rtp_session_signals[LAST_SIGNAL] = { 0 }; @@ -3712,6 +3712,11 @@ rtp_session_on_timeout (RTPSession * sess, GstClockTime current_time, 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); @@ -3757,12 +3762,15 @@ done: * @max_delay: maximum delay * * Request transmission of early RTCP + * + * Returns: %TRUE if the related RTCP can be scheduled. */ -void +gboolean rtp_session_request_early_rtcp (RTPSession * sess, GstClockTime current_time, GstClockTime max_delay) { GstClockTime T_dither_max; + gboolean ret; /* Implements the algorithm described in RFC 4585 section 3.5.2 */ @@ -3772,21 +3780,14 @@ rtp_session_request_early_rtcp (RTPSession * sess, GstClockTime current_time, /* RFC 4585 section 3.5.2 step 2 */ if (GST_CLOCK_TIME_IS_VALID (sess->next_early_rtcp_time)) { GST_LOG_OBJECT (sess, "already have next early rtcp time"); - goto dont_send; + ret = TRUE; + goto end; } if (!GST_CLOCK_TIME_IS_VALID (sess->next_rtcp_check_time)) { GST_LOG_OBJECT (sess, "no 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) { - 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)); - goto dont_send; + ret = FALSE; + goto end; } /* RFC 4585 section 3.5.2 step 2b */ @@ -3805,20 +3806,27 @@ 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"); - goto dont_send; - } - - /* RFC 4585 section 3.5.2 step 4 - * Don't send if allow_early is FALSE, but not if we are in - * immediate mode, meaning we are part of a group of at most the - * application-specific threshold. - */ - if (sess->total_sources > sess->rtcp_immediate_feedback_threshold && - sess->allow_early == FALSE) { - GST_LOG_OBJECT (sess, "can't allow early feedback"); - goto dont_send; + ret = FALSE; + goto end; + } + + /* RFC 4585 section 3.5.2 step 4a */ + if (sess->allow_early == FALSE) { + /* 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, "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 { + GST_LOG_OBJECT (sess, "can't allow early feedback"); + ret = FALSE; + } + goto end; } + /* RFC 4585 section 3.5.2 step 4b */ if (T_dither_max) { /* Schedule an early transmission later */ sess->next_early_rtcp_time = g_random_double () * T_dither_max + @@ -3828,6 +3836,11 @@ rtp_session_request_early_rtcp (RTPSession * sess, GstClockTime current_time, sess->next_early_rtcp_time = current_time; } + /* RFC 4585 section 3.5.2 step 6 */ + sess->allow_early = FALSE; + /* TODO(mparis): "R MUST recalculate tn = tp + 2*T_rr, + * and MUST set tp to the previous tn" */ + GST_LOG_OBJECT (sess, "next early RTCP time %" GST_TIME_FORMAT, GST_TIME_ARGS (sess->next_early_rtcp_time)); RTP_SESSION_UNLOCK (sess); @@ -3837,24 +3850,26 @@ rtp_session_request_early_rtcp (RTPSession * sess, GstClockTime current_time, if (sess->callbacks.reconsider) sess->callbacks.reconsider (sess, sess->reconsider_user_data); - return; + return TRUE; -dont_send: +end: RTP_SESSION_UNLOCK (sess); + + return ret; } -static void +static gboolean rtp_session_send_rtcp (RTPSession * sess, GstClockTime max_delay) { GstClockTime now; if (!sess->callbacks.send_rtcp) - return; + return FALSE; now = sess->callbacks.request_time (sess, sess->request_time_user_data); - rtp_session_request_early_rtcp (sess, now, max_delay); + return rtp_session_request_early_rtcp (sess, now, max_delay); } gboolean @@ -3863,6 +3878,11 @@ rtp_session_request_key_unit (RTPSession * sess, guint32 ssrc, { RTPSource *src; + if (!rtp_session_send_rtcp (sess, 5 * GST_SECOND)) { + GST_DEBUG ("FIR/PLI not sent"); + return FALSE; + } + RTP_SESSION_LOCK (sess); src = find_source (sess, ssrc); if (src == NULL) @@ -3880,8 +3900,6 @@ rtp_session_request_key_unit (RTPSession * sess, guint32 ssrc, } RTP_SESSION_UNLOCK (sess); - rtp_session_send_rtcp (sess, 200 * GST_MSECOND); - return TRUE; /* ERRORS */ @@ -3909,6 +3927,11 @@ rtp_session_request_nack (RTPSession * sess, guint32 ssrc, guint16 seqnum, { RTPSource *source; + if (!rtp_session_send_rtcp (sess, max_delay)) { + GST_DEBUG ("NACK not sent"); + return FALSE; + } + RTP_SESSION_LOCK (sess); source = find_source (sess, ssrc); if (source == NULL) @@ -3918,8 +3941,6 @@ rtp_session_request_nack (RTPSession * sess, guint32 ssrc, guint16 seqnum, rtp_source_register_nack (source, seqnum); RTP_SESSION_UNLOCK (sess); - rtp_session_send_rtcp (sess, max_delay); - return TRUE; /* ERRORS */ diff --git a/gst/rtpmanager/rtpsession.h b/gst/rtpmanager/rtpsession.h index b567ee4..11763ac 100644 --- a/gst/rtpmanager/rtpsession.h +++ b/gst/rtpmanager/rtpsession.h @@ -301,7 +301,7 @@ struct _RTPSessionClass { gboolean early); void (*on_feedback_rtcp) (RTPSession *sess, guint type, guint fbtype, guint sender_ssrc, guint media_ssrc, GstBuffer *fci); - void (*send_rtcp) (RTPSession *sess, GstClockTime max_delay); + gboolean (*send_rtcp) (RTPSession *sess, GstClockTime max_delay); }; GType rtp_session_get_type (void); @@ -374,7 +374,7 @@ GstFlowReturn rtp_session_on_timeout (RTPSession *sess, GstClockTi guint64 ntpnstime, GstClockTime running_time); /* request the transmittion of an early RTCP packet */ -void rtp_session_request_early_rtcp (RTPSession * sess, GstClockTime current_time, +gboolean rtp_session_request_early_rtcp (RTPSession * sess, GstClockTime current_time, GstClockTime max_delay); /* Notify session of a request for a new key unit */ -- 2.7.4