From 6a2de911fa665151c5a45781455b8aecd80f538b Mon Sep 17 00:00:00 2001 From: George Kiagiadakis Date: Thu, 14 Nov 2013 16:19:29 +0200 Subject: [PATCH] rtpsession: fix rb blocks disappearing after the first rtcp cycle with multiple senders Previously, when the session had multiple internal sender SSRCs, it would issue SR reports with RB blocks only on the first RTCP timeout and afterwards SR reports would be sent empty. This was because the "generation" number in RTPSource would increase more than once during the same cycle and afterwards it would always be greater than the session's generation, which would cause it to be skipped from being included in RBs. This commit fixes this problem by: 1) Increasing the RTPSource generation only at the end of each cycle, which essentially fixes the problem but only when the internal senders are less than GST_RTCP_MAX_RB_COUNT. 2) Keeping for each RTPSource a set of SSRCs which stores which SSRC's SR the given RTPSource has been reported in, which also fixes the problem when the internal senders are more than GST_RTCP_MAX_RB_COUNT. This is necessary because of the fact that any RTPSource is marked as reported in itself's SR and makes it impossible to know if it has been reported in other SRs too or not, and which. --- gst/rtpmanager/rtpsession.c | 48 ++++++++++++++++++++++++++++--------- gst/rtpmanager/rtpsource.c | 5 ++++ gst/rtpmanager/rtpsource.h | 2 ++ 3 files changed, 44 insertions(+), 11 deletions(-) diff --git a/gst/rtpmanager/rtpsession.c b/gst/rtpmanager/rtpsession.c index b8d1cec4b2..31708eec88 100644 --- a/gst/rtpmanager/rtpsession.c +++ b/gst/rtpmanager/rtpsession.c @@ -2975,15 +2975,21 @@ session_report_blocks (const gchar * key, RTPSource * source, ReportData * data) return; } - /* only report about other sender */ - if (source == data->source) - goto reported; + if (g_hash_table_contains (source->reported_in_sr_of, + GUINT_TO_POINTER (data->source->ssrc))) { + GST_DEBUG ("source %08x already reported in this generation", source->ssrc); + return; + } if (gst_rtcp_packet_get_rb_count (packet) == GST_RTCP_MAX_RB_COUNT) { GST_DEBUG ("max RB count reached"); return; } + /* only report about other sender */ + if (source == data->source) + goto reported; + if (!RTP_SOURCE_IS_SENDER (source)) { GST_DEBUG ("source %08x not sender", source->ssrc); goto reported; @@ -3009,14 +3015,8 @@ session_report_blocks (const gchar * key, RTPSource * source, ReportData * data) exthighestseq, jitter, lsr, dlsr); reported: - /* source is reported, move to next generation */ - source->generation = sess->generation + 1; - - /* if we reported all sources in this generation, move to next */ - if (--data->num_to_report == 0) { - sess->generation++; - GST_DEBUG ("all reported, generation now %u", sess->generation); - } + g_hash_table_add (source->reported_in_sr_of, + GUINT_TO_POINTER (data->source->ssrc)); } /* construct FIR */ @@ -3534,6 +3534,28 @@ generate_rtcp (const gchar * key, RTPSource * source, ReportData * data) g_queue_push_tail (&data->output, output); } +static void +update_generation (const gchar * key, RTPSource * source, ReportData * data) +{ + RTPSession *sess = data->sess; + + if (g_hash_table_size (source->reported_in_sr_of) >= + sess->stats.internal_sources) { + /* source is reported, move to next generation */ + source->generation = sess->generation + 1; + g_hash_table_remove_all (source->reported_in_sr_of); + + GST_LOG ("reported source %x, new generation: %d", source->ssrc, + source->generation); + + /* if we reported all sources in this generation, move to next */ + if (--data->num_to_report == 0) { + sess->generation++; + GST_DEBUG ("all reported, generation now %u", sess->generation); + } + } +} + /** * rtp_session_on_timeout: * @sess: an #RTPSession @@ -3621,6 +3643,10 @@ rtp_session_on_timeout (RTPSession * sess, GstClockTime current_time, g_hash_table_foreach (sess->ssrcs[sess->mask_idx], (GHFunc) generate_rtcp, &data); + /* update the generation for all the sources that have been reported */ + g_hash_table_foreach (sess->ssrcs[sess->mask_idx], + (GHFunc) update_generation, &data); + /* we keep track of the last report time in order to timeout inactive * receivers or senders */ if (!data.is_early && !data.may_suppress) diff --git a/gst/rtpmanager/rtpsource.c b/gst/rtpmanager/rtpsource.c index 2c71fd327a..9a56955c6a 100644 --- a/gst/rtpmanager/rtpsource.c +++ b/gst/rtpmanager/rtpsource.c @@ -224,6 +224,7 @@ rtp_source_reset (RTPSource * src) g_free (src->bye_reason); src->bye_reason = NULL; src->sent_bye = FALSE; + g_hash_table_remove_all (src->reported_in_sr_of); src->stats.cycles = -1; src->stats.jitter = 0; @@ -261,6 +262,8 @@ rtp_source_init (RTPSource * src) src->retained_feedback = g_queue_new (); src->nacks = g_array_new (FALSE, FALSE, sizeof (guint32)); + src->reported_in_sr_of = g_hash_table_new (g_direct_hash, g_direct_equal); + rtp_source_reset (src); } @@ -304,6 +307,8 @@ rtp_source_finalize (GObject * object) if (src->rtcp_from) g_object_unref (src->rtcp_from); + g_hash_table_unref (src->reported_in_sr_of); + G_OBJECT_CLASS (rtp_source_parent_class)->finalize (object); } diff --git a/gst/rtpmanager/rtpsource.h b/gst/rtpmanager/rtpsource.h index 3a957bb655..9af7d8acea 100644 --- a/gst/rtpmanager/rtpsource.h +++ b/gst/rtpmanager/rtpsource.h @@ -136,6 +136,8 @@ struct _RTPSource { guint32 ssrc; guint16 generation; + GHashTable *reported_in_sr_of; /* set of SSRCs */ + guint probation; guint curr_probation; gboolean validated;