mirrors/gstreamer
mirrors/gstreamer
1
1
Fork 0
mirror of https://gitlab.freedesktop.org/gstreamer/gstreamer.git synced 2025-01-09 08:55:33 +00:00
Code Issues 1 Projects Releases Wiki Activity

rtpmanager: Refactor RTCP packet loops to fix control flow

Browse source 
Mixing C loops with switch statements is a bad idea as break has a
different meaning in both. Breaking inside the switch statements wrongly
caused further loop iterations.

Instead use goto to get out of the loop and continue to do another loop
iteration, and never ever use break except for the end of a case.

Part-of: <https://gitlab.freedesktop.org/gstreamer/gstreamer/-/merge_requests/2336>
This commit is contained in:
Sebastian Dröge 2022-04-29 23:13:15 +03:00
parent 6619f1611f
commit 2c405da921
2 changed files with 55 additions and 32 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

46
subprojects/gst-plugins-good/gst/rtpmanager/gstrtpbin.c
View file

@ -1735,7 +1735,7 @@ gst_rtp_bin_handle_sync (GstElement * jitterbuffer, GstStructure * s,
GstRTCPPacket packet; GstRTCPPacket packet;
guint32 ssrc; guint32 ssrc;
guint64 ntpnstime, inband_ntpnstime; guint64 ntpnstime, inband_ntpnstime;
gboolean have_sr, have_sdes; gboolean have_sr;
gboolean more; gboolean more;
guint64 base_rtptime; guint64 base_rtptime;
guint64 base_time; guint64 base_time;
@ -1807,22 +1807,20 @@ gst_rtp_bin_handle_sync (GstElement * jitterbuffer, GstStructure * s,
buffer = gst_value_get_buffer (gst_structure_get_value (s, "sr-buffer")); buffer = gst_value_get_buffer (gst_structure_get_value (s, "sr-buffer"));
have_sr = FALSE; have_sr = FALSE;
have_sdes = FALSE;
gst_rtcp_buffer_map (buffer, GST_MAP_READ, &rtcp); gst_rtcp_buffer_map (buffer, GST_MAP_READ, &rtcp);
GST_RTCP_BUFFER_FOR_PACKETS (more, &rtcp, &packet) { GST_RTCP_BUFFER_FOR_PACKETS (more, &rtcp, &packet) {
if (have_sr && (cname || have_sdes))
break;
/* first packet must be SR or RR or else the validate would have failed */ /* first packet must be SR or RR or else the validate would have failed */
switch (gst_rtcp_packet_get_type (&packet)) { switch (gst_rtcp_packet_get_type (&packet)) {
case GST_RTCP_TYPE_SR: case GST_RTCP_TYPE_SR:
/* only parse first. There is only supposed to be one SR in the packet /* only parse first. There is only supposed to be one SR in the packet
* but we will deal with malformed packets gracefully */ * but we will deal with malformed packets gracefully by trying the
* next RTCP packet. */
if (have_sr) if (have_sr)
break; continue;
/* get NTP and RTP times */
/* get NTP time */
gst_rtcp_packet_sr_get_sender_info (&packet, &ssrc, &ntpnstime, NULL, gst_rtcp_packet_sr_get_sender_info (&packet, &ssrc, &ntpnstime, NULL,
NULL, NULL); NULL, NULL);
@ -1831,7 +1829,8 @@ gst_rtp_bin_handle_sync (GstElement * jitterbuffer, GstStructure * s,
(G_GINT64_CONSTANT (1) << 32)); (G_GINT64_CONSTANT (1) << 32));
GST_DEBUG_OBJECT (bin, "received sync packet from SSRC %08x", ssrc); GST_DEBUG_OBJECT (bin, "received sync packet from SSRC %08x", ssrc);
/* ignore SR that is not ours */
/* ignore SR that is not ours and check the next RTCP packet */
if (ssrc != stream->ssrc) if (ssrc != stream->ssrc)
continue; continue;
@ -1845,21 +1844,22 @@ gst_rtp_bin_handle_sync (GstElement * jitterbuffer, GstStructure * s,
(const guint8 *) cname, ntpnstime, extrtptime, base_rtptime, (const guint8 *) cname, ntpnstime, extrtptime, base_rtptime,
base_time, clock_rate, clock_base); base_time, clock_rate, clock_base);
GST_RTP_BIN_UNLOCK (bin); GST_RTP_BIN_UNLOCK (bin);
break;
goto out;
} }
break; break;
case GST_RTCP_TYPE_SDES: case GST_RTCP_TYPE_SDES:
{ {
gboolean more_items, more_entries; gboolean more_items;
/* only deal with first SDES, there is only supposed to be one SDES in /* Bail out if we have not seen an SR item yet. */
* the RTCP packet but we deal with bad packets gracefully. Also bail if (!have_sr)
* out if we have not seen an SR item yet. */ goto out;
if (have_sdes || !have_sr)
break;
GST_RTCP_SDES_FOR_ITEMS (more_items, &packet) { GST_RTCP_SDES_FOR_ITEMS (more_items, &packet) {
gboolean more_entries;
/* skip items that are not about the SSRC of the sender */ /* skip items that are not about the SSRC of the sender */
if (gst_rtcp_packet_sdes_get_ssrc (&packet) != ssrc) if (gst_rtcp_packet_sdes_get_ssrc (&packet) != ssrc)
continue; continue;
@ -1868,9 +1868,10 @@ gst_rtp_bin_handle_sync (GstElement * jitterbuffer, GstStructure * s,
GST_RTCP_SDES_FOR_ENTRIES (more_entries, &packet) { GST_RTCP_SDES_FOR_ENTRIES (more_entries, &packet) {
GstRTCPSDESType type; GstRTCPSDESType type;
guint8 len; guint8 len;
guint8 *data; const guint8 *data;
gst_rtcp_packet_sdes_get_entry (&packet, &type, &len, &data); gst_rtcp_packet_sdes_get_entry (&packet, &type, &len,
(guint8 **) & data);
if (type == GST_RTCP_SDES_CNAME) { if (type == GST_RTCP_SDES_CNAME) {
GST_RTP_BIN_LOCK (bin); GST_RTP_BIN_LOCK (bin);
@ -1879,17 +1880,22 @@ gst_rtp_bin_handle_sync (GstElement * jitterbuffer, GstStructure * s,
ntpnstime, extrtptime, base_rtptime, base_time, clock_rate, ntpnstime, extrtptime, base_rtptime, base_time, clock_rate,
clock_base); clock_base);
GST_RTP_BIN_UNLOCK (bin); GST_RTP_BIN_UNLOCK (bin);
goto out;
} }
} }
} }
have_sdes = TRUE;
break; /* only deal with first SDES, there is only supposed to be one SDES in
* the RTCP packet but we deal with bad packets gracefully. */
goto out;
} }
default: default:
/* we can ignore these packets */ /* we can ignore these packets */
break; break;
} }
} }
out:
gst_rtcp_buffer_unmap (&rtcp); gst_rtcp_buffer_unmap (&rtcp);
} }

41
subprojects/gst-plugins-good/gst/rtpmanager/gstrtpjitterbuffer.c
View file

@ -4700,7 +4700,7 @@ gst_rtp_jitter_buffer_chain_rtcp (GstPad * pad, GstObject * parent,
guint32 rtptime; guint32 rtptime;
GstRTCPBuffer rtcp = { NULL, }; GstRTCPBuffer rtcp = { NULL, };
gchar *cname = NULL; gchar *cname = NULL;
gboolean have_sr = FALSE, have_sdes = FALSE; gboolean have_sr = FALSE;
gboolean more; gboolean more;
jitterbuffer = GST_RTP_JITTER_BUFFER (parent); jitterbuffer = GST_RTP_JITTER_BUFFER (parent);
@ -4716,24 +4716,35 @@ gst_rtp_jitter_buffer_chain_rtcp (GstPad * pad, GstObject * parent,
/* first packet must be SR or RR or else the validate would have failed */ /* first packet must be SR or RR or else the validate would have failed */
switch (gst_rtcp_packet_get_type (&packet)) { switch (gst_rtcp_packet_get_type (&packet)) {
case GST_RTCP_TYPE_SR: case GST_RTCP_TYPE_SR:
/* only parse first. There is only supposed to be one SR in the packet
* but we will deal with malformed packets gracefully by trying the
* next RTCP packet */
if (have_sr)
continue;
/* get NTP and RTP times */
gst_rtcp_packet_sr_get_sender_info (&packet, &ssrc, &ntptime, &rtptime, gst_rtcp_packet_sr_get_sender_info (&packet, &ssrc, &ntptime, &rtptime,
NULL, NULL); NULL, NULL);
/* convert ntptime to nanoseconds */
ntpnstime = ntpnstime =
gst_util_uint64_scale (ntptime, GST_SECOND, gst_util_uint64_scale (ntptime, GST_SECOND,
G_GUINT64_CONSTANT (1) << 32); G_GUINT64_CONSTANT (1) << 32);
have_sr = TRUE; have_sr = TRUE;
break; break;
case GST_RTCP_TYPE_SDES: case GST_RTCP_TYPE_SDES:
{ {
gboolean more_items, more_entries; gboolean more_items;
/* only deal with first SDES, there is only supposed to be one SDES in /* Bail out if we have not seen an SR item yet. */
* the RTCP packet but we deal with bad packets gracefully. Also bail if (!have_sr)
* out if we have not seen an SR item yet. */ goto ignore_buffer;
if (have_sdes || !have_sr)
break;
GST_RTCP_SDES_FOR_ITEMS (more_items, &packet) { GST_RTCP_SDES_FOR_ITEMS (more_items, &packet) {
gboolean more_entries;
/* skip items that are not about the SSRC of the sender */ /* skip items that are not about the SSRC of the sender */
if (gst_rtcp_packet_sdes_get_ssrc (&packet) != ssrc) if (gst_rtcp_packet_sdes_get_ssrc (&packet) != ssrc)
continue; continue;
@ -4742,22 +4753,28 @@ gst_rtp_jitter_buffer_chain_rtcp (GstPad * pad, GstObject * parent,
GST_RTCP_SDES_FOR_ENTRIES (more_entries, &packet) { GST_RTCP_SDES_FOR_ENTRIES (more_entries, &packet) {
GstRTCPSDESType type; GstRTCPSDESType type;
guint8 len; guint8 len;
guint8 *data; const guint8 *data;
gst_rtcp_packet_sdes_get_entry (&packet, &type, &len, &data); gst_rtcp_packet_sdes_get_entry (&packet, &type, &len,
(guint8 **) & data);
if (type == GST_RTCP_SDES_CNAME) { if (type == GST_RTCP_SDES_CNAME) {
cname = g_strndup ((const gchar *) data, len); cname = g_strndup ((const gchar *) data, len);
goto out;
} }
} }
} }
have_sdes = TRUE;
break; /* only deal with first SDES, there is only supposed to be one SDES in
* the RTCP packet but we deal with bad packets gracefully. */
goto out;
} }
default: default:
goto ignore_buffer; /* we can ignore these packets */
break;
} }
} }
out:
gst_rtcp_buffer_unmap (&rtcp); gst_rtcp_buffer_unmap (&rtcp);
GST_DEBUG_OBJECT (jitterbuffer, "received RTCP of SSRC %08x from CNAME %s", GST_DEBUG_OBJECT (jitterbuffer, "received RTCP of SSRC %08x from CNAME %s",