From a801018ef1ee9502e3ab1bda5fdbd7f4796fc85c Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Olivier=20Cr=C3=AAte?= <olivier.crete@collabora.com>
Date: Fri, 20 Nov 2020 17:32:44 -0500
Subject: [PATCH] webrtc: Make ssrc map into separate data structures

They now contain a weak reference and that could be freed later
causing strange crashes as GWeakRef are not movable.

Part-of: <https://gitlab.freedesktop.org/gstreamer/gst-plugins-bad/-/merge_requests/1766>
---
 ext/webrtc/gstwebrtcbin.c    | 17 +++--------------
 ext/webrtc/gstwebrtcstats.c  |  3 +--
 ext/webrtc/transportstream.c | 23 ++++++++++++++++++-----
 ext/webrtc/transportstream.h |  5 ++++-
 4 files changed, 26 insertions(+), 22 deletions(-)

diff --git a/ext/webrtc/gstwebrtcbin.c b/ext/webrtc/gstwebrtcbin.c
index 1ef9343bce..a12e19c700 100644
--- a/ext/webrtc/gstwebrtcbin.c
+++ b/ext/webrtc/gstwebrtcbin.c
@@ -4883,16 +4883,7 @@ _set_description_task (GstWebRTCBin * webrtc, struct set_description *sd)
 
           if (split[0] && sscanf (split[0], "%u", &ssrc) && split[1]
               && g_str_has_prefix (split[1], "cname:")) {
-            SsrcMapItem ssrc_item;
-
-            ssrc_item.media_idx = i;
-            ssrc_item.ssrc = ssrc;
-            g_array_append_val (item->remote_ssrcmap, ssrc_item);
-
-            /* Must be done aftrer the value has been appended because
-             * a weak ref cannot be moved. */
-            g_weak_ref_init (&g_array_index (item->remote_ssrcmap, SsrcMapItem,
-                    item->remote_ssrcmap->len - 1).rtpjitterbuffer, NULL);
+            g_ptr_array_add (item->remote_ssrcmap, ssrcmap_item_new (ssrc, i));
           }
           g_strfreev (split);
         }
@@ -5522,8 +5513,7 @@ on_rtpbin_pad_added (GstElement * rtpbin, GstPad * new_pad,
     media_idx = session_id;
 
     for (i = 0; i < stream->remote_ssrcmap->len; i++) {
-      SsrcMapItem *item =
-          &g_array_index (stream->remote_ssrcmap, SsrcMapItem, i);
+      SsrcMapItem *item = g_ptr_array_index (stream->remote_ssrcmap, i);
       if (item->ssrc == ssrc) {
         media_idx = item->media_idx;
         found_ssrc = TRUE;
@@ -5974,8 +5964,7 @@ on_rtpbin_new_jitterbuffer (GstElement * rtpbin, GstElement * jitterbuffer,
         WEBRTC_TRANSCEIVER (trans)->do_nack, NULL);
 
     for (i = 0; i < trans->stream->remote_ssrcmap->len; i++) {
-      SsrcMapItem *item =
-          &g_array_index (trans->stream->remote_ssrcmap, SsrcMapItem, i);
+      SsrcMapItem *item = g_ptr_array_index (trans->stream->remote_ssrcmap, i);
 
       if (item->ssrc == ssrc) {
         g_weak_ref_set (&item->rtpjitterbuffer, jitterbuffer);
diff --git a/ext/webrtc/gstwebrtcstats.c b/ext/webrtc/gstwebrtcstats.c
index 8436f321c6..498304908d 100644
--- a/ext/webrtc/gstwebrtcstats.c
+++ b/ext/webrtc/gstwebrtcstats.c
@@ -318,8 +318,7 @@ _get_stats_from_rtp_source_stats (GstWebRTCBin * webrtc,
     gst_structure_get (source_stats, "have-sr", G_TYPE_BOOLEAN, &have_sr, NULL);
 
     for (i = 0; i < stream->remote_ssrcmap->len; i++) {
-      SsrcMapItem *item =
-          &g_array_index (stream->remote_ssrcmap, SsrcMapItem, i);
+      SsrcMapItem *item = g_ptr_array_index (stream->remote_ssrcmap, i);
 
       if (item->ssrc == ssrc) {
         GObject *jb = g_weak_ref_get (&item->rtpjitterbuffer);
diff --git a/ext/webrtc/transportstream.c b/ext/webrtc/transportstream.c
index 67d4bee396..11e9cad8a8 100644
--- a/ext/webrtc/transportstream.c
+++ b/ext/webrtc/transportstream.c
@@ -197,7 +197,7 @@ transport_stream_finalize (GObject * object)
   TransportStream *stream = TRANSPORT_STREAM (object);
 
   g_array_free (stream->ptmap, TRUE);
-  g_array_free (stream->remote_ssrcmap, TRUE);
+  g_ptr_array_free (stream->remote_ssrcmap, TRUE);
 
   G_OBJECT_CLASS (parent_class)->finalize (object);
 }
@@ -289,10 +289,24 @@ clear_ptmap_item (PtMapItem * item)
   if (item->caps)
     gst_caps_unref (item->caps);
 }
+
+SsrcMapItem *
+ssrcmap_item_new (guint32 ssrc, guint media_idx)
+{
+  SsrcMapItem *ssrc_item = g_slice_new (SsrcMapItem);
+
+  ssrc_item->media_idx = media_idx;
+  ssrc_item->ssrc = ssrc;
+  g_weak_ref_init (&ssrc_item->rtpjitterbuffer, NULL);
+
+  return ssrc_item;
+}
+
 static void
-clear_ssrcmap_item (SsrcMapItem * item)
+ssrcmap_item_free (SsrcMapItem * item)
 {
   g_weak_ref_clear (&item->rtpjitterbuffer);
+  g_slice_free (SsrcMapItem, item);
 }
 
 static void
@@ -300,9 +314,8 @@ transport_stream_init (TransportStream * stream)
 {
   stream->ptmap = g_array_new (FALSE, TRUE, sizeof (PtMapItem));
   g_array_set_clear_func (stream->ptmap, (GDestroyNotify) clear_ptmap_item);
-  stream->remote_ssrcmap = g_array_new (FALSE, TRUE, sizeof (SsrcMapItem));
-  g_array_set_clear_func (stream->remote_ssrcmap,
-      (GDestroyNotify) clear_ssrcmap_item);
+  stream->remote_ssrcmap = g_ptr_array_new_with_free_func (
+      (GDestroyNotify) ssrcmap_item_free);
 }
 
 TransportStream *
diff --git a/ext/webrtc/transportstream.h b/ext/webrtc/transportstream.h
index f5cf8c225d..da05c2207d 100644
--- a/ext/webrtc/transportstream.h
+++ b/ext/webrtc/transportstream.h
@@ -44,6 +44,9 @@ typedef struct
   GWeakRef rtpjitterbuffer; /* for stats */
 } SsrcMapItem;
 
+SsrcMapItem *           ssrcmap_item_new            (guint32 ssrc,
+                                                     guint media_idx);
+
 struct _TransportStream
 {
   GstObject                 parent;
@@ -61,7 +64,7 @@ struct _TransportStream
   GstWebRTCDTLSTransport   *transport;
 
   GArray                   *ptmap;                  /* array of PtMapItem's */
-  GArray                   *remote_ssrcmap;         /* array of SsrcMapItem's */
+  GPtrArray                *remote_ssrcmap;         /* array of SsrcMapItem's */
   gboolean                  output_connected;       /* whether receive bin is connected to rtpbin */
 
   GstElement               *rtxsend;