From fa1805d531fb0045b2ae981a8ddc5590eb5af476 Mon Sep 17 00:00:00 2001 From: Mathieu Duponchelle Date: Tue, 26 Oct 2021 01:09:58 +0200 Subject: [PATCH] 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: --- .../ext/closedcaption/gstcccombiner.c | 28 +++++++++++-------- .../tests/check/elements/cccombiner.c | 2 +- 2 files changed, 17 insertions(+), 13 deletions(-) diff --git a/subprojects/gst-plugins-bad/ext/closedcaption/gstcccombiner.c b/subprojects/gst-plugins-bad/ext/closedcaption/gstcccombiner.c index 07d073e98b..e7f230dc6a 100644 --- 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 cb7c809fdc..1cce935bab 100644 --- 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);