rtpbasepayload: Try harder to reuse previously configured caps values and give more preference to anything set as properties

This affects the pt, ssrc, seqnum-offset and timestamp-offset properties. If
they were set from a property, or we configured caps before, we try to use
that value for them. Even if the first structure of the downstream caps
specifies a different value, we check if the value is supported by other
structures.
Only if all this fails, we use the values given by downstream in the first
structure, i.e. if no properties were set and these are the first caps we
negotiate or downstream does not support our values.

By doing this we ensure that we don't spuriously change ssrcs or other fields
in the middle of the stream (and also consider property values more). Ssrc
changes would currently happen after sending an RTX packet (thus creating a
new internal source inside the rtpsession), and then renegotiating the
payloader (which then gets the RTX ssrc from rtpsession).

https://bugzilla.gnome.org/show_bug.cgi?id=749581

1 files changed, 216 insertions, 68 deletions

diff --git a/gst-libs/gst/rtp/gstrtpbasepayload.c b/gst-libs/gst/rtp/gstrtpbasepayload.c
index e3bf92afd..d620be7fd 100644
--- a/gst-libs/gst/rtp/gstrtpbasepayload.c
+++ b/gst-libs/gst/rtp/gstrtpbasepayload.c
@@ -44,6 +44,8 @@ struct _GstRTPBasePayloadPrivate
   gboolean perfect_rtptime;
   gint notified_first_timestamp;
 
+  gboolean pt_set;
+
   guint64 base_offset;
   gint64 base_rtime;
   guint64 base_rtime_hz;
@@ -370,6 +372,7 @@ gst_rtp_base_payload_init (GstRTPBasePayload * rtpbasepayload, gpointer g_class)
   priv->seqnum_offset_random = (rtpbasepayload->seqnum_offset == -1);
   priv->ts_offset_random = (rtpbasepayload->ts_offset == -1);
   priv->ssrc_random = (rtpbasepayload->ssrc == -1);
+  priv->pt_set = FALSE;
 
   rtpbasepayload->max_ptime = DEFAULT_MAX_PTIME;
   rtpbasepayload->min_ptime = DEFAULT_MIN_PTIME;
@@ -771,7 +774,7 @@ gst_rtp_base_payload_set_outcaps (GstRTPBasePayload * payload,
 static gboolean
 gst_rtp_base_payload_negotiate (GstRTPBasePayload * payload)
 {
-  GstCaps *peercaps, *filter, *srccaps;
+  GstCaps *templ, *peercaps, *srccaps;
   gboolean res;
 
   payload->priv->caps_max_ptime = DEFAULT_MAX_PTIME;
@@ -779,22 +782,21 @@ gst_rtp_base_payload_negotiate (GstRTPBasePayload * payload)
 
   gst_pad_check_reconfigure (payload->srcpad);
 
-  filter = gst_pad_get_pad_template_caps (payload->srcpad);
+  templ = gst_pad_get_pad_template_caps (payload->srcpad);
 
   if (payload->priv->subclass_srccaps) {
     GstCaps *tmp = gst_caps_intersect (payload->priv->subclass_srccaps,
-        filter);
-    gst_caps_unref (filter);
-    filter = tmp;
+        templ);
+    gst_caps_unref (templ);
+    templ = tmp;
   }
 
-  peercaps = gst_pad_peer_query_caps (payload->srcpad, filter);
-  gst_caps_unref (filter);
+  peercaps = gst_pad_peer_query_caps (payload->srcpad, templ);
 
   if (peercaps == NULL) {
     /* no peer caps, just add the other properties */
 
-    srccaps = gst_caps_copy (payload->priv->subclass_srccaps);
+    srccaps = gst_caps_copy (templ);
     gst_caps_set_simple (srccaps,
         "payload", G_TYPE_INT, GST_RTP_BASE_PAYLOAD_PT (payload),
         "ssrc", G_TYPE_UINT, payload->current_ssrc,
@@ -806,91 +808,235 @@ gst_rtp_base_payload_negotiate (GstRTPBasePayload * payload)
     GstCaps *temp;
     GstStructure *s, *d;
     const GValue *value;
-    gint pt;
+    gboolean have_pt = FALSE, have_ssrc = FALSE;
+    gboolean have_ts_offset = FALSE;
+    gboolean have_seqnum_offset = FALSE;
     guint max_ptime, ptime;
 
     /* peer provides caps we can use to fixate. They are already intersected
      * with our srccaps, just make them writable */
     temp = gst_caps_make_writable (peercaps);
+    peercaps = NULL;
 
     if (gst_caps_is_empty (temp)) {
       gst_caps_unref (temp);
+      gst_caps_unref (templ);
       return FALSE;
     }
 
-    /* now fixate, start by taking the first caps */
-    temp = gst_caps_truncate (temp);
+    /* We prefer the pt, ssrc, timestamp-offset, seqnum-offset from the
+     * property (if set), or any previously configured value over what
+     * downstream prefers. Only if downstream can't accept that, or the
+     * properties were not set, we fall back to choosing downstream's
+     * preferred value
+     */
 
-    /* get first structure */
-    s = gst_caps_get_structure (temp, 0);
+    /* try to use the previously set pt, or the one from the property */
+    if (payload->priv->pt_set || gst_pad_has_current_caps (payload->srcpad)) {
+      GstCaps *probe_caps = gst_caps_copy (templ);
+      GstCaps *intersection;
 
-    if (gst_structure_get_uint (s, "maxptime", &max_ptime))
-      payload->priv->caps_max_ptime = max_ptime * GST_MSECOND;
+      gst_caps_set_simple (probe_caps, "payload", G_TYPE_INT,
+          GST_RTP_BASE_PAYLOAD_PT (payload), NULL);
+      intersection = gst_caps_intersect (probe_caps, temp);
 
-    if (gst_structure_get_uint (s, "ptime", &ptime))
-      payload->ptime = ptime * GST_MSECOND;
-
-    if (gst_structure_get_int (s, "payload", &pt)) {
-      /* use peer pt */
-      GST_RTP_BASE_PAYLOAD_PT (payload) = pt;
-      GST_LOG_OBJECT (payload, "using peer pt %d", pt);
-    } else {
-      if (gst_structure_has_field (s, "payload")) {
-        /* can only fixate if there is a field */
-        gst_structure_fixate_field_nearest_int (s, "payload",
+      if (!gst_caps_is_empty (intersection)) {
+        GST_LOG_OBJECT (payload, "Using selected pt %d",
             GST_RTP_BASE_PAYLOAD_PT (payload));
-        gst_structure_get_int (s, "payload", &pt);
-        GST_LOG_OBJECT (payload, "using peer pt %d", pt);
+        have_pt = TRUE;
+        gst_caps_unref (temp);
+        temp = intersection;
+      } else {
+        GST_WARNING_OBJECT (payload, "Can't use selected pt %d",
+            GST_RTP_BASE_PAYLOAD_PT (payload));
+        gst_caps_unref (intersection);
+      }
+      gst_caps_unref (probe_caps);
+    }
+
+    /* If we got no pt above, select one now */
+    if (!have_pt) {
+      gint pt;
+
+      /* get first structure */
+      s = gst_caps_get_structure (temp, 0);
+
+      if (gst_structure_get_int (s, "payload", &pt)) {
+        /* use peer pt */
         GST_RTP_BASE_PAYLOAD_PT (payload) = pt;
+        GST_LOG_OBJECT (payload, "using peer pt %d", pt);
       } else {
-        /* no pt field, use the internal pt */
-        pt = GST_RTP_BASE_PAYLOAD_PT (payload);
-        gst_structure_set (s, "payload", G_TYPE_INT, pt, NULL);
-        GST_LOG_OBJECT (payload, "using internal pt %d", pt);
+        if (gst_structure_has_field (s, "payload")) {
+          /* can only fixate if there is a field */
+          gst_structure_fixate_field_nearest_int (s, "payload",
+              GST_RTP_BASE_PAYLOAD_PT (payload));
+          gst_structure_get_int (s, "payload", &pt);
+          GST_RTP_BASE_PAYLOAD_PT (payload) = pt;
+          GST_LOG_OBJECT (payload, "using peer pt %d", pt);
+        } else {
+          /* no pt field, use the internal pt */
+          pt = GST_RTP_BASE_PAYLOAD_PT (payload);
+          gst_structure_set (s, "payload", G_TYPE_INT, pt, NULL);
+          GST_LOG_OBJECT (payload, "using internal pt %d", pt);
+        }
       }
+      s = NULL;
     }
 
-    if (gst_structure_has_field_typed (s, "ssrc", G_TYPE_UINT)) {
-      value = gst_structure_get_value (s, "ssrc");
-      payload->current_ssrc = g_value_get_uint (value);
-      GST_LOG_OBJECT (payload, "using peer ssrc %08x", payload->current_ssrc);
-    } else {
-      /* FIXME, fixate_nearest_uint would be even better */
-      gst_structure_set (s, "ssrc", G_TYPE_UINT, payload->current_ssrc, NULL);
-      GST_LOG_OBJECT (payload, "using internal ssrc %08x",
-          payload->current_ssrc);
+    /* try to select the previously used ssrc, or the one from the property */
+    if (!payload->priv->ssrc_random
+        || gst_pad_has_current_caps (payload->srcpad)) {
+      GstCaps *probe_caps = gst_caps_copy (templ);
+      GstCaps *intersection;
+
+      gst_caps_set_simple (probe_caps, "ssrc", G_TYPE_UINT,
+          payload->current_ssrc, NULL);
+      intersection = gst_caps_intersect (probe_caps, temp);
+
+      if (!gst_caps_is_empty (intersection)) {
+        GST_LOG_OBJECT (payload, "Using selected ssrc %08x",
+            payload->current_ssrc);
+        gst_caps_unref (temp);
+        temp = intersection;
+        have_ssrc = TRUE;
+      } else {
+        GST_WARNING_OBJECT (payload, "Can't use selected ssrc %08x",
+            payload->current_ssrc);
+        gst_caps_unref (intersection);
+      }
+      gst_caps_unref (probe_caps);
     }
 
-    if (gst_structure_has_field_typed (s, "timestamp-offset", G_TYPE_UINT)) {
-      value = gst_structure_get_value (s, "timestamp-offset");
-      payload->ts_base = g_value_get_uint (value);
-      GST_LOG_OBJECT (payload, "using peer timestamp-offset %u",
-          payload->ts_base);
-    } else {
-      /* FIXME, fixate_nearest_uint would be even better */
-      gst_structure_set (s, "timestamp-offset", G_TYPE_UINT, payload->ts_base,
-          NULL);
-      GST_LOG_OBJECT (payload, "using internal timestamp-offset %u",
-          payload->ts_base);
+    /* If we got no ssrc above, select one now */
+    if (!have_ssrc) {
+      /* get first structure */
+      s = gst_caps_get_structure (temp, 0);
+
+      if (gst_structure_has_field_typed (s, "ssrc", G_TYPE_UINT)) {
+        value = gst_structure_get_value (s, "ssrc");
+        payload->current_ssrc = g_value_get_uint (value);
+        GST_LOG_OBJECT (payload, "using peer ssrc %08x", payload->current_ssrc);
+      } else {
+        /* FIXME, fixate_nearest_uint would be even better but we
+         * don't support uint ranges so how likely is it that anybody
+         * uses a list of possible ssrcs */
+        gst_structure_set (s, "ssrc", G_TYPE_UINT, payload->current_ssrc, NULL);
+        GST_LOG_OBJECT (payload, "using internal ssrc %08x",
+            payload->current_ssrc);
+      }
+      s = NULL;
     }
-    if (gst_structure_has_field_typed (s, "seqnum-offset", G_TYPE_UINT)) {
-      value = gst_structure_get_value (s, "seqnum-offset");
-      payload->seqnum_base = g_value_get_uint (value);
-      GST_LOG_OBJECT (payload, "using peer seqnum-offset %u",
-          payload->seqnum_base);
-      payload->priv->next_seqnum = payload->seqnum_base;
-      payload->seqnum = payload->seqnum_base;
-      payload->priv->seqnum_offset_random = FALSE;
-    } else {
-      /* FIXME, fixate_nearest_uint would be even better */
-      gst_structure_set (s, "seqnum-offset", G_TYPE_UINT, payload->seqnum_base,
-          NULL);
-      GST_LOG_OBJECT (payload, "using internal seqnum-offset %u",
-          payload->seqnum_base);
+
+    /* try to select the previously used timestamp-offset, or the one from the property */
+    if (!payload->priv->ts_offset_random
+        || gst_pad_has_current_caps (payload->srcpad)) {
+      GstCaps *probe_caps = gst_caps_copy (templ);
+      GstCaps *intersection;
+
+      gst_caps_set_simple (probe_caps, "timestamp-offset", G_TYPE_UINT,
+          payload->ts_base, NULL);
+      intersection = gst_caps_intersect (probe_caps, temp);
+
+      if (!gst_caps_is_empty (intersection)) {
+        GST_LOG_OBJECT (payload, "Using selected timestamp-offset %u",
+            payload->ts_base);
+        gst_caps_unref (temp);
+        temp = intersection;
+        have_ts_offset = TRUE;
+      } else {
+        GST_WARNING_OBJECT (payload, "Can't use selected timestamp-offset %u",
+            payload->ts_base);
+        gst_caps_unref (intersection);
+      }
+      gst_caps_unref (probe_caps);
+    }
+
+    /* If we got no timestamp-offset above, select one now */
+    if (!have_ts_offset) {
+      /* get first structure */
+      s = gst_caps_get_structure (temp, 0);
+
+      if (gst_structure_has_field_typed (s, "timestamp-offset", G_TYPE_UINT)) {
+        value = gst_structure_get_value (s, "timestamp-offset");
+        payload->ts_base = g_value_get_uint (value);
+        GST_LOG_OBJECT (payload, "using peer timestamp-offset %u",
+            payload->ts_base);
+      } else {
+        /* FIXME, fixate_nearest_uint would be even better but we
+         * don't support uint ranges so how likely is it that anybody
+         * uses a list of possible timestamp-offsets */
+        gst_structure_set (s, "timestamp-offset", G_TYPE_UINT, payload->ts_base,
+            NULL);
+        GST_LOG_OBJECT (payload, "using internal timestamp-offset %u",
+            payload->ts_base);
+      }
+      s = NULL;
     }
 
-    /* make the target caps by copying over all the fixed caps, removing the
-     * unfixed caps. */
+    /* try to select the previously used seqnum-offset, or the one from the property */
+    if (!payload->priv->seqnum_offset_random
+        || gst_pad_has_current_caps (payload->srcpad)) {
+      GstCaps *probe_caps = gst_caps_copy (templ);
+      GstCaps *intersection;
+
+      gst_caps_set_simple (probe_caps, "seqnum-offset", G_TYPE_UINT,
+          payload->seqnum_base, NULL);
+      intersection = gst_caps_intersect (probe_caps, temp);
+
+      if (!gst_caps_is_empty (intersection)) {
+        GST_LOG_OBJECT (payload, "Using selected seqnum-offset %u",
+            payload->seqnum_base);
+        gst_caps_unref (temp);
+        temp = intersection;
+        have_seqnum_offset = TRUE;
+      } else {
+        GST_WARNING_OBJECT (payload, "Can't use selected seqnum-offset %u",
+            payload->seqnum_base);
+        gst_caps_unref (intersection);
+      }
+      gst_caps_unref (probe_caps);
+    }
+
+    /* If we got no seqnum-offset above, select one now */
+    if (!have_seqnum_offset) {
+      /* get first structure */
+      s = gst_caps_get_structure (temp, 0);
+
+      if (gst_structure_has_field_typed (s, "seqnum-offset", G_TYPE_UINT)) {
+        value = gst_structure_get_value (s, "seqnum-offset");
+        payload->seqnum_base = g_value_get_uint (value);
+        GST_LOG_OBJECT (payload, "using peer seqnum-offset %u",
+            payload->seqnum_base);
+        payload->priv->next_seqnum = payload->seqnum_base;
+        payload->seqnum = payload->seqnum_base;
+        payload->priv->seqnum_offset_random = FALSE;
+      } else {
+        /* FIXME, fixate_nearest_uint would be even better but we
+         * don't support uint ranges so how likely is it that anybody
+         * uses a list of possible seqnum-offsets */
+        gst_structure_set (s, "seqnum-offset", G_TYPE_UINT,
+            payload->seqnum_base, NULL);
+        GST_LOG_OBJECT (payload, "using internal seqnum-offset %u",
+            payload->seqnum_base);
+      }
+
+      s = NULL;
+    }
+
+    /* now fixate, start by taking the first caps */
+    temp = gst_caps_truncate (temp);
+
+    /* get first structure */
+    s = gst_caps_get_structure (temp, 0);
+
+    if (gst_structure_get_uint (s, "maxptime", &max_ptime))
+      payload->priv->caps_max_ptime = max_ptime * GST_MSECOND;
+
+    if (gst_structure_get_uint (s, "ptime", &ptime))
+      payload->ptime = ptime * GST_MSECOND;
+
+    /* make the target caps by copying over all the fixed fields, removing the
+     * unfixed fields. */
     srccaps = gst_caps_new_empty_simple (gst_structure_get_name (s));
     d = gst_caps_get_structure (srccaps, 0);
 
@@ -905,6 +1051,7 @@ gst_rtp_base_payload_negotiate (GstRTPBasePayload * payload)
 
   res = gst_pad_set_caps (GST_RTP_BASE_PAYLOAD_SRCPAD (payload), srccaps);
   gst_caps_unref (srccaps);
+  gst_caps_unref (templ);
 
   return res;
 }
@@ -1222,6 +1369,7 @@ gst_rtp_base_payload_set_property (GObject * object, guint prop_id,
       break;
     case PROP_PT:
       rtpbasepayload->pt = g_value_get_uint (value);
+      priv->pt_set = TRUE;
       break;
     case PROP_SSRC:
       val = g_value_get_uint (value);