From 724c443a65c03027ee892f0ae02f464d04b97e70 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Piotr=20Brzezi=C5=84ski?= <piotr@centricular.com>
Date: Mon, 5 Aug 2024 14:37:57 +0200
Subject: [PATCH] videoencoder: Expose release_frame() and drop_frame() as
 public API

release_frame() can be useful for manually dropping frames without posting QoS messages like finish_frame() would.
Matches the same kind of API on the decoder side of things.

Modifies the behaviour of release_frame() to make sure events from released frames are stored as 'pending'
and pushed before the next non-dropped frame. This is needed because now release_frame() can be called outside of
finish_frame(), so we would potentially just lose events and bad things would happen.

drop_frame() was also added to match the decoder API. It functions almost identically to finish_frame() without a buffer
attached to the frame, except instead of immediately pushing the frame's events, it will store them as pending.

Part-of: <https://gitlab.freedesktop.org/gstreamer/gstreamer/-/merge_requests/7190>
---
 girs/GstVideo-1.0.gir                         |  46 ++++++
 .../gst-libs/gst/video/gstvideoencoder.c      | 131 +++++++++++++++---
 .../gst-libs/gst/video/gstvideoencoder.h      |   6 +
 3 files changed, 167 insertions(+), 16 deletions(-)

diff --git a/girs/GstVideo-1.0.gir b/girs/GstVideo-1.0.gir
index f0ac89363c..af1762780a 100644
--- a/girs/GstVideo-1.0.gir
+++ b/girs/GstVideo-1.0.gir
@@ -8929,6 +8929,28 @@ keep references to the frame, not the buffer.</doc>
           </parameter>
         </parameters>
       </method>
+      <method name="drop_frame" c:identifier="gst_video_encoder_drop_frame" version="1.26">
+        <doc xml:space="preserve" filename="../subprojects/gst-plugins-base/gst-libs/gst/video/gstvideoencoder.c">Removes @frame from the list of pending frames, releases it
+and posts a QoS message with the frame's details on the bus.
+Similar to calling gst_video_encoder_finish_frame() without a buffer
+attached to @frame, but this function additionally stores events
+from @frame as pending, to be pushed out alongside the next frame
+submitted via gst_video_encoder_finish_frame().</doc>
+        <source-position filename="../subprojects/gst-plugins-base/gst-libs/gst/video/gstvideoencoder.h"/>
+        <return-value transfer-ownership="none">
+          <type name="none" c:type="void"/>
+        </return-value>
+        <parameters>
+          <instance-parameter name="encoder" transfer-ownership="none">
+            <doc xml:space="preserve" filename="../subprojects/gst-plugins-base/gst-libs/gst/video/gstvideoencoder.c">a #GstVideoEncoder</doc>
+            <type name="VideoEncoder" c:type="GstVideoEncoder*"/>
+          </instance-parameter>
+          <parameter name="frame" transfer-ownership="full">
+            <doc xml:space="preserve" filename="../subprojects/gst-plugins-base/gst-libs/gst/video/gstvideoencoder.c">a #GstVideoCodecFrame</doc>
+            <type name="VideoCodecFrame" c:type="GstVideoCodecFrame*"/>
+          </parameter>
+        </parameters>
+      </method>
       <method name="finish_frame" c:identifier="gst_video_encoder_finish_frame">
         <doc xml:space="preserve" filename="../subprojects/gst-plugins-base/gst-libs/gst/video/gstvideoencoder.c">@frame must have a valid encoded data buffer, whose metadata fields
 are then appropriately set according to frame data or no buffer at
@@ -8936,6 +8958,10 @@ all if the frame should be dropped.
 It is subsequently pushed downstream or provided to @pre_push.
 In any case, the frame is considered finished and released.
 
+If @frame does not have a buffer attached, it will be dropped, and
+a QoS message will be posted on the bus. Events from @frame will be
+pushed out immediately.
+
 After calling this function the output buffer of the frame is to be
 considered read-only. This function will also change the metadata
 of the buffer.</doc>
@@ -9216,6 +9242,26 @@ elements (e.g. muxers).</doc>
           </parameter>
         </parameters>
       </method>
+      <method name="release_frame" c:identifier="gst_video_encoder_release_frame" version="1.26">
+        <doc xml:space="preserve" filename="../subprojects/gst-plugins-base/gst-libs/gst/video/gstvideoencoder.c">Removes @frame from list of pending frames and releases it, similar
+to calling gst_video_encoder_finish_frame() without a buffer attached
+to the frame, but does not post a QoS message or do any additional
+processing. Events from @frame are moved to the pending events list.</doc>
+        <source-position filename="../subprojects/gst-plugins-base/gst-libs/gst/video/gstvideoencoder.h"/>
+        <return-value transfer-ownership="none">
+          <type name="none" c:type="void"/>
+        </return-value>
+        <parameters>
+          <instance-parameter name="encoder" transfer-ownership="none">
+            <doc xml:space="preserve" filename="../subprojects/gst-plugins-base/gst-libs/gst/video/gstvideoencoder.c">a #GstVideoEncoder</doc>
+            <type name="VideoEncoder" c:type="GstVideoEncoder*"/>
+          </instance-parameter>
+          <parameter name="frame" transfer-ownership="full">
+            <doc xml:space="preserve" filename="../subprojects/gst-plugins-base/gst-libs/gst/video/gstvideoencoder.c">a #GstVideoCodecFrame</doc>
+            <type name="VideoCodecFrame" c:type="GstVideoCodecFrame*"/>
+          </parameter>
+        </parameters>
+      </method>
       <method name="set_headers" c:identifier="gst_video_encoder_set_headers">
         <doc xml:space="preserve" filename="../subprojects/gst-plugins-base/gst-libs/gst/video/gstvideoencoder.c">Set the codec headers to be sent downstream whenever requested.</doc>
         <source-position filename="../subprojects/gst-plugins-base/gst-libs/gst/video/gstvideoencoder.h"/>
diff --git a/subprojects/gst-plugins-base/gst-libs/gst/video/gstvideoencoder.c b/subprojects/gst-plugins-base/gst-libs/gst/video/gstvideoencoder.c
index 2c269e1aa3..23ea21f2fc 100644
--- a/subprojects/gst-plugins-base/gst-libs/gst/video/gstvideoencoder.c
+++ b/subprojects/gst-plugins-base/gst-libs/gst/video/gstvideoencoder.c
@@ -137,8 +137,12 @@ struct _GstVideoEncoderPrivate
   /* Tracks whether the latency message was posted at least once */
   gboolean posted_latency_msg;
 
+  /* events that should apply to the current frame */
   /* FIXME 2.0: Use a GQueue or similar, see GstVideoCodecFrame::events */
   GList *current_frame_events;
+  /* events that should be pushed before the next frame */
+  /* FIXME 2.0: Use a GQueue or similar, see GstVideoCodecFrame::events */
+  GList *pending_events;
 
   GList *headers;
   gboolean new_headers;         /* Whether new headers were just set */
@@ -281,6 +285,11 @@ static gboolean gst_video_encoder_negotiate_default (GstVideoEncoder * encoder);
 static gboolean gst_video_encoder_negotiate_unlocked (GstVideoEncoder *
     encoder);
 
+static void gst_video_encoder_post_qos_drop (GstVideoEncoder * enc,
+    GstVideoCodecFrame * frame);
+static void gst_video_encoder_push_pending_unlocked (GstVideoEncoder * encoder,
+    GstVideoCodecFrame * frame, gboolean dropping);
+
 static gboolean gst_video_encoder_sink_query_default (GstVideoEncoder * encoder,
     GstQuery * query);
 static gboolean gst_video_encoder_src_query_default (GstVideoEncoder * encoder,
@@ -503,9 +512,11 @@ gst_video_encoder_reset (GstVideoEncoder * encoder, gboolean hard)
       priv->allocator = NULL;
     }
 
-    g_list_foreach (priv->current_frame_events, (GFunc) gst_event_unref, NULL);
-    g_list_free (priv->current_frame_events);
+    g_list_free_full (priv->current_frame_events,
+        (GDestroyNotify) gst_event_unref);
     priv->current_frame_events = NULL;
+    g_list_free_full (priv->pending_events, (GDestroyNotify) gst_event_unref);
+    priv->pending_events = NULL;
 
     GST_OBJECT_LOCK (encoder);
     priv->proportion = 0.5;
@@ -2135,7 +2146,7 @@ gst_video_encoder_allocate_output_frame (GstVideoEncoder *
 }
 
 static void
-gst_video_encoder_release_frame (GstVideoEncoder * enc,
+gst_video_encoder_release_frame_unlocked (GstVideoEncoder * enc,
     GstVideoCodecFrame * frame)
 {
   GList *link;
@@ -2146,10 +2157,73 @@ gst_video_encoder_release_frame (GstVideoEncoder * enc,
     gst_video_codec_frame_unref (frame);
     g_queue_delete_link (&enc->priv->frames, link);
   }
+  if (frame->events) {
+    enc->priv->pending_events =
+        g_list_concat (frame->events, enc->priv->pending_events);
+    frame->events = NULL;
+  }
+
   /* unref because this function takes ownership */
   gst_video_codec_frame_unref (frame);
 }
 
+/**
+ * gst_video_encoder_release_frame:
+ * @encoder: a #GstVideoEncoder
+ * @frame: (transfer full): a #GstVideoCodecFrame
+ *
+ * Removes @frame from list of pending frames and releases it, similar 
+ * to calling gst_video_encoder_finish_frame() without a buffer attached
+ * to the frame, but does not post a QoS message or do any additional
+ * processing. Events from @frame are moved to the pending events list.
+ *
+ * Since: 1.26
+ */
+void
+gst_video_encoder_release_frame (GstVideoEncoder * enc,
+    GstVideoCodecFrame * frame)
+{
+  g_return_if_fail (GST_IS_VIDEO_ENCODER (enc));
+  g_return_if_fail (frame != NULL);
+
+  GST_LOG_OBJECT (enc, "Releasing frame %p", frame);
+
+  GST_VIDEO_ENCODER_STREAM_LOCK (enc);
+  gst_video_encoder_release_frame_unlocked (enc, frame);
+  GST_VIDEO_ENCODER_STREAM_UNLOCK (enc);
+}
+
+/**
+ * gst_video_encoder_drop_frame:
+ * @encoder: a #GstVideoEncoder
+ * @frame: (transfer full): a #GstVideoCodecFrame
+ *
+ * Removes @frame from the list of pending frames, releases it 
+ * and posts a QoS message with the frame's details on the bus.
+ * Similar to calling gst_video_encoder_finish_frame() without a buffer 
+ * attached to @frame, but this function additionally stores events
+ * from @frame as pending, to be pushed out alongside the next frame 
+ * submitted via gst_video_encoder_finish_frame().
+ *
+ * Since: 1.26
+ */
+void
+gst_video_encoder_drop_frame (GstVideoEncoder * enc, GstVideoCodecFrame * frame)
+{
+  g_return_if_fail (GST_IS_VIDEO_ENCODER (enc));
+  g_return_if_fail (frame != NULL);
+
+  GST_VIDEO_ENCODER_STREAM_LOCK (enc);
+
+  GST_LOG_OBJECT (enc, "Dropping frame %p", frame);
+
+  gst_video_encoder_push_pending_unlocked (enc, frame, TRUE);
+  gst_video_encoder_post_qos_drop (enc, frame);
+  gst_video_encoder_release_frame_unlocked (enc, frame);
+
+  GST_VIDEO_ENCODER_STREAM_UNLOCK (enc);
+}
+
 static gboolean
 gst_video_encoder_transform_meta_default (GstVideoEncoder *
     encoder, GstVideoCodecFrame * frame, GstMeta * meta)
@@ -2217,8 +2291,10 @@ foreach_metadata (GstBuffer * inbuf, GstMeta ** meta, gpointer user_data)
   return TRUE;
 }
 
+/* called with STREAM_LOCK */
 static void
-gst_video_encoder_drop_frame (GstVideoEncoder * enc, GstVideoCodecFrame * frame)
+gst_video_encoder_post_qos_drop (GstVideoEncoder * enc,
+    GstVideoCodecFrame * frame)
 {
   GstVideoEncoderPrivate *priv = enc->priv;
   GstClockTime stream_time, jitter, earliest_time, qostime, timestamp;
@@ -2282,23 +2358,32 @@ gst_video_encoder_can_push_unlocked (GstVideoEncoder * encoder)
   return GST_FLOW_OK;
 }
 
+static void
+gst_video_encoder_push_event_list (GstVideoEncoder * encoder, GList * events)
+{
+  GList *l;
+
+  /* events are stored in reverse order */
+  for (l = g_list_last (events); l; l = g_list_previous (l)) {
+    GST_LOG_OBJECT (encoder, "pushing %s event", GST_EVENT_TYPE_NAME (l->data));
+    gst_video_encoder_push_event (encoder, l->data);
+  }
+  g_list_free (events);
+}
+
 static void
 gst_video_encoder_push_pending_unlocked (GstVideoEncoder * encoder,
-    GstVideoCodecFrame * frame)
+    GstVideoCodecFrame * frame, gboolean dropping)
 {
   GstVideoEncoderPrivate *priv = encoder->priv;
-  GList *l;
+  GList *l, *events = NULL;
 
   /* Push all pending events that arrived before this frame */
   for (l = priv->frames.head; l; l = l->next) {
     GstVideoCodecFrame *tmp = l->data;
 
     if (tmp->events) {
-      GList *k;
-
-      for (k = g_list_last (tmp->events); k; k = k->prev)
-        gst_video_encoder_push_event (encoder, k->data);
-      g_list_free (tmp->events);
+      events = g_list_concat (tmp->events, events);
       tmp->events = NULL;
     }
 
@@ -2306,6 +2391,16 @@ gst_video_encoder_push_pending_unlocked (GstVideoEncoder * encoder,
       break;
   }
 
+  if (dropping) {
+    /* Push before the next frame that is not dropped */
+    priv->pending_events = g_list_concat (events, priv->pending_events);
+  } else {
+    gst_video_encoder_push_event_list (encoder, priv->pending_events);
+    priv->pending_events = NULL;
+
+    gst_video_encoder_push_event_list (encoder, events);
+  }
+
   gst_video_encoder_check_and_push_tags (encoder);
 }
 
@@ -2501,6 +2596,10 @@ gst_video_encoder_send_key_unit_unlocked (GstVideoEncoder * encoder,
  * It is subsequently pushed downstream or provided to @pre_push.
  * In any case, the frame is considered finished and released.
  *
+ * If @frame does not have a buffer attached, it will be dropped, and
+ * a QoS message will be posted on the bus. Events from @frame will be
+ * pushed out immediately.
+ *
  * After calling this function the output buffer of the frame is to be
  * considered read-only. This function will also change the metadata
  * of the buffer.
@@ -2541,11 +2640,11 @@ gst_video_encoder_finish_frame (GstVideoEncoder * encoder,
     goto done;
 
   if (frame->abidata.ABI.num_subframes == 0)
-    gst_video_encoder_push_pending_unlocked (encoder, frame);
+    gst_video_encoder_push_pending_unlocked (encoder, frame, FALSE);
 
   /* no buffer data means this frame is skipped/dropped */
   if (!frame->output_buffer) {
-    gst_video_encoder_drop_frame (encoder, frame);
+    gst_video_encoder_post_qos_drop (encoder, frame);
     goto done;
   }
 
@@ -2626,7 +2725,7 @@ gst_video_encoder_finish_frame (GstVideoEncoder * encoder,
    * if possible, i.e. if the subclass does not hold additional references
    * to the frame
    */
-  gst_video_encoder_release_frame (encoder, frame);
+  gst_video_encoder_release_frame_unlocked (encoder, frame);
   frame = NULL;
 
   if (ret == GST_FLOW_OK) {
@@ -2638,7 +2737,7 @@ gst_video_encoder_finish_frame (GstVideoEncoder * encoder,
 done:
   /* handed out */
   if (frame)
-    gst_video_encoder_release_frame (encoder, frame);
+    gst_video_encoder_release_frame_unlocked (encoder, frame);
 
   GST_VIDEO_ENCODER_STREAM_UNLOCK (encoder);
 
@@ -2704,7 +2803,7 @@ gst_video_encoder_finish_subframe (GstVideoEncoder * encoder,
    * Push new incoming events on finish_frame otherwise.
    */
   if (frame->abidata.ABI.num_subframes == 0)
-    gst_video_encoder_push_pending_unlocked (encoder, frame);
+    gst_video_encoder_push_pending_unlocked (encoder, frame, FALSE);
 
   if (GST_VIDEO_CODEC_FRAME_IS_SYNC_POINT (frame)
       && frame->abidata.ABI.num_subframes == 0) {
diff --git a/subprojects/gst-plugins-base/gst-libs/gst/video/gstvideoencoder.h b/subprojects/gst-plugins-base/gst-libs/gst/video/gstvideoencoder.h
index 2a03fdda20..86372a1106 100644
--- a/subprojects/gst-plugins-base/gst-libs/gst/video/gstvideoencoder.h
+++ b/subprojects/gst-plugins-base/gst-libs/gst/video/gstvideoencoder.h
@@ -387,6 +387,12 @@ void                 gst_video_encoder_set_min_force_key_unit_interval (GstVideo
 GST_VIDEO_API
 GstClockTime         gst_video_encoder_get_min_force_key_unit_interval (GstVideoEncoder * encoder);
 
+GST_VIDEO_API
+void                 gst_video_encoder_release_frame (GstVideoEncoder *encoder, GstVideoCodecFrame *frame);
+
+GST_VIDEO_API
+void                 gst_video_encoder_drop_frame (GstVideoEncoder *encoder, GstVideoCodecFrame *frame);
+
 G_DEFINE_AUTOPTR_CLEANUP_FUNC(GstVideoEncoder, gst_object_unref)
 
 G_END_DECLS