webrtcbin: reuese the same fec/rtx/red payload types for the same media payload
authorMatthew Waters <matthew@centricular.com>
Tue, 24 May 2022 04:36:36 +0000 (14:36 +1000)
committerGStreamer Marge Bot <gitlab-merge-bot@gstreamer-foundation.org>
Tue, 24 May 2022 10:21:11 +0000 (10:21 +0000)
WHen bundling, if multiple medias are used with the same media payload, then
each of the fec/rtx/red additions would add a distinct payload.  This could
very easily overflow the available payload space.

Instead, track the relationship between the media payload value and
the relevant fec/rtx/red payload values and reuse them whenever
necessary, even when bundling.

e.g.

...
a=group:BUNDLE video0 video1
m=video 9 UDP/SAVPF 96 97
a=mid:video0
a=rtpmap:96 VP8/90000
a=rtpmap:97 rtx/90000
a=fmtp:97 apt=96
...
m=video 9 UDP/SAVPF 96 97
a=mid:video1
a=rtpmap:96 VP8/90000
a=rtpmap:97 rtx/90000
a=fmtp:97 apt=96
...

Part-of: <https://gitlab.freedesktop.org/gstreamer/gstreamer/-/merge_requests/2474>

subprojects/gst-plugins-bad/ext/webrtc/gstwebrtcbin.c
subprojects/gst-plugins-bad/tests/check/elements/webrtcbin.c

index a1c7fa0..fd8318a 100644 (file)
@@ -2654,38 +2654,93 @@ _get_or_create_transport_stream (GstWebRTCBin * webrtc, guint session_id,
     return _get_or_create_rtp_transport_channel (webrtc, session_id);
 }
 
-static guint
-g_array_find_uint (GArray * array, guint val)
+struct media_payload_map_item
+{
+  guint media_pt;
+  guint red_pt;
+  guint ulpfec_pt;
+  guint rtx_pt;
+  guint red_rtx_pt;
+};
+
+static void
+media_payload_map_item_init (struct media_payload_map_item *item,
+    guint media_pt)
+{
+  item->media_pt = media_pt;
+  item->red_pt = G_MAXUINT;
+  item->rtx_pt = G_MAXUINT;
+  item->ulpfec_pt = G_MAXUINT;
+  item->red_rtx_pt = G_MAXUINT;
+}
+
+static struct media_payload_map_item *
+find_payload_map_for_media_pt (GArray * media_mapping, guint media_pt)
 {
   guint i;
 
-  for (i = 0; i < array->len; i++) {
-    if (g_array_index (array, guint, i) == val)
-      return i;
+  for (i = 0; i < media_mapping->len; i++) {
+    struct media_payload_map_item *item;
+
+    item = &g_array_index (media_mapping, struct media_payload_map_item, i);
+
+    if (item->media_pt == media_pt)
+      return item;
   }
 
-  return G_MAXUINT;
+  return NULL;
+}
+
+static struct media_payload_map_item *
+find_or_create_payload_map_for_media_pt (GArray * media_mapping, guint media_pt)
+{
+  struct media_payload_map_item new_item;
+  struct media_payload_map_item *item;
+
+  if ((item = find_payload_map_for_media_pt (media_mapping, media_pt)))
+    return item;
+
+  media_payload_map_item_init (&new_item, media_pt);
+  g_array_append_val (media_mapping, new_item);
+  return &g_array_index (media_mapping, struct media_payload_map_item,
+      media_mapping->len - 1);
 }
 
 static gboolean
-_pick_available_pt (GArray * reserved_pts, guint * i)
+_pick_available_pt (GArray * media_mapping, guint * ret)
 {
-  gboolean ret = FALSE;
+  int i;
 
-  for (*i = 96; *i <= 127; (*i)++) {
-    if (g_array_find_uint (reserved_pts, *i) == G_MAXUINT) {
-      g_array_append_val (reserved_pts, *i);
-      ret = TRUE;
-      break;
+  for (i = 96; i <= 127; i++) {
+    int j;
+
+    for (j = 0; j < media_mapping->len; j++) {
+      struct media_payload_map_item *item;
+
+      item = &g_array_index (media_mapping, struct media_payload_map_item, j);
+      if (item->media_pt == i)
+        continue;
+      if (item->red_pt == i)
+        continue;
+      if (item->rtx_pt == i)
+        continue;
+      if (item->ulpfec_pt == i)
+        continue;
+      if (item->red_rtx_pt == i)
+        continue;
+
+      *ret = i;
+      return TRUE;
     }
   }
 
-  return ret;
+  *ret = G_MAXUINT;
+  return FALSE;
 }
 
 static gboolean
 _pick_fec_payload_types (GstWebRTCBin * webrtc, WebRTCTransceiver * trans,
-    GArray * reserved_pts, gint clockrate, gint * rtx_target_pt,
+    GArray * media_mapping, gint clockrate, gint media_pt, gint * rtx_target_pt,
     GstSDPMedia * media)
 {
   gboolean ret = TRUE;
@@ -2694,30 +2749,35 @@ _pick_fec_payload_types (GstWebRTCBin * webrtc, WebRTCTransceiver * trans,
     goto done;
 
   if (trans->fec_type == GST_WEBRTC_FEC_TYPE_ULP_RED && clockrate != -1) {
-    guint pt;
+    struct media_payload_map_item *item;
     gchar *str;
 
-    if (!(ret = _pick_available_pt (reserved_pts, &pt)))
-      goto done;
+    item = find_or_create_payload_map_for_media_pt (media_mapping, media_pt);
+    if (item->red_pt == G_MAXUINT) {
+      if (!(ret = _pick_available_pt (media_mapping, &item->red_pt)))
+        goto done;
+    }
 
     /* https://tools.ietf.org/html/rfc5109#section-14.1 */
 
-    str = g_strdup_printf ("%u", pt);
+    str = g_strdup_printf ("%u", item->red_pt);
     gst_sdp_media_add_format (media, str);
     g_free (str);
-    str = g_strdup_printf ("%u red/%d", pt, clockrate);
+    str = g_strdup_printf ("%u red/%d", item->red_pt, clockrate);
     gst_sdp_media_add_attribute (media, "rtpmap", str);
     g_free (str);
 
-    *rtx_target_pt = pt;
+    *rtx_target_pt = item->red_pt;
 
-    if (!(ret = _pick_available_pt (reserved_pts, &pt)))
-      goto done;
+    if (item->ulpfec_pt == G_MAXUINT) {
+      if (!(ret = _pick_available_pt (media_mapping, &item->ulpfec_pt)))
+        goto done;
+    }
 
-    str = g_strdup_printf ("%u", pt);
+    str = g_strdup_printf ("%u", item->ulpfec_pt);
     gst_sdp_media_add_format (media, str);
     g_free (str);
-    str = g_strdup_printf ("%u ulpfec/%d", pt, clockrate);
+    str = g_strdup_printf ("%u ulpfec/%d", item->ulpfec_pt, clockrate);
     gst_sdp_media_add_attribute (media, "rtpmap", str);
     g_free (str);
   }
@@ -2726,10 +2786,37 @@ done:
   return ret;
 }
 
+static void
+add_rtx_to_media (WebRTCTransceiver * trans, gint clockrate, gint rtx_pt,
+    gint rtx_target_pt, guint target_ssrc, GstSDPMedia * media)
+{
+  char *str;
+
+  /* https://tools.ietf.org/html/rfc4588#section-8.6 */
+  if (target_ssrc != -1) {
+    str = g_strdup_printf ("%u", target_ssrc);
+    gst_structure_set (trans->local_rtx_ssrc_map, str, G_TYPE_UINT,
+        g_random_int (), NULL);
+    g_free (str);
+  }
+
+  str = g_strdup_printf ("%u", rtx_pt);
+  gst_sdp_media_add_format (media, str);
+  g_free (str);
+
+  str = g_strdup_printf ("%u rtx/%d", rtx_pt, clockrate);
+  gst_sdp_media_add_attribute (media, "rtpmap", str);
+  g_free (str);
+
+  str = g_strdup_printf ("%u apt=%d", rtx_pt, rtx_target_pt);
+  gst_sdp_media_add_attribute (media, "fmtp", str);
+  g_free (str);
+}
+
 static gboolean
 _pick_rtx_payload_types (GstWebRTCBin * webrtc, WebRTCTransceiver * trans,
-    GArray * reserved_pts, gint clockrate, gint target_pt, guint target_ssrc,
-    GstSDPMedia * media)
+    GArray * media_mapping, gint clockrate, gint media_pt, gint target_pt,
+    guint target_ssrc, GstSDPMedia * media)
 {
   gboolean ret = TRUE;
 
@@ -2740,32 +2827,26 @@ _pick_rtx_payload_types (GstWebRTCBin * webrtc, WebRTCTransceiver * trans,
       gst_structure_new_empty ("application/x-rtp-ssrc-map");
 
   if (trans->do_nack) {
-    guint pt;
-    gchar *str;
-
-    if (!(ret = _pick_available_pt (reserved_pts, &pt)))
-      goto done;
+    struct media_payload_map_item *item;
 
-    /* https://tools.ietf.org/html/rfc4588#section-8.6 */
-
-    if (target_ssrc != -1) {
-      str = g_strdup_printf ("%u", target_ssrc);
-      gst_structure_set (trans->local_rtx_ssrc_map, str, G_TYPE_UINT,
-          g_random_int (), NULL);
-      g_free (str);
+    item = find_or_create_payload_map_for_media_pt (media_mapping, media_pt);
+    if (item->rtx_pt == G_MAXUINT) {
+      if (!(ret = _pick_available_pt (media_mapping, &item->rtx_pt)))
+        goto done;
     }
 
-    str = g_strdup_printf ("%u", pt);
-    gst_sdp_media_add_format (media, str);
-    g_free (str);
-
-    str = g_strdup_printf ("%u rtx/%d", pt, clockrate);
-    gst_sdp_media_add_attribute (media, "rtpmap", str);
-    g_free (str);
+    add_rtx_to_media (trans, clockrate, item->rtx_pt, media_pt, target_ssrc,
+        media);
 
-    str = g_strdup_printf ("%u apt=%d", pt, target_pt);
-    gst_sdp_media_add_attribute (media, "fmtp", str);
-    g_free (str);
+    if (item->red_pt != G_MAXUINT) {
+      /* Workaround chrome bug: https://bugs.chromium.org/p/webrtc/issues/detail?id=6196 */
+      if (item->red_rtx_pt == G_MAXUINT) {
+        if (!(ret = _pick_available_pt (media_mapping, &item->red_rtx_pt)))
+          goto done;
+      }
+      add_rtx_to_media (trans, clockrate, item->red_rtx_pt, item->red_pt,
+          target_ssrc, media);
+    }
   }
 
 done:
@@ -3096,7 +3177,7 @@ static gboolean
 sdp_media_from_transceiver (GstWebRTCBin * webrtc, GstSDPMedia * media,
     const GstSDPMedia * last_media, GstWebRTCRTPTransceiver * trans,
     guint media_idx, GString * bundled_mids, guint bundle_idx,
-    gchar * bundle_ufrag, gchar * bundle_pwd, GArray * reserved_pts,
+    gchar * bundle_ufrag, gchar * bundle_pwd, GArray * media_mapping,
     GHashTable * all_mids, GError ** error)
 {
   /* TODO:
@@ -3270,14 +3351,14 @@ sdp_media_from_transceiver (GstWebRTCBin * webrtc, GstSDPMedia * media,
     const GstStructure *s = gst_caps_get_structure (caps, 0);
     gint clockrate = -1;
     gint rtx_target_pt;
-    gint original_rtx_target_pt;        /* Workaround chrome bug: https://bugs.chromium.org/p/webrtc/issues/detail?id=6196 */
     guint rtx_target_ssrc = -1;
+    gint media_pt;
 
-    if (gst_structure_get_int (s, "payload", &rtx_target_pt) &&
+    if (gst_structure_get_int (s, "payload", &media_pt) &&
         webrtc->bundle_policy == GST_WEBRTC_BUNDLE_POLICY_NONE)
-      g_array_append_val (reserved_pts, rtx_target_pt);
+      find_or_create_payload_map_for_media_pt (media_mapping, media_pt);
 
-    original_rtx_target_pt = rtx_target_pt;
+    rtx_target_pt = media_pt;
 
     if (!gst_structure_get_int (s, "clock-rate", &clockrate))
       GST_WARNING_OBJECT (webrtc,
@@ -3289,13 +3370,10 @@ sdp_media_from_transceiver (GstWebRTCBin * webrtc, GstSDPMedia * media,
       }
     }
 
-    _pick_fec_payload_types (webrtc, WEBRTC_TRANSCEIVER (trans), reserved_pts,
-        clockrate, &rtx_target_pt, media);
-    _pick_rtx_payload_types (webrtc, WEBRTC_TRANSCEIVER (trans), reserved_pts,
-        clockrate, rtx_target_pt, rtx_target_ssrc, media);
-    if (original_rtx_target_pt != rtx_target_pt)
-      _pick_rtx_payload_types (webrtc, WEBRTC_TRANSCEIVER (trans), reserved_pts,
-          clockrate, original_rtx_target_pt, rtx_target_ssrc, media);
+    _pick_fec_payload_types (webrtc, WEBRTC_TRANSCEIVER (trans), media_mapping,
+        clockrate, media_pt, &rtx_target_pt, media);
+    _pick_rtx_payload_types (webrtc, WEBRTC_TRANSCEIVER (trans), media_mapping,
+        clockrate, media_pt, rtx_target_pt, rtx_target_ssrc, media);
   }
 
   _media_add_ssrcs (media, caps, webrtc, WEBRTC_TRANSCEIVER (trans));
@@ -3374,30 +3452,31 @@ sdp_media_from_transceiver (GstWebRTCBin * webrtc, GstSDPMedia * media,
 }
 
 static void
-gather_pad_pt (GstWebRTCBinPad * pad, GArray * reserved_pts)
+gather_pad_pt (GstWebRTCBinPad * pad, GArray * media_mapping)
 {
   if (pad->received_caps) {
     GstStructure *s = gst_caps_get_structure (pad->received_caps, 0);
     gint pt;
 
     if (gst_structure_get_int (s, "payload", &pt)) {
-      GST_TRACE_OBJECT (pad, "have reserved pt %u from received caps", pt);
-      g_array_append_val (reserved_pts, pt);
+      GST_TRACE_OBJECT (pad, "have media pt %u from received caps", pt);
+      find_or_create_payload_map_for_media_pt (media_mapping, pt);
     }
   }
 }
 
 static GArray *
-gather_reserved_pts (GstWebRTCBin * webrtc)
+gather_media_mapping (GstWebRTCBin * webrtc)
 {
   GstElement *element = GST_ELEMENT (webrtc);
-  GArray *reserved_pts = g_array_new (FALSE, FALSE, sizeof (guint));
+  GArray *media_mapping =
+      g_array_new (FALSE, FALSE, sizeof (struct media_payload_map_item));
   guint i;
 
   GST_OBJECT_LOCK (webrtc);
-  g_list_foreach (element->sinkpads, (GFunc) gather_pad_pt, reserved_pts);
+  g_list_foreach (element->sinkpads, (GFunc) gather_pad_pt, media_mapping);
   g_list_foreach (webrtc->priv->pending_pads, (GFunc) gather_pad_pt,
-      reserved_pts);
+      media_mapping);
 
   for (i = 0; i < webrtc->priv->transceivers->len; i++) {
     GstWebRTCRTPTransceiver *trans;
@@ -3412,9 +3491,9 @@ gather_reserved_pts (GstWebRTCBin * webrtc)
       for (j = 0; j < n; j++) {
         GstStructure *s = gst_caps_get_structure (trans->codec_preferences, j);
         if (gst_structure_get_int (s, "payload", &pt)) {
-          GST_TRACE_OBJECT (trans, "have reserved pt %u from codec preferences",
+          GST_TRACE_OBJECT (trans, "have media pt %u from codec preferences",
               pt);
-          g_array_append_val (reserved_pts, pt);
+          find_or_create_payload_map_for_media_pt (media_mapping, pt);
         }
       }
     }
@@ -3422,7 +3501,7 @@ gather_reserved_pts (GstWebRTCBin * webrtc)
   }
   GST_OBJECT_UNLOCK (webrtc);
 
-  return reserved_pts;
+  return media_mapping;
 }
 
 static gboolean
@@ -3529,7 +3608,7 @@ _create_offer_task (GstWebRTCBin * webrtc, const GstStructure * options,
   GString *bundled_mids = NULL;
   gchar *bundle_ufrag = NULL;
   gchar *bundle_pwd = NULL;
-  GArray *reserved_pts = NULL;
+  GArray *media_mapping = NULL;
   GHashTable *all_mids =
       g_hash_table_new_full (g_str_hash, g_str_equal, g_free, NULL);
 
@@ -3568,7 +3647,7 @@ _create_offer_task (GstWebRTCBin * webrtc, const GstStructure * options,
     GStrv last_bundle = NULL;
     guint bundle_media_index;
 
-    reserved_pts = gather_reserved_pts (webrtc);
+    media_mapping = gather_media_mapping (webrtc);
     if (last_offer && _parse_bundle (last_offer, &last_bundle, NULL)
         && last_bundle && last_bundle[0]
         && _get_bundle_index (last_offer, last_bundle, &bundle_media_index)) {
@@ -3625,13 +3704,15 @@ _create_offer_task (GstWebRTCBin * webrtc, const GstStructure * options,
                 trans->mid, media_idx);
 
             if (webrtc->bundle_policy == GST_WEBRTC_BUNDLE_POLICY_NONE) {
-              reserved_pts = g_array_new (FALSE, FALSE, sizeof (guint));
+              media_mapping =
+                  g_array_new (FALSE, FALSE,
+                  sizeof (struct media_payload_map_item));
             }
 
             gst_sdp_media_init (&media);
             if (!sdp_media_from_transceiver (webrtc, &media, last_media, trans,
                     media_idx, bundled_mids, 0, bundle_ufrag, bundle_pwd,
-                    reserved_pts, all_mids, error)) {
+                    media_mapping, all_mids, error)) {
               gst_sdp_media_uninit (&media);
               if (!*error)
                 g_set_error_literal (error, GST_WEBRTC_ERROR,
@@ -3640,8 +3721,8 @@ _create_offer_task (GstWebRTCBin * webrtc, const GstStructure * options,
             }
 
             if (webrtc->bundle_policy == GST_WEBRTC_BUNDLE_POLICY_NONE) {
-              g_array_free (reserved_pts, TRUE);
-              reserved_pts = NULL;
+              g_array_free (media_mapping, TRUE);
+              media_mapping = NULL;
             }
             if (*error)
               goto cancel_offer;
@@ -3777,14 +3858,15 @@ _create_offer_task (GstWebRTCBin * webrtc, const GstStructure * options,
     gst_sdp_media_init (&media);
 
     if (webrtc->bundle_policy == GST_WEBRTC_BUNDLE_POLICY_NONE) {
-      reserved_pts = g_array_new (FALSE, FALSE, sizeof (guint));
+      media_mapping =
+          g_array_new (FALSE, FALSE, sizeof (struct media_payload_map_item));
     }
 
     GST_LOG_OBJECT (webrtc, "adding transceiver %" GST_PTR_FORMAT " at media "
         "index %u", trans, media_idx);
 
     if (sdp_media_from_transceiver (webrtc, &media, NULL, trans, media_idx,
-            bundled_mids, 0, bundle_ufrag, bundle_pwd, reserved_pts, all_mids,
+            bundled_mids, 0, bundle_ufrag, bundle_pwd, media_mapping, all_mids,
             error)) {
       /* as per JSEP, a=rtcp-mux-only is only added for new streams */
       gst_sdp_media_add_attribute (&media, "rtcp-mux-only", "");
@@ -3795,16 +3877,16 @@ _create_offer_task (GstWebRTCBin * webrtc, const GstStructure * options,
     }
 
     if (webrtc->bundle_policy == GST_WEBRTC_BUNDLE_POLICY_NONE) {
-      g_array_free (reserved_pts, TRUE);
-      reserved_pts = NULL;
+      g_array_free (media_mapping, TRUE);
+      media_mapping = NULL;
     }
     if (*error)
       goto cancel_offer;
   }
 
   if (webrtc->bundle_policy != GST_WEBRTC_BUNDLE_POLICY_NONE) {
-    g_array_free (reserved_pts, TRUE);
-    reserved_pts = NULL;
+    g_array_free (media_mapping, TRUE);
+    media_mapping = NULL;
   }
 
   webrtc->priv->max_sink_pad_serial = MAX (webrtc->priv->max_sink_pad_serial,
@@ -3835,8 +3917,8 @@ _create_offer_task (GstWebRTCBin * webrtc, const GstStructure * options,
   }
 
 out:
-  if (reserved_pts)
-    g_array_free (reserved_pts, TRUE);
+  if (media_mapping)
+    g_array_free (media_mapping, TRUE);
 
   g_hash_table_unref (all_mids);
 
index b1ce5b1..c6b853d 100644 (file)
@@ -5179,6 +5179,65 @@ GST_START_TEST (test_simulcast_fec_rtx)
 
 GST_END_TEST;
 
+GST_START_TEST (test_bundle_multiple_media_rtx_payload_mapping)
+{
+  struct test_webrtc *t = test_webrtc_new ();
+  guint offer_media_format_count[] = { 5, 5, };
+  VAL_SDP_INIT (payloads0, on_sdp_media_payload_types, GUINT_TO_POINTER (0),
+      NULL);
+  VAL_SDP_INIT (payloads1, on_sdp_media_payload_types, GUINT_TO_POINTER (1),
+      &payloads0);
+  VAL_SDP_INIT (no_dup_payloads, on_sdp_media_no_duplicate_payloads, NULL,
+      &payloads1);
+  VAL_SDP_INIT (media_formats, on_sdp_media_count_formats,
+      offer_media_format_count, &no_dup_payloads);
+  const gchar *expected_offer_setup[] = { "actpass", "actpass", };
+  VAL_SDP_INIT (setup, on_sdp_media_setup, expected_offer_setup,
+      &media_formats);
+  const gchar *expected_offer_direction[] = { "recvonly", "recvonly", };
+  VAL_SDP_INIT (offer, on_sdp_media_direction, expected_offer_direction,
+      &setup);
+  GstWebRTCRTPTransceiverDirection direction;
+  GstWebRTCRTPTransceiver *trans;
+  GstCaps *caps;
+
+  /* add two identical transceivers that will only receive a vp8 stream and check that
+   * the created offer has the same rtx/red mappings */
+  t->on_negotiation_needed = NULL;
+  t->on_ice_candidate = NULL;
+
+  gst_util_set_object_arg (G_OBJECT (t->webrtc1), "bundle-policy",
+      "max-bundle");
+  gst_util_set_object_arg (G_OBJECT (t->webrtc2), "bundle-policy",
+      "max-bundle");
+
+  /* setup recvonly transceiver */
+  caps = gst_caps_from_string (VP8_RTP_CAPS (97));
+  direction = GST_WEBRTC_RTP_TRANSCEIVER_DIRECTION_RECVONLY;
+  g_signal_emit_by_name (t->webrtc1, "add-transceiver", direction, caps,
+      &trans);
+  fail_unless (trans != NULL);
+  g_object_set (GST_OBJECT (trans), "do-nack", TRUE, "fec-type",
+      GST_WEBRTC_FEC_TYPE_ULP_RED, NULL);
+  gst_object_unref (trans);
+
+  direction = GST_WEBRTC_RTP_TRANSCEIVER_DIRECTION_RECVONLY;
+  g_signal_emit_by_name (t->webrtc1, "add-transceiver", direction, caps,
+      &trans);
+  fail_unless (trans != NULL);
+  g_object_set (GST_OBJECT (trans), "do-nack", TRUE, "fec-type",
+      GST_WEBRTC_FEC_TYPE_ULP_RED, NULL);
+  gst_object_unref (trans);
+
+  gst_caps_unref (caps);
+
+  /* don't really care about the answer */
+  test_validate_sdp (t, &offer, NULL);
+
+  test_webrtc_free (t);
+}
+
+GST_END_TEST;
 static Suite *
 webrtcbin_suite (void)
 {
@@ -5238,6 +5297,7 @@ webrtcbin_suite (void)
     tcase_add_test (tc, test_max_bundle_fec);
     tcase_add_test (tc, test_simulcast);
     tcase_add_test (tc, test_simulcast_fec_rtx);
+    tcase_add_test (tc, test_bundle_multiple_media_rtx_payload_mapping);
     if (sctpenc && sctpdec) {
       tcase_add_test (tc, test_data_channel_create);
       tcase_add_test (tc, test_data_channel_remote_notify);