pad: Fix sticky event ordering for instant-rate-change

The event type for instant-rate-change events was poorly chosen,
leading to them being re-sent too late and even after EOS.

Add a mechanism in GstPad for the sticky event order to be
different to the value of the event type to fix that up.

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

subprojects/gstreamer/gst/gstevent.c

@@ -213,6 +213,33 @@ gst_event_type_get_flags (GstEventType type)
   return ret;
 }
 
+/**
+ * gst_event_type_to_sticky_ordering
+ * @type: a #GstEventType
+ *
+ * Converts the #GstEventType to an unsigned integer that
+ * represents the ordering of sticky events when re-sending them.
+ * A lower value represents a higher-priority event.
+ *
+ * Returns: an unsigned integer
+ * Since: 1.22
+ */
+/* FIXME 2.0: Remove the sticky event order overrides once
+ * the event type numbers are fixed */
+guint
+gst_event_type_to_sticky_ordering (GstEventType type)
+{
+  guint sticky_order = type;
+
+  /* Fix up the sticky event ordering for events where the
+   * type was chosen poorly */
+  if (type == GST_EVENT_INSTANT_RATE_CHANGE) {
+    sticky_order = GST_EVENT_SEGMENT + 1;
+  }
+
+  return sticky_order;
+}
+
 static void
 _gst_event_free (GstEvent * event)
 {

subprojects/gstreamer/gst/gstevent.h

@@ -169,6 +169,7 @@ typedef enum {
   GST_EVENT_GAP                   = GST_EVENT_MAKE_TYPE (160, _FLAG(DOWNSTREAM) | _FLAG(SERIALIZED)),
 
   /* sticky downstream non-serialized */
+  /* FIXME 2.0: change to value 72 and move after the GST_EVENT_SEGMENT event */
   GST_EVENT_INSTANT_RATE_CHANGE   = GST_EVENT_MAKE_TYPE (180, _FLAG(DOWNSTREAM) | _FLAG(STICKY)),
 
   /* upstream events */
@@ -417,6 +418,10 @@ GST_API
 GstEventTypeFlags
                 gst_event_type_get_flags        (GstEventType type);
 
+
+GST_API
+guint gst_event_type_to_sticky_ordering (GstEventType type) G_GNUC_CONST;
+
 #ifndef GST_DISABLE_MINIOBJECT_INLINE_FUNCTIONS
 /* refcounting */
 static inline GstEvent *

subprojects/gstreamer/gst/gstpad.c

@@ -128,6 +128,7 @@ enum
 typedef struct
 {
   gboolean received;
+  guint sticky_order;
   GstEvent *event;
 } PadEvent;
 
@@ -459,6 +460,8 @@ remove_events (GstPad * pad)
   }
 }
 
+#define _to_sticky_order(t) gst_event_type_to_sticky_ordering(t)
+
 /* should be called with object lock */
 static PadEvent *
 find_event_by_type (GstPad * pad, GstEventType type, guint idx)
@@ -466,6 +469,7 @@ find_event_by_type (GstPad * pad, GstEventType type, guint idx)
   guint i, len;
   GArray *events;
   PadEvent *ev;
+  guint last_sticky_order = _to_sticky_order (type);
 
   events = pad->priv->events;
   len = events->len;
@@ -479,7 +483,7 @@ find_event_by_type (GstPad * pad, GstEventType type, guint idx)
       if (idx == 0)
         goto found;
       idx--;
-    } else if (GST_EVENT_TYPE (ev->event) > type) {
+    } else if (ev->sticky_order > last_sticky_order) {
       break;
     }
   }
@@ -499,11 +503,12 @@ find_event (GstPad * pad, GstEvent * event)
   events = pad->priv->events;
   len = events->len;
 
+  guint sticky_order = _to_sticky_order (GST_EVENT_TYPE (event));
   for (i = 0; i < len; i++) {
     ev = &g_array_index (events, PadEvent, i);
     if (event == ev->event)
       goto found;
-    else if (GST_EVENT_TYPE (ev->event) > GST_EVENT_TYPE (event))
+    else if (ev->sticky_order > sticky_order)
       break;
   }
   ev = NULL;
@@ -522,13 +527,15 @@ remove_event_by_type (GstPad * pad, GstEventType type)
   events = pad->priv->events;
   len = events->len;
 
+  guint last_sticky_order = _to_sticky_order (type);
+
   i = 0;
   while (i < len) {
     ev = &g_array_index (events, PadEvent, i);
     if (ev->event == NULL)
       goto next;
 
-    if (GST_EVENT_TYPE (ev->event) > type)
+    if (ev->sticky_order > last_sticky_order)
       break;
     else if (GST_EVENT_TYPE (ev->event) != type)
       goto next;
@@ -599,6 +606,7 @@ restart:
       goto next;
 
     /* take additional ref, func might release the lock */
+    ev_ret.sticky_order = ev->sticky_order;
     ev_ret.event = gst_event_ref (ev->event);
     ev_ret.received = ev->received;
 
@@ -4033,12 +4041,17 @@ push_sticky (GstPad * pad, PadEvent * ev, gpointer user_data)
     return TRUE;
   }
 
+  guint data_sticky_order = 0;
+  if (data->event) {
+    data_sticky_order = _to_sticky_order (GST_EVENT_TYPE (data->event));
+  }
+
   /* If we're called because of an sticky event, only forward
    * events that would come before this new event and the
    * event itself */
   if (data->event && GST_EVENT_IS_STICKY (data->event) &&
-      GST_EVENT_TYPE (data->event) <= GST_EVENT_SEGMENT &&
+      data_sticky_order <= _to_sticky_order (GST_EVENT_SEGMENT) &&
-      GST_EVENT_TYPE (data->event) < GST_EVENT_TYPE (event)) {
+      data_sticky_order < ev->sticky_order) {
     data->ret = GST_FLOW_CUSTOM_SUCCESS_1;
   } else {
     data->ret = gst_pad_push_event_unchecked (pad, gst_event_ref (event),
@@ -5295,6 +5308,7 @@ store_sticky_event (GstPad * pad, GstEvent * event)
   gboolean insert = TRUE;
 
   type = GST_EVENT_TYPE (event);
+  guint sticky_order = _to_sticky_order (type);
 
   /* Store all sticky events except SEGMENT/EOS when we're flushing,
    * otherwise they can be dropped and nothing would ever resend them.
@@ -5343,11 +5357,11 @@ store_sticky_event (GstPad * pad, GstEvent * event)
       break;
     }
 
-    if (type < GST_EVENT_TYPE (ev->event) || (type != GST_EVENT_TYPE (ev->event)
+    if (sticky_order < ev->sticky_order || (type != GST_EVENT_TYPE (ev->event)
             && GST_EVENT_TYPE (ev->event) == GST_EVENT_EOS)) {
       /* STREAM_START, CAPS and SEGMENT must be delivered in this order. By
        * storing the sticky ordered we can check that this is respected. */
-      if (G_UNLIKELY (GST_EVENT_TYPE (ev->event) <= GST_EVENT_SEGMENT
+      if (G_UNLIKELY (ev->sticky_order <= _to_sticky_order (GST_EVENT_SEGMENT)
               || GST_EVENT_TYPE (ev->event) == GST_EVENT_EOS))
         g_warning (G_STRLOC
             ":%s:<%s:%s> Sticky event misordering, got '%s' before '%s'",
@@ -5359,6 +5373,7 @@ store_sticky_event (GstPad * pad, GstEvent * event)
   }
   if (insert) {
     PadEvent ev;
+    ev.sticky_order = sticky_order;
     ev.event = gst_event_ref (event);
     ev.received = FALSE;
     g_array_insert_val (events, i, ev);
@@ -5438,7 +5453,7 @@ sticky_changed (GstPad * pad, PadEvent * ev, gpointer user_data)
 
   /* Forward all sticky events before our current one that are pending */
   if (ev->event != data->event
-      && GST_EVENT_TYPE (ev->event) < GST_EVENT_TYPE (data->event))
+      && ev->sticky_order < _to_sticky_order (GST_EVENT_TYPE (data->event)))
     return push_sticky (pad, ev, data);
 
   return TRUE;

subprojects/gstreamer/tests/check/gst/gstpad.c

@@ -2591,6 +2591,12 @@ test_sticky_events_handler (GstPad * pad, GstObject * parent, GstEvent * event)
     case 2:
       fail_unless (GST_EVENT_TYPE (event) == GST_EVENT_SEGMENT);
       break;
+    case 3:
+      fail_unless (GST_EVENT_TYPE (event) == GST_EVENT_INSTANT_RATE_CHANGE);
+      break;
+    case 4:
+      fail_unless (GST_EVENT_TYPE (event) == GST_EVENT_STREAM_COLLECTION);
+      break;
     default:
       fail_unless (FALSE);
       break;
@@ -2642,6 +2648,15 @@ GST_START_TEST (test_sticky_events)
   gst_segment_init (&seg, GST_FORMAT_TIME);
   gst_pad_push_event (srcpad, gst_event_new_segment (&seg));
 
+  /* Push a stream collection */
+  GstStreamCollection *collection = gst_stream_collection_new (0);
+  gst_pad_push_event (srcpad, gst_event_new_stream_collection (collection));
+  gst_object_unref (collection);
+
+  /* Push an instant rate change, which should be sent earlier than the preceding stream collection */
+  gst_pad_push_event (srcpad, gst_event_new_instant_rate_change (1.0,
+          GST_SEGMENT_FLAG_NONE));
+
   /* now make a sinkpad */
   sinkpad = gst_pad_new ("sink", GST_PAD_SINK);
   fail_unless (sinkpad != NULL);
@@ -2662,13 +2677,13 @@ GST_START_TEST (test_sticky_events)
   gst_pad_push_event (srcpad, gst_event_new_caps (caps));
   gst_caps_unref (caps);
 
-  /* should have triggered 2 events, the segment event is still pending */
+  /* should have triggered 2 events, the segment, stream collection and instant-rate events are still pending */
   fail_unless_equals_int (sticky_count, 2);
 
   fail_unless (gst_pad_push (srcpad, gst_buffer_new ()) == GST_FLOW_OK);
 
-  /* should have triggered 3 events */
+  /* should have triggered 5 events */
-  fail_unless_equals_int (sticky_count, 3);
+  fail_unless_equals_int (sticky_count, 5);
 
   gst_object_unref (srcpad);
   gst_object_unref (sinkpad);