cccombiner: stop attaching caption buffers when caption pad has gone EOS

When schedule is true (as is the case by default), we insert padding
when no caption data is present in the schedule queue, and previously
weren't checking whether the caption pad had gone EOS, leading to
infinite scheduling of padding after EOS on the caption pad.

Rectify that by adding a "drain" parameter to dequeue_caption()

In addition, update the captions_and_eos test to push valid cc_data
in: without this cccombiner was attaching padding buffers it had
generated itself, and with that patch would now stop attaching
said padding to the second buffer. By pushing valid, non-padding
cc_data we ensure a caption buffer is indeed attached to the first
and second video buffers.

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

diff --git a/subprojects/gst-plugins-bad/ext/closedcaption/gstcccombiner.c b/subprojects/gst-plugins-bad/ext/closedcaption/gstcccombiner.c
index 07d073e..e7f230d 100644 (file)
--- a/subprojects/gst-plugins-bad/ext/closedcaption/gstcccombiner.c
+++ b/subprojects/gst-plugins-bad/ext/closedcaption/gstcccombiner.c
@@ -683,7 +683,8 @@ schedule_caption (GstCCCombiner * self, GstBuffer * caption_buf,
 }
 
 static void
-dequeue_caption (GstCCCombiner * self, const GstVideoTimeCode * tc, guint field)
+dequeue_caption (GstCCCombiner * self, const GstVideoTimeCode * tc, guint field,
+    gboolean drain)
 {
   CaptionQueueItem *scheduled;
   CaptionData caption_data;
@@ -692,7 +693,7 @@ dequeue_caption (GstCCCombiner * self, const GstVideoTimeCode * tc, guint field)
     caption_data.buffer = scheduled->buffer;
     caption_data.caption_type = self->caption_type;
     g_array_append_val (self->current_frame_captions, caption_data);
-  } else {
+  } else if (!drain) {
     caption_data.caption_type = self->caption_type;
     caption_data.buffer = make_padding (self, tc, field);
     g_array_append_val (self->current_frame_captions, caption_data);
@@ -708,6 +709,7 @@ gst_cc_combiner_collect_captions (GstCCCombiner * self, gboolean timeout)
   GstBuffer *video_buf;
   GstVideoTimeCodeMeta *tc_meta;
   GstVideoTimeCode *tc = NULL;
+  gboolean caption_pad_is_eos = FALSE;
 
   g_assert (self->current_video_buffer != NULL);
 
@@ -741,6 +743,8 @@ gst_cc_combiner_collect_captions (GstCCCombiner * self, gboolean timeout)
     if (!caption_buf) {
       if (gst_aggregator_pad_is_eos (caption_pad)) {
         GST_DEBUG_OBJECT (self, "Caption pad is EOS, we're done");
+
+        caption_pad_is_eos = TRUE;
         break;
       } else if (!timeout) {
         GST_DEBUG_OBJECT (self, "Need more caption data");
@@ -854,10 +858,10 @@ gst_cc_combiner_collect_captions (GstCCCombiner * self, gboolean timeout)
         if (GST_BUFFER_FLAG_IS_SET (self->current_video_buffer,
                 GST_VIDEO_BUFFER_FLAG_INTERLACED)) {
           if (!GST_VIDEO_BUFFER_IS_BOTTOM_FIELD (self->current_video_buffer)) {
-            dequeue_caption (self, tc, 0);
+            dequeue_caption (self, tc, 0, caption_pad_is_eos);
           }
         } else {
-          dequeue_caption (self, tc, 0);
+          dequeue_caption (self, tc, 0, caption_pad_is_eos);
         }
         break;
       }
@@ -865,33 +869,33 @@ gst_cc_combiner_collect_captions (GstCCCombiner * self, gboolean timeout)
       case GST_VIDEO_CAPTION_TYPE_CEA608_S334_1A:
       {
         if (self->progressive) {
-          dequeue_caption (self, tc, 0);
+          dequeue_caption (self, tc, 0, caption_pad_is_eos);
         } else if (GST_BUFFER_FLAG_IS_SET (self->current_video_buffer,
                 GST_VIDEO_BUFFER_FLAG_INTERLACED) &&
             GST_BUFFER_FLAG_IS_SET (self->current_video_buffer,
                 GST_VIDEO_BUFFER_FLAG_ONEFIELD)) {
           if (GST_VIDEO_BUFFER_IS_TOP_FIELD (self->current_video_buffer)) {
-            dequeue_caption (self, tc, 0);
+            dequeue_caption (self, tc, 0, caption_pad_is_eos);
           } else {
-            dequeue_caption (self, tc, 1);
+            dequeue_caption (self, tc, 1, caption_pad_is_eos);
           }
         } else {
-          dequeue_caption (self, tc, 0);
-          dequeue_caption (self, tc, 1);
+          dequeue_caption (self, tc, 0, caption_pad_is_eos);
+          dequeue_caption (self, tc, 1, caption_pad_is_eos);
         }
         break;
       }
       case GST_VIDEO_CAPTION_TYPE_CEA608_RAW:
       {
         if (self->progressive) {
-          dequeue_caption (self, tc, 0);
+          dequeue_caption (self, tc, 0, caption_pad_is_eos);
         } else if (GST_BUFFER_FLAG_IS_SET (self->current_video_buffer,
                 GST_VIDEO_BUFFER_FLAG_INTERLACED)) {
           if (!GST_VIDEO_BUFFER_IS_BOTTOM_FIELD (self->current_video_buffer)) {
-            dequeue_caption (self, tc, 0);
+            dequeue_caption (self, tc, 0, caption_pad_is_eos);
           }
         } else {
-          dequeue_caption (self, tc, 0);
+          dequeue_caption (self, tc, 0, caption_pad_is_eos);
         }
         break;
       }

diff --git a/subprojects/gst-plugins-bad/tests/check/elements/cccombiner.c b/subprojects/gst-plugins-bad/tests/check/elements/cccombiner.c
index cb7c809..1cce935 100644 (file)
--- a/subprojects/gst-plugins-bad/tests/check/elements/cccombiner.c
+++ b/subprojects/gst-plugins-bad/tests/check/elements/cccombiner.c
@@ -103,7 +103,7 @@ GST_START_TEST (captions_and_eos)
   GstCaps *caps;
   GstVideoCaptionMeta *meta;
   GstBuffer *second_video_buf, *second_caption_buf;
-  const guint8 cc_data[3] = { 0x0, 0x0, 0x0 };
+  const guint8 cc_data[3] = { 0xfc, 0x20, 0x20 };
 
   h = gst_harness_new_with_padnames ("cccombiner", "sink", "src");
   h2 = gst_harness_new_with_element (h->element, NULL, NULL);