From 9794c9bfd04216e293e695112522f07c13618abe Mon Sep 17 00:00:00 2001 From: Johan Sternerup Date: Thu, 27 Oct 2022 17:30:28 +0200 Subject: [PATCH] Use the correct SSRC(s) when routing a RTCB FB FIR Previously we tried to route an incoming RTCP FB FIR to the correct ssrc using the "media source" component of the RTCP FB message. However, according to RFC5104 (section 4.3.1.2) the "media source" SHALL be set to 0. Instead the ssrc(s) in use are propagated via the FCI data. Now a specific GstForceKeyUnit event is sent for every ssrc. Part-of: --- .../gst-plugins-good/gst/rtpmanager/rtpsession.c | 44 ++++++++++++---------- .../tests/check/elements/rtpsession.c | 8 ++++ 2 files changed, 33 insertions(+), 19 deletions(-) diff --git a/subprojects/gst-plugins-good/gst/rtpmanager/rtpsession.c b/subprojects/gst-plugins-good/gst/rtpmanager/rtpsession.c index 7a20424..838c24c 100644 --- a/subprojects/gst-plugins-good/gst/rtpmanager/rtpsession.c +++ b/subprojects/gst-plugins-good/gst/rtpmanager/rtpsession.c @@ -2742,9 +2742,13 @@ rtp_session_process_app (RTPSession * sess, GstRTCPPacket * packet, static gboolean rtp_session_request_local_key_unit (RTPSession * sess, RTPSource * src, - guint32 media_ssrc, gboolean fir, GstClockTime current_time) + const guint32 * ssrcs, guint num_ssrcs, gboolean fir, + GstClockTime current_time) { guint32 round_trip = 0; + gint i; + + g_return_val_if_fail (ssrcs != NULL && num_ssrcs > 0, FALSE); rtp_source_get_last_rb (src, NULL, NULL, NULL, NULL, NULL, NULL, NULL, &round_trip); @@ -2770,14 +2774,17 @@ rtp_session_request_local_key_unit (RTPSession * sess, RTPSource * src, src->last_keyframe_request = current_time; - GST_LOG ("received %s request from %X about %X %p(%p)", fir ? "FIR" : "PLI", - rtp_source_get_ssrc (src), media_ssrc, sess->callbacks.process_rtp, - sess->callbacks.request_key_unit); + for (i = 0; i < num_ssrcs; ++i) { + GST_LOG ("received %s request from %X about %X %p(%p)", + fir ? "FIR" : "PLI", + rtp_source_get_ssrc (src), ssrcs[i], sess->callbacks.process_rtp, + sess->callbacks.request_key_unit); - RTP_SESSION_UNLOCK (sess); - sess->callbacks.request_key_unit (sess, media_ssrc, fir, - sess->request_key_unit_user_data); - RTP_SESSION_LOCK (sess); + RTP_SESSION_UNLOCK (sess); + sess->callbacks.request_key_unit (sess, ssrcs[i], fir, + sess->request_key_unit_user_data); + RTP_SESSION_LOCK (sess); + } return TRUE; } @@ -2799,19 +2806,19 @@ rtp_session_process_pli (RTPSession * sess, guint32 sender_ssrc, return; } - rtp_session_request_local_key_unit (sess, src, media_ssrc, FALSE, + rtp_session_request_local_key_unit (sess, src, &media_ssrc, 1, FALSE, current_time); } static void rtp_session_process_fir (RTPSession * sess, guint32 sender_ssrc, - guint32 media_ssrc, guint8 * fci_data, guint fci_length, - GstClockTime current_time) + guint8 * fci_data, guint fci_length, GstClockTime current_time) { RTPSource *src; guint32 ssrc; guint position = 0; - gboolean our_request = FALSE; + guint32 ssrcs[32]; + guint num_ssrcs = 0; if (!sess->callbacks.request_key_unit) return; @@ -2849,15 +2856,14 @@ rtp_session_process_fir (RTPSession * sess, guint32 sender_ssrc, if (own == NULL) continue; - if (own->internal) { - our_request = TRUE; - break; + if (own->internal && num_ssrcs < 32) { + ssrcs[num_ssrcs++] = ssrc; } } - if (!our_request) + if (num_ssrcs == 0) return; - rtp_session_request_local_key_unit (sess, src, media_ssrc, TRUE, + rtp_session_request_local_key_unit (sess, src, ssrcs, num_ssrcs, TRUE, current_time); } @@ -3022,8 +3028,8 @@ rtp_session_process_feedback (RTPSession * sess, GstRTCPPacket * packet, case GST_RTCP_PSFB_TYPE_FIR: if (src) src->stats.recv_fir_count++; - rtp_session_process_fir (sess, sender_ssrc, media_ssrc, fci_data, - fci_length, current_time); + rtp_session_process_fir (sess, sender_ssrc, fci_data, fci_length, + current_time); break; default: break; diff --git a/subprojects/gst-plugins-good/tests/check/elements/rtpsession.c b/subprojects/gst-plugins-good/tests/check/elements/rtpsession.c index ab7c5ad..ab3d4e9 100644 --- a/subprojects/gst-plugins-good/tests/check/elements/rtpsession.c +++ b/subprojects/gst-plugins-good/tests/check/elements/rtpsession.c @@ -988,6 +988,7 @@ GST_START_TEST (test_receive_regular_pli) { SessionHarness *h = session_harness_new (); GstEvent *ev; + const GstStructure *s; /* PLI packet */ guint8 rtcp_pkt[] = { @@ -1017,6 +1018,9 @@ GST_START_TEST (test_receive_regular_pli) fail_unless ((ev = gst_harness_pull_upstream_event (h->send_rtp_h)) != NULL); fail_unless_equals_int (GST_EVENT_CUSTOM_UPSTREAM, GST_EVENT_TYPE (ev)); fail_unless (gst_video_event_is_force_key_unit (ev)); + s = gst_event_get_structure (ev); + fail_unless (s); + fail_unless (G_VALUE_HOLDS_UINT (gst_structure_get_value (s, "ssrc"))); gst_event_unref (ev); session_harness_free (h); @@ -1028,6 +1032,7 @@ GST_START_TEST (test_receive_pli_no_sender_ssrc) { SessionHarness *h = session_harness_new (); GstEvent *ev; + const GstStructure *s; /* PLI packet */ guint8 rtcp_pkt[] = { @@ -1057,6 +1062,9 @@ GST_START_TEST (test_receive_pli_no_sender_ssrc) fail_unless ((ev = gst_harness_pull_upstream_event (h->send_rtp_h)) != NULL); fail_unless_equals_int (GST_EVENT_CUSTOM_UPSTREAM, GST_EVENT_TYPE (ev)); fail_unless (gst_video_event_is_force_key_unit (ev)); + s = gst_event_get_structure (ev); + fail_unless (s); + fail_unless (G_VALUE_HOLDS_UINT (gst_structure_get_value (s, "ssrc"))); gst_event_unref (ev); session_harness_free (h); -- 2.7.4