basetextoverlay: Don't miscalculate text running times
authorJan Schmidt <jan@centricular.com>
Wed, 27 Jul 2022 12:34:42 +0000 (22:34 +1000)
committerGStreamer Marge Bot <gitlab-merge-bot@gstreamer-foundation.org>
Fri, 12 Aug 2022 12:08:18 +0000 (12:08 +0000)
When a new segment event arrives, it immediately updates
the current stored segment, which was used for calculating
the running time of the current text buffer for every
passing video frame. This means a segment that arrives
after the text buffer might get used to (mis)calculate
the running times subsequently.

Instead, calculate and store the right running time
using the current segment when storing the buffer. Later
the stored segment can get freely updated.

This fixes the case where pieces of video and text streams
are seamlessly concatenated and fed through the text overlay.
Previously, it could lead to the current text buffer suddenly
have a massive running time and blocking all further input.

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

subprojects/gst-plugins-base/ext/pango/gstbasetextoverlay.c
subprojects/gst-plugins-base/ext/pango/gstbasetextoverlay.h

index b9d8fd4..2454a50 100644 (file)
@@ -776,6 +776,8 @@ gst_base_text_overlay_init (GstBaseTextOverlay * overlay,
       (PangoAlignment) overlay->line_align);
 
   overlay->text_buffer = NULL;
+  overlay->text_buffer_running_time = GST_CLOCK_TIME_NONE;
+  overlay->text_buffer_running_time_end = GST_CLOCK_TIME_NONE;
   overlay->text_linked = FALSE;
 
   overlay->composition = NULL;
@@ -2399,6 +2401,18 @@ gst_base_text_overlay_text_event (GstPad * pad, GstObject * parent,
   GST_LOG_OBJECT (pad, "received event %s", GST_EVENT_TYPE_NAME (event));
 
   switch (GST_EVENT_TYPE (event)) {
+    case GST_EVENT_STREAM_START:
+      /* Clear any pending EOS and segment on a new stream start */
+      GST_BASE_TEXT_OVERLAY_LOCK (overlay);
+      GST_INFO_OBJECT (overlay, "text stream-start");
+      overlay->text_flushing = FALSE;
+      overlay->text_eos = FALSE;
+      gst_base_text_overlay_pop_text (overlay);
+      gst_segment_init (&overlay->text_segment, GST_FORMAT_TIME);
+      GST_BASE_TEXT_OVERLAY_UNLOCK (overlay);
+      gst_event_unref (event);
+      ret = TRUE;
+      break;
     case GST_EVENT_CAPS:
     {
       GstCaps *caps;
@@ -2509,6 +2523,16 @@ gst_base_text_overlay_video_event (GstPad * pad, GstObject * parent,
   GST_DEBUG_OBJECT (pad, "received event %s", GST_EVENT_TYPE_NAME (event));
 
   switch (GST_EVENT_TYPE (event)) {
+    case GST_EVENT_STREAM_START:
+      /* Clear any EOS and segment on a new stream */
+      GST_BASE_TEXT_OVERLAY_LOCK (overlay);
+      GST_INFO_OBJECT (overlay, "video stream-start");
+      overlay->video_flushing = FALSE;
+      overlay->video_eos = FALSE;
+      gst_segment_init (&overlay->segment, GST_FORMAT_TIME);
+      GST_BASE_TEXT_OVERLAY_UNLOCK (overlay);
+      ret = gst_pad_event_default (pad, parent, event);
+      break;
     case GST_EVENT_CAPS:
     {
       GstCaps *caps;
@@ -2610,6 +2634,8 @@ gst_base_text_overlay_pop_text (GstBaseTextOverlay * overlay)
         overlay->text_buffer);
     gst_buffer_unref (overlay->text_buffer);
     overlay->text_buffer = NULL;
+    overlay->text_buffer_running_time = overlay->text_buffer_running_time_end =
+        GST_CLOCK_TIME_NONE;
   }
 
   /* Let the text task know we used that buffer */
@@ -2687,8 +2713,29 @@ gst_base_text_overlay_text_chain (GstPad * pad, GstObject * parent,
       }
     }
 
-    if (GST_BUFFER_TIMESTAMP_IS_VALID (buffer))
+    /* Calculate and store the running time for this text buffer in 
+     * the current segment. We might receive a new text pad segment
+     * event while this buffer is still active, and that would
+     * lead to incorrect running time calculations if we did it later.
+     */
+    overlay->text_buffer_running_time = overlay->text_buffer_running_time_end =
+        GST_CLOCK_TIME_NONE;
+
+    if (GST_BUFFER_TIMESTAMP_IS_VALID (buffer)) {
+      GstClockTime text_start = GST_BUFFER_TIMESTAMP (buffer);
+
       overlay->text_segment.position = clip_start;
+      overlay->text_buffer_running_time =
+          gst_segment_to_running_time (&overlay->text_segment,
+          GST_FORMAT_TIME, text_start);
+
+      if (GST_BUFFER_DURATION_IS_VALID (buffer)) {
+        GstClockTime text_end = text_start + GST_BUFFER_DURATION (buffer);
+        overlay->text_buffer_running_time_end =
+            gst_segment_to_running_time (&overlay->text_segment,
+            GST_FORMAT_TIME, text_end);
+      }
+    }
 
     overlay->text_buffer = buffer;      /* pass ownership of @buffer */
     buffer = NULL;
@@ -2834,23 +2881,19 @@ wait_for_text_buf:
     /* Text pad linked, check if we have a text buffer queued */
     if (overlay->text_buffer) {
       gboolean pop_text = FALSE, valid_text_time = TRUE;
-      GstClockTime text_start = GST_CLOCK_TIME_NONE;
-      GstClockTime text_end = GST_CLOCK_TIME_NONE;
-      GstClockTime text_running_time = GST_CLOCK_TIME_NONE;
-      GstClockTime text_running_time_end = GST_CLOCK_TIME_NONE;
+      GstClockTime text_running_time = overlay->text_buffer_running_time;
+      GstClockTime text_running_time_end =
+          overlay->text_buffer_running_time_end;
       GstClockTime vid_running_time, vid_running_time_end;
 
       /* if the text buffer isn't stamped right, pop it off the
        * queue and display it for the current video frame only */
-      if (!GST_BUFFER_TIMESTAMP_IS_VALID (overlay->text_buffer) ||
-          !GST_BUFFER_DURATION_IS_VALID (overlay->text_buffer)) {
+      if (!GST_CLOCK_TIME_IS_VALID (overlay->text_buffer_running_time) ||
+          !GST_CLOCK_TIME_IS_VALID (overlay->text_buffer_running_time_end)) {
         GST_WARNING_OBJECT (overlay,
             "Got text buffer with invalid timestamp or duration");
         pop_text = TRUE;
         valid_text_time = FALSE;
-      } else {
-        text_start = GST_BUFFER_TIMESTAMP (overlay->text_buffer);
-        text_end = text_start + GST_BUFFER_DURATION (overlay->text_buffer);
       }
 
       vid_running_time =
@@ -2860,16 +2903,6 @@ wait_for_text_buf:
           gst_segment_to_running_time (&overlay->segment, GST_FORMAT_TIME,
           stop);
 
-      /* If timestamp and duration are valid */
-      if (valid_text_time) {
-        text_running_time =
-            gst_segment_to_running_time (&overlay->text_segment,
-            GST_FORMAT_TIME, text_start);
-        text_running_time_end =
-            gst_segment_to_running_time (&overlay->text_segment,
-            GST_FORMAT_TIME, text_end);
-      }
-
       GST_LOG_OBJECT (overlay, "T: %" GST_TIME_FORMAT " - %" GST_TIME_FORMAT,
           GST_TIME_ARGS (text_running_time),
           GST_TIME_ARGS (text_running_time_end));
index d3eea68..86cc01c 100644 (file)
@@ -149,6 +149,8 @@ struct _GstBaseTextOverlay {
     GstSegment               segment;
     GstSegment               text_segment;
     GstBuffer               *text_buffer;
+    GstClockTime             text_buffer_running_time;
+    GstClockTime             text_buffer_running_time_end;
     gboolean                 text_linked;
     gboolean                 video_flushing;
     gboolean                 video_eos;