* [FFmpeg-devel] [PATCH v4 1/6] ccfifo: Properly handle CEA-708 captions through framerate conversion
2023-04-28 16:37 [FFmpeg-devel] [PATCH v4 0/6] Add support for Closed Caption FIFO Devin Heitmueller
@ 2023-04-28 16:37 ` Devin Heitmueller
2023-05-03 9:20 ` Anton Khirnov
2023-04-28 16:37 ` [FFmpeg-devel] [PATCH v4 2/6] vf_fps: properly preserve CEA-708 captions Devin Heitmueller
` (4 subsequent siblings)
5 siblings, 1 reply; 14+ messages in thread
From: Devin Heitmueller @ 2023-04-28 16:37 UTC (permalink / raw)
To: ffmpeg-devel; +Cc: Devin Heitmueller
When transcoding video that contains 708 closed captions, the
caption data is tied to the frames as side data. Simply dropping
or adding frames to change the framerate will result in loss of
data, so the caption data needs to be preserved and reformatted.
For example, without this patch converting 720p59 to 1080i59
would result in loss of 50% of the caption bytes, resulting in
garbled 608 captions and 708 probably wouldn't render at all.
Further, the frames that are there will have an illegal
cc_count for the target framerate, so some decoders may ignore
the packets entirely.
Extract the 608 and 708 tuples and insert them onto queues. Then
after dropping/adding frames, re-write the tuples back into the
resulting frames at the appropriate rate given the target
framerate. This includes both having the correct cc_count as
well as clocking out the 608 pairs at the appropriate rate.
Thanks for Lance Wang <lance.lmwang@gmail.com> for providing
review/feedback.
Signed-off-by: Devin Heitmueller <dheitmueller@ltnglobal.com>
---
libavfilter/Makefile | 1 +
libavfilter/ccfifo.c | 240 +++++++++++++++++++++++++++++++++++++++++++++++++++
libavfilter/ccfifo.h | 94 ++++++++++++++++++++
3 files changed, 335 insertions(+)
create mode 100644 libavfilter/ccfifo.c
create mode 100644 libavfilter/ccfifo.h
diff --git a/libavfilter/Makefile b/libavfilter/Makefile
index 71e198b..628ade8 100644
--- a/libavfilter/Makefile
+++ b/libavfilter/Makefile
@@ -14,6 +14,7 @@ OBJS = allfilters.o \
buffersink.o \
buffersrc.o \
colorspace.o \
+ ccfifo.o \
drawutils.o \
fifo.o \
formats.o \
diff --git a/libavfilter/ccfifo.c b/libavfilter/ccfifo.c
new file mode 100644
index 0000000..05a77dd
--- /dev/null
+++ b/libavfilter/ccfifo.c
@@ -0,0 +1,240 @@
+/*
+ * CEA-708 Closed Captioning FIFO
+ * Copyright (c) 2023 LTN Global Communications
+ *
+ * Author: Devin Heitmueller <dheitmueller@ltnglobal.com>
+ *
+ * This file is part of FFmpeg.
+ *
+ * FFmpeg is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 2.1 of the License, or (at your option) any later version.
+ *
+ * FFmpeg is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with FFmpeg; if not, write to the Free Software
+ * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
+ */
+
+#include "ccfifo.h"
+
+struct AVCCFifo {
+ AVFifo *cc_608_fifo;
+ AVFifo *cc_708_fifo;
+ int expected_cc_count;
+ int expected_608;
+ int cc_detected;
+ int passthrough;
+ void *log_ctx;
+};
+
+#define MAX_CC_ELEMENTS 128
+#define CC_BYTES_PER_ENTRY 3
+
+struct cc_lookup {
+ int num;
+ int den;
+ int cc_count;
+ int num_608;
+};
+
+const static struct cc_lookup cc_lookup_vals[] = {
+ { 15, 1, 40, 4 },
+ { 24, 1, 25, 3 },
+ { 24000, 1001, 25, 3 },
+ { 30, 1, 20, 2 },
+ { 30000, 1001, 20, 2},
+ { 60, 1, 10, 1 },
+ { 60000, 1001, 10, 1},
+};
+
+void ff_ccfifo_freep(AVCCFifo **ccf)
+{
+ if (ccf && *ccf) {
+ AVCCFifo *tmp = *ccf;
+ if (tmp->cc_608_fifo)
+ av_fifo_freep2(&tmp->cc_608_fifo);
+ if (tmp->cc_708_fifo)
+ av_fifo_freep2(&tmp->cc_708_fifo);
+ av_freep(*ccf);
+ }
+}
+
+AVCCFifo *ff_ccfifo_alloc(AVRational *framerate, void *log_ctx)
+{
+ AVCCFifo *ccf;
+ int i;
+
+ ccf = av_mallocz(sizeof(*ccf));
+ if (!ccf)
+ return NULL;
+
+ if (!(ccf->cc_708_fifo = av_fifo_alloc2(MAX_CC_ELEMENTS, CC_BYTES_PER_ENTRY, 0)))
+ goto error;
+
+ if (!(ccf->cc_608_fifo = av_fifo_alloc2(MAX_CC_ELEMENTS, CC_BYTES_PER_ENTRY, 0)))
+ goto error;
+
+ /* Based on the target FPS, figure out the expected cc_count and number of
+ 608 tuples per packet. See ANSI/CTA-708-E Sec 4.3.6.1. */
+ for (i = 0; i < FF_ARRAY_ELEMS(cc_lookup_vals); i++) {
+ if (framerate->num == cc_lookup_vals[i].num &&
+ framerate->den == cc_lookup_vals[i].den) {
+ ccf->expected_cc_count = cc_lookup_vals[i].cc_count;
+ ccf->expected_608 = cc_lookup_vals[i].num_608;
+ break;
+ }
+ }
+
+ if (ccf->expected_608 == 0) {
+ /* We didn't find an output frame we support. We'll let the call succeed
+ and the FIFO to be allocated, but the extract/inject functions will simply
+ leave everything the way it is */
+ av_log(ccf->log_ctx, AV_LOG_WARNING, "cc_fifo cannot transcode captions fps=%d/%d\n",
+ framerate->num, framerate->den);
+ ccf->passthrough = 1;
+ }
+
+ return ccf;
+
+error:
+ ff_ccfifo_freep(&ccf);
+ return NULL;
+}
+
+int ff_ccfifo_injectbytes(AVCCFifo *ccf, uint8_t **data, size_t *len)
+{
+ char *cc_data;
+ int cc_filled = 0;
+ int i;
+
+ if (!ccf)
+ return AVERROR(EINVAL);
+
+ if (ccf->passthrough) {
+ *data = NULL;
+ *len = 0;
+ return 0;
+ }
+
+ cc_data = av_mallocz(ccf->expected_cc_count * CC_BYTES_PER_ENTRY);
+ if (!cc_data) {
+ return AVERROR(ENOMEM);
+ }
+
+ for (i = 0; i < ccf->expected_608; i++) {
+ if (av_fifo_can_read(ccf->cc_608_fifo) >= CC_BYTES_PER_ENTRY) {
+ av_fifo_read(ccf->cc_608_fifo, &cc_data[cc_filled * CC_BYTES_PER_ENTRY],
+ CC_BYTES_PER_ENTRY);
+ cc_filled++;
+ } else {
+ break;
+ }
+ }
+
+ /* Insert any available data from the 708 FIFO */
+ while (cc_filled < ccf->expected_cc_count) {
+ if (av_fifo_can_read(ccf->cc_708_fifo) >= CC_BYTES_PER_ENTRY) {
+ av_fifo_read(ccf->cc_708_fifo, &cc_data[cc_filled * CC_BYTES_PER_ENTRY],
+ CC_BYTES_PER_ENTRY);
+ cc_filled++;
+ } else {
+ break;
+ }
+ }
+
+ /* Insert 708 padding into any remaining fields */
+ while (cc_filled < ccf->expected_cc_count) {
+ cc_data[cc_filled * CC_BYTES_PER_ENTRY] = 0xfa;
+ cc_data[cc_filled * CC_BYTES_PER_ENTRY + 1] = 0x00;
+ cc_data[cc_filled * CC_BYTES_PER_ENTRY + 2] = 0x00;
+ cc_filled++;
+ }
+
+ *data = cc_data;
+ *len = ccf->expected_cc_count * CC_BYTES_PER_ENTRY;
+ return 0;
+}
+
+int ff_ccfifo_inject(AVCCFifo *ccf, AVFrame *frame)
+{
+ AVFrameSideData *sd;
+ uint8_t *cc_data;
+ size_t cc_size;
+ int ret;
+
+ if (!ccf)
+ return AVERROR(EINVAL);
+
+ if (ccf->passthrough == 1 || ccf->cc_detected == 0 || ccf->expected_cc_count == 0)
+ return 0;
+
+ ret = ff_ccfifo_injectbytes(ccf, &cc_data, &cc_size);
+ if (ret == 0) {
+ sd = av_frame_new_side_data(frame, AV_FRAME_DATA_A53_CC, cc_size);
+ if (!sd) {
+ av_freep(&cc_data);
+ return AVERROR(ENOMEM);
+ }
+ memcpy(sd->data, cc_data, cc_size);
+ av_freep(&cc_data);
+ }
+
+ return 0;
+}
+
+int ff_ccfifo_extractbytes(AVCCFifo *ccf, uint8_t *cc_bytes, size_t len)
+{
+ int cc_count = len / CC_BYTES_PER_ENTRY;
+
+ if (!ccf)
+ return AVERROR(EINVAL);
+
+ if (ccf->passthrough == 1)
+ return 0;
+
+ ccf->cc_detected = 1;
+
+ for (int i = 0; i < cc_count; i++) {
+ /* See ANSI/CTA-708-E Sec 4.3, Table 3 */
+ uint8_t cc_valid = (cc_bytes[CC_BYTES_PER_ENTRY*i] & 0x04) >> 2;
+ uint8_t cc_type = cc_bytes[CC_BYTES_PER_ENTRY*i] & 0x03;
+ if (cc_type == 0x00 || cc_type == 0x01) {
+ av_fifo_write(ccf->cc_608_fifo, &cc_bytes[CC_BYTES_PER_ENTRY*i],
+ CC_BYTES_PER_ENTRY);
+ } else if (cc_valid && (cc_type == 0x02 || cc_type == 0x03)) {
+ av_fifo_write(ccf->cc_708_fifo, &cc_bytes[CC_BYTES_PER_ENTRY*i],
+ CC_BYTES_PER_ENTRY);
+ }
+ }
+ return 0;
+}
+
+int ff_ccfifo_extract(AVCCFifo *ccf, AVFrame *frame)
+{
+ if (!ccf)
+ return AVERROR(EINVAL);
+
+ if (ccf->passthrough == 1)
+ return 0;
+
+ /* Read the A53 side data, discard padding, and put 608/708 into
+ queues so we can ensure they get into the output frames at
+ the correct rate... */
+ if (ccf->expected_cc_count > 0) {
+ AVFrameSideData *side_data = av_frame_get_side_data(frame, AV_FRAME_DATA_A53_CC);
+ if (side_data) {
+ ff_ccfifo_extractbytes(ccf, side_data->data, side_data->size);
+
+ /* Remove the side data, as we will re-create it on the
+ output as needed */
+ av_frame_remove_side_data(frame, AV_FRAME_DATA_A53_CC);
+ }
+ }
+ return 0;
+}
diff --git a/libavfilter/ccfifo.h b/libavfilter/ccfifo.h
new file mode 100644
index 0000000..baa4ae1
--- /dev/null
+++ b/libavfilter/ccfifo.h
@@ -0,0 +1,94 @@
+/*
+ * CEA-708 Closed Captioning FIFO
+ * Copyright (c) 2023 LTN Global Communications
+ *
+ * Author: Devin Heitmueller <dheitmueller@ltnglobal.com>
+ *
+ * This file is part of FFmpeg.
+ *
+ * FFmpeg is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 2.1 of the License, or (at your option) any later version.
+ *
+ * FFmpeg is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with FFmpeg; if not, write to the Free Software
+ * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
+ */
+
+/**
+ * @file
+ * CC FIFO Buffer
+ */
+
+#ifndef AVFILTER_CCFIFO_H
+#define AVFILTER_CCFIFO_H
+
+#include "libavutil/avutil.h"
+#include "libavutil/frame.h"
+#include "libavutil/fifo.h"
+
+typedef struct AVCCFifo AVCCFifo;
+
+/**
+ * Allocate an AVCCFifo.
+ *
+ * @param framerate output framerate
+ * @param log_ctx used for any av_log() calls
+ * @return newly allocated AVCCFifo, or NULL on error
+ */
+AVCCFifo *ff_ccfifo_alloc(AVRational *framerate, void *log_ctx);
+
+/**
+ * Free an AVCCFifo
+ *
+ * @param ccf Pointer to the pointer to the AVCCFifo which should be freed
+ * @note `*ptr = NULL` is safe and leads to no action.
+ */
+void ff_ccfifo_freep(AVCCFifo **ccf);
+
+
+/**
+ * Extract CC data from an AVFrame
+ *
+ * Extract CC bytes from the AVFrame, insert them into our queue, and
+ * remove the side data from the AVFrame. The side data is removed
+ * as it will be re-inserted at the appropriate rate later in the
+ * filter.
+ *
+ * @param af AVCCFifo to write to
+ * @param frame AVFrame with the video frame to operate on
+ * @return Zero on success, or negative AVERROR
+ * code on failure.
+ */
+int ff_ccfifo_extract(AVCCFifo *ccf, AVFrame *frame);
+
+/**
+ *Just like ff_ccfifo_extract(), but takes the raw bytes instead of an AVFrame
+ */
+int ff_ccfifo_extractbytes(AVCCFifo *ccf, uint8_t *data, size_t len);
+
+/**
+ * Insert CC data from the FIFO into an AVFrame (as side data)
+ *
+ * Dequeue the appropriate number of CC tuples based on the
+ * frame rate, and insert them into the AVFrame
+ *
+ * @param af AVCCFifo to read from
+ * @param frame AVFrame with the video frame to operate on
+ * @return Zero on success, or negative AVERROR
+ * code on failure.
+ */
+int ff_ccfifo_inject(AVCCFifo *ccf, AVFrame *frame);
+
+/* Just like ff_ccfifo_inject(), but takes the raw bytes instead of an AVFrame
+ * Note: the caller is responsible for calling av_freep() against the value
+ * returned in the "data" argument */
+int ff_ccfifo_injectbytes(AVCCFifo *ccf, uint8_t **data, size_t *len);
+
+#endif /* AVFILTER_CCFIFO_H */
--
1.8.3.1
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
^ permalink raw reply [flat|nested] 14+ messages in thread* Re: [FFmpeg-devel] [PATCH v4 1/6] ccfifo: Properly handle CEA-708 captions through framerate conversion
2023-04-28 16:37 ` [FFmpeg-devel] [PATCH v4 1/6] ccfifo: Properly handle CEA-708 captions through framerate conversion Devin Heitmueller
@ 2023-05-03 9:20 ` Anton Khirnov
2023-05-03 13:15 ` Devin Heitmueller
0 siblings, 1 reply; 14+ messages in thread
From: Anton Khirnov @ 2023-05-03 9:20 UTC (permalink / raw)
To: FFmpeg development discussions and patches; +Cc: Devin Heitmueller
Quoting Devin Heitmueller (2023-04-28 18:37:46)
> +void ff_ccfifo_freep(AVCCFifo **ccf)
> +{
> + if (ccf && *ccf) {
Don't check for ccf, it makes no sense to call this function with
ccf==NULL, so silently ignoring it can hide bugs.
> + AVCCFifo *tmp = *ccf;
> + if (tmp->cc_608_fifo)
> + av_fifo_freep2(&tmp->cc_608_fifo);
Useless checks, av_fifo_freep2 can be called on &NULL.
> + if (tmp->cc_708_fifo)
> + av_fifo_freep2(&tmp->cc_708_fifo);
> + av_freep(*ccf);
> + }
> +}
> +
> +AVCCFifo *ff_ccfifo_alloc(AVRational *framerate, void *log_ctx)
We typically pass AVRational directly, not as pointers. That also makes
it clear that the passed value is not modified.
> +{
> + AVCCFifo *ccf;
> + int i;
> +
> + ccf = av_mallocz(sizeof(*ccf));
> + if (!ccf)
> + return NULL;
> +
> + if (!(ccf->cc_708_fifo = av_fifo_alloc2(MAX_CC_ELEMENTS, CC_BYTES_PER_ENTRY, 0)))
> + goto error;
> +
> + if (!(ccf->cc_608_fifo = av_fifo_alloc2(MAX_CC_ELEMENTS, CC_BYTES_PER_ENTRY, 0)))
> + goto error;
> +
> + /* Based on the target FPS, figure out the expected cc_count and number of
> + 608 tuples per packet. See ANSI/CTA-708-E Sec 4.3.6.1. */
> + for (i = 0; i < FF_ARRAY_ELEMS(cc_lookup_vals); i++) {
> + if (framerate->num == cc_lookup_vals[i].num &&
> + framerate->den == cc_lookup_vals[i].den) {
> + ccf->expected_cc_count = cc_lookup_vals[i].cc_count;
> + ccf->expected_608 = cc_lookup_vals[i].num_608;
> + break;
> + }
> + }
> +
> + if (ccf->expected_608 == 0) {
> + /* We didn't find an output frame we support. We'll let the call succeed
> + and the FIFO to be allocated, but the extract/inject functions will simply
> + leave everything the way it is */
> + av_log(ccf->log_ctx, AV_LOG_WARNING, "cc_fifo cannot transcode captions fps=%d/%d\n",
> + framerate->num, framerate->den);
Won't this result in spurious warnings for users who are just converting
framerates and don't have any captions. If so it should probably be
printed the first time we actually see caption side data.
> + ccf->passthrough = 1;
> + }
> +
> + return ccf;
> +
> +error:
> + ff_ccfifo_freep(&ccf);
> + return NULL;
> +}
> +
> +int ff_ccfifo_injectbytes(AVCCFifo *ccf, uint8_t **data, size_t *len)
> +{
> + char *cc_data;
> + int cc_filled = 0;
> + int i;
> +
> + if (!ccf)
> + return AVERROR(EINVAL);
For all the extract/inject functions: it should be invalid to call them
with a NULL context, so this should be an av_assert0() or not be here at
all.
> +
> + if (ccf->passthrough) {
> + *data = NULL;
> + *len = 0;
> + return 0;
> + }
> +
> + cc_data = av_mallocz(ccf->expected_cc_count * CC_BYTES_PER_ENTRY);
This buffer size is constant for a given AVCCFifo object, so perhaps
ff_ccfifo_alloc() could return required buffer size and this function
could write into a user-provided buffer and avoid constant dynamic
allocation.
> + if (!cc_data) {
> + return AVERROR(ENOMEM);
> + }
> +
> + for (i = 0; i < ccf->expected_608; i++) {
> + if (av_fifo_can_read(ccf->cc_608_fifo) >= CC_BYTES_PER_ENTRY) {
> + av_fifo_read(ccf->cc_608_fifo, &cc_data[cc_filled * CC_BYTES_PER_ENTRY],
> + CC_BYTES_PER_ENTRY);
This looks wrong, as fifo operations are in elements, and your
elements are already CC_BYTES_PER_ENTRY. So every read actually writes
CC_BYTES_PER_ENTRY * CC_BYTES_PER_ENTRY bytes to cc_data.
I think you can also do this as a single av_fifo_read() call without a
loop.
--
Anton Khirnov
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
^ permalink raw reply [flat|nested] 14+ messages in thread* Re: [FFmpeg-devel] [PATCH v4 1/6] ccfifo: Properly handle CEA-708 captions through framerate conversion
2023-05-03 9:20 ` Anton Khirnov
@ 2023-05-03 13:15 ` Devin Heitmueller
0 siblings, 0 replies; 14+ messages in thread
From: Devin Heitmueller @ 2023-05-03 13:15 UTC (permalink / raw)
To: FFmpeg development discussions and patches, Devin Heitmueller
Hi Anton,
Thanks for your feedback. Comments inline:
On Wed, May 3, 2023 at 5:20 AM Anton Khirnov <anton@khirnov.net> wrote:
>
> Quoting Devin Heitmueller (2023-04-28 18:37:46)
> > +void ff_ccfifo_freep(AVCCFifo **ccf)
> > +{
> > + if (ccf && *ccf) {
>
> Don't check for ccf, it makes no sense to call this function with
> ccf==NULL, so silently ignoring it can hide bugs.
Ok.
> > + AVCCFifo *tmp = *ccf;
> > + if (tmp->cc_608_fifo)
> > + av_fifo_freep2(&tmp->cc_608_fifo);
>
> Useless checks, av_fifo_freep2 can be called on &NULL.
Ok.
> > + if (tmp->cc_708_fifo)
> > + av_fifo_freep2(&tmp->cc_708_fifo);
> > + av_freep(*ccf);
> > + }
> > +}
> > +
> > +AVCCFifo *ff_ccfifo_alloc(AVRational *framerate, void *log_ctx)
>
> We typically pass AVRational directly, not as pointers. That also makes
> it clear that the passed value is not modified.
Ok.
> > +{
> > + AVCCFifo *ccf;
> > + int i;
> > +
> > + ccf = av_mallocz(sizeof(*ccf));
> > + if (!ccf)
> > + return NULL;
> > +
> > + if (!(ccf->cc_708_fifo = av_fifo_alloc2(MAX_CC_ELEMENTS, CC_BYTES_PER_ENTRY, 0)))
> > + goto error;
> > +
> > + if (!(ccf->cc_608_fifo = av_fifo_alloc2(MAX_CC_ELEMENTS, CC_BYTES_PER_ENTRY, 0)))
> > + goto error;
> > +
> > + /* Based on the target FPS, figure out the expected cc_count and number of
> > + 608 tuples per packet. See ANSI/CTA-708-E Sec 4.3.6.1. */
> > + for (i = 0; i < FF_ARRAY_ELEMS(cc_lookup_vals); i++) {
> > + if (framerate->num == cc_lookup_vals[i].num &&
> > + framerate->den == cc_lookup_vals[i].den) {
> > + ccf->expected_cc_count = cc_lookup_vals[i].cc_count;
> > + ccf->expected_608 = cc_lookup_vals[i].num_608;
> > + break;
> > + }
> > + }
> > +
> > + if (ccf->expected_608 == 0) {
> > + /* We didn't find an output frame we support. We'll let the call succeed
> > + and the FIFO to be allocated, but the extract/inject functions will simply
> > + leave everything the way it is */
> > + av_log(ccf->log_ctx, AV_LOG_WARNING, "cc_fifo cannot transcode captions fps=%d/%d\n",
> > + framerate->num, framerate->den);
>
> Won't this result in spurious warnings for users who are just converting
> framerates and don't have any captions. If so it should probably be
> printed the first time we actually see caption side data.
Yeah, I see your point. Let me investigate further and see what I can do here.
> > + ccf->passthrough = 1;
> > + }
> > +
> > + return ccf;
> > +
> > +error:
> > + ff_ccfifo_freep(&ccf);
> > + return NULL;
> > +}
> > +
> > +int ff_ccfifo_injectbytes(AVCCFifo *ccf, uint8_t **data, size_t *len)
> > +{
> > + char *cc_data;
> > + int cc_filled = 0;
> > + int i;
> > +
> > + if (!ccf)
> > + return AVERROR(EINVAL);
>
> For all the extract/inject functions: it should be invalid to call them
> with a NULL context, so this should be an av_assert0() or not be here at
> all.
Ok.
> > +
> > + if (ccf->passthrough) {
> > + *data = NULL;
> > + *len = 0;
> > + return 0;
> > + }
> > +
> > + cc_data = av_mallocz(ccf->expected_cc_count * CC_BYTES_PER_ENTRY);
>
> This buffer size is constant for a given AVCCFifo object, so perhaps
> ff_ccfifo_alloc() could return required buffer size and this function
> could write into a user-provided buffer and avoid constant dynamic
> allocation.
So I had previously considered the approach you suggested, but decided
against it. This is because while the AVCCFifo today always returns
the same number of bytes, there are cases where cc_count could vary on
a per frame basis (and thus the buffer size differs). In particular
with 30i with 3:2 pulldown the cc_count alternates between 20 and 30.
See CTA-708-E R-2018 Sec 4.6.3.1.
Having the queue return a properly sized buffer avoids situations
where you make a call to get the size and then make a separate call to
fill the buffer (where the size might not be correct).
I don't particularly love the approach, given it means callers have to
memcpy() the result into the final buffer and then free the buffer
created by the call to inject. But it seemed like the better approach
given the issue described above.
I guess because the API is private and we don't support that feature
today, we could do it as you described and then change the API later.
Suggestions welcome.
> > + if (!cc_data) {
> > + return AVERROR(ENOMEM);
> > + }
> > +
> > + for (i = 0; i < ccf->expected_608; i++) {
> > + if (av_fifo_can_read(ccf->cc_608_fifo) >= CC_BYTES_PER_ENTRY) {
> > + av_fifo_read(ccf->cc_608_fifo, &cc_data[cc_filled * CC_BYTES_PER_ENTRY],
> > + CC_BYTES_PER_ENTRY);
>
> This looks wrong, as fifo operations are in elements, and your
> elements are already CC_BYTES_PER_ENTRY. So every read actually writes
> CC_BYTES_PER_ENTRY * CC_BYTES_PER_ENTRY bytes to cc_data.
>
> I think you can also do this as a single av_fifo_read() call without a
> loop.
I'll review the logic above based on your comments and if appropriate
rework the loop.
Thanks,
Devin
--
Devin Heitmueller, Senior Software Engineer
LTN Global Communications
o: +1 (301) 363-1001
w: https://ltnglobal.com e: devin.heitmueller@ltnglobal.com
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
^ permalink raw reply [flat|nested] 14+ messages in thread
* [FFmpeg-devel] [PATCH v4 2/6] vf_fps: properly preserve CEA-708 captions
2023-04-28 16:37 [FFmpeg-devel] [PATCH v4 0/6] Add support for Closed Caption FIFO Devin Heitmueller
2023-04-28 16:37 ` [FFmpeg-devel] [PATCH v4 1/6] ccfifo: Properly handle CEA-708 captions through framerate conversion Devin Heitmueller
@ 2023-04-28 16:37 ` Devin Heitmueller
2023-04-28 16:37 ` [FFmpeg-devel] [PATCH v4 3/6] yadif: Properly preserve CEA-708 closed captions Devin Heitmueller
` (3 subsequent siblings)
5 siblings, 0 replies; 14+ messages in thread
From: Devin Heitmueller @ 2023-04-28 16:37 UTC (permalink / raw)
To: ffmpeg-devel; +Cc: Devin Heitmueller
The existing implementation made an attempt to remove duplicate
captions if increasing the framerate, but made no attempt to
handle reducing the framerate, nor did it rewrite the caption
payloads to have the appropriate cc_count (e.g. the cc_count needs
to change from 20 to 10 when going from 1080i59 to 720p59 and
vice-versa).
Make use of the new ccfifo mechanism to ensure that caption data
is properly preserved.
Signed-off-by: Devin Heitmueller <dheitmueller@ltnglobal.com>
---
libavfilter/vf_fps.c | 9 ++++++++-
1 file changed, 8 insertions(+), 1 deletion(-)
diff --git a/libavfilter/vf_fps.c b/libavfilter/vf_fps.c
index 051d278..19c727e 100644
--- a/libavfilter/vf_fps.c
+++ b/libavfilter/vf_fps.c
@@ -34,6 +34,7 @@
#include "libavutil/mathematics.h"
#include "libavutil/opt.h"
#include "avfilter.h"
+#include "ccfifo.h"
#include "filters.h"
#include "internal.h"
@@ -85,6 +86,7 @@ typedef struct FPSContext {
AVFrame *frames[2]; ///< buffered frames
int frames_count; ///< number of buffered frames
+ AVCCFifo *cc_fifo; ///< closed captions
int64_t next_pts; ///< pts of the next frame to output
@@ -165,6 +167,7 @@ static av_cold void uninit(AVFilterContext *ctx)
frame = shift_frame(ctx, s);
av_frame_free(&frame);
}
+ ff_ccfifo_freep(&s->cc_fifo);
av_log(ctx, AV_LOG_VERBOSE, "%d frames in, %d frames out; %d frames dropped, "
"%d frames duplicated.\n", s->frames_in, s->frames_out, s->drop, s->dup);
@@ -210,6 +213,9 @@ static int config_props(AVFilterLink* outlink)
s->in_pts_off, s->out_pts_off, s->start_time);
}
+ if (!(s->cc_fifo = ff_ccfifo_alloc(&outlink->frame_rate, ctx)))
+ av_log(ctx, AV_LOG_VERBOSE, "Failure to setup CC FIFO queue\n");
+
av_log(ctx, AV_LOG_VERBOSE, "fps=%d/%d\n", outlink->frame_rate.num, outlink->frame_rate.den);
return 0;
@@ -242,6 +248,7 @@ static int read_frame(AVFilterContext *ctx, FPSContext *s, AVFilterLink *inlink,
av_log(ctx, AV_LOG_DEBUG, "Read frame with in pts %"PRId64", out pts %"PRId64"\n",
in_pts, frame->pts);
+ ff_ccfifo_extract(s->cc_fifo, frame);
s->frames[s->frames_count++] = frame;
s->frames_in++;
@@ -289,7 +296,7 @@ static int write_frame(AVFilterContext *ctx, FPSContext *s, AVFilterLink *outlin
if (!frame)
return AVERROR(ENOMEM);
// Make sure Closed Captions will not be duplicated
- av_frame_remove_side_data(s->frames[0], AV_FRAME_DATA_A53_CC);
+ ff_ccfifo_inject(s->cc_fifo, frame);
frame->pts = s->next_pts++;
frame->duration = 1;
--
1.8.3.1
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
^ permalink raw reply [flat|nested] 14+ messages in thread* [FFmpeg-devel] [PATCH v4 3/6] yadif: Properly preserve CEA-708 closed captions
2023-04-28 16:37 [FFmpeg-devel] [PATCH v4 0/6] Add support for Closed Caption FIFO Devin Heitmueller
2023-04-28 16:37 ` [FFmpeg-devel] [PATCH v4 1/6] ccfifo: Properly handle CEA-708 captions through framerate conversion Devin Heitmueller
2023-04-28 16:37 ` [FFmpeg-devel] [PATCH v4 2/6] vf_fps: properly preserve CEA-708 captions Devin Heitmueller
@ 2023-04-28 16:37 ` Devin Heitmueller
2023-04-28 16:37 ` [FFmpeg-devel] [PATCH v4 4/6] tinterlace: " Devin Heitmueller
` (2 subsequent siblings)
5 siblings, 0 replies; 14+ messages in thread
From: Devin Heitmueller @ 2023-04-28 16:37 UTC (permalink / raw)
To: ffmpeg-devel; +Cc: Devin Heitmueller
Various deinterlacing modes have the effect of doubling the
framerate, and we need to ensure that the caption data isn't
duplicated (or else you get double captions on-screen).
Use the new ccfifo mechanism for yadif (and yadif_cuda and bwdif
since they use the same yadif core) so that CEA-708 data is
properly preserved through this filter.
Signed-off-by: Devin Heitmueller <dheitmueller@ltnglobal.com>
---
libavfilter/vf_bwdif.c | 7 +++++++
libavfilter/vf_yadif.c | 6 ++++++
libavfilter/vf_yadif_cuda.c | 8 ++++++++
libavfilter/yadif.h | 2 ++
libavfilter/yadif_common.c | 5 +++++
5 files changed, 28 insertions(+)
diff --git a/libavfilter/vf_bwdif.c b/libavfilter/vf_bwdif.c
index 34e8c5e..fb46fdc 100644
--- a/libavfilter/vf_bwdif.c
+++ b/libavfilter/vf_bwdif.c
@@ -297,6 +297,7 @@ static av_cold void uninit(AVFilterContext *ctx)
av_frame_free(&yadif->prev);
av_frame_free(&yadif->cur );
av_frame_free(&yadif->next);
+ ff_ccfifo_freep(&yadif->cc_fifo);
}
static const enum AVPixelFormat pix_fmts[] = {
@@ -332,6 +333,12 @@ static int config_props(AVFilterLink *link)
if(yadif->mode&1)
link->frame_rate = av_mul_q(link->src->inputs[0]->frame_rate, (AVRational){2,1});
+ else
+ link->frame_rate = ctx->inputs[0]->frame_rate;
+
+ if (!(yadif->cc_fifo = ff_ccfifo_alloc(&link->frame_rate, ctx)))
+ av_log(ctx, AV_LOG_VERBOSE, "Failure to setup CC FIFO queue\n");
+
if (link->w < 3 || link->h < 4) {
av_log(ctx, AV_LOG_ERROR, "Video of less than 3 columns or 4 lines is not supported\n");
diff --git a/libavfilter/vf_yadif.c b/libavfilter/vf_yadif.c
index 1be02de..d3b6900 100644
--- a/libavfilter/vf_yadif.c
+++ b/libavfilter/vf_yadif.c
@@ -261,6 +261,7 @@ static av_cold void uninit(AVFilterContext *ctx)
av_frame_free(&yadif->prev);
av_frame_free(&yadif->cur );
av_frame_free(&yadif->next);
+ ff_ccfifo_freep(&yadif->cc_fifo);
}
static const enum AVPixelFormat pix_fmts[] = {
@@ -293,6 +294,11 @@ static int config_output(AVFilterLink *outlink)
if(s->mode & 1)
outlink->frame_rate = av_mul_q(ctx->inputs[0]->frame_rate,
(AVRational){2, 1});
+ else
+ outlink->frame_rate = ctx->inputs[0]->frame_rate;
+
+ if (!(s->cc_fifo = ff_ccfifo_alloc(&outlink->frame_rate, ctx)))
+ av_log(ctx, AV_LOG_VERBOSE, "Failure to setup CC FIFO queue\n");
if (outlink->w < 3 || outlink->h < 3) {
av_log(ctx, AV_LOG_ERROR, "Video of less than 3 columns or lines is not supported\n");
diff --git a/libavfilter/vf_yadif_cuda.c b/libavfilter/vf_yadif_cuda.c
index 685b8a2..e6773ff 100644
--- a/libavfilter/vf_yadif_cuda.c
+++ b/libavfilter/vf_yadif_cuda.c
@@ -206,6 +206,9 @@ static av_cold void deint_cuda_uninit(AVFilterContext *ctx)
av_frame_free(&y->cur);
av_frame_free(&y->next);
+ if (yadif->cc_fifo)
+ ff_cc_fifo_free(yadif->cc_fifo);
+
av_buffer_unref(&s->device_ref);
s->hwctx = NULL;
av_buffer_unref(&s->input_frames_ref);
@@ -291,6 +294,11 @@ static int config_output(AVFilterLink *link)
if(y->mode & 1)
link->frame_rate = av_mul_q(ctx->inputs[0]->frame_rate,
(AVRational){2, 1});
+ else
+ outlink->frame_rate = ctx->inputs[0]->frame_rate;
+
+ if (!(s->cc_fifo = ff_cc_fifo_alloc(&outlink->frame_rate, ctx)))
+ av_log(ctx, AV_LOG_VERBOSE, "Failure to setup CC FIFO queue\n");
if (link->w < 3 || link->h < 3) {
av_log(ctx, AV_LOG_ERROR, "Video of less than 3 columns or lines is not supported\n");
diff --git a/libavfilter/yadif.h b/libavfilter/yadif.h
index c928911..1077576 100644
--- a/libavfilter/yadif.h
+++ b/libavfilter/yadif.h
@@ -22,6 +22,7 @@
#include "libavutil/opt.h"
#include "libavutil/pixdesc.h"
#include "avfilter.h"
+#include "ccfifo.h"
enum YADIFMode {
YADIF_MODE_SEND_FRAME = 0, ///< send 1 frame for each frame
@@ -76,6 +77,7 @@ typedef struct YADIFContext {
int eof;
uint8_t *temp_line;
int temp_line_size;
+ AVCCFifo *cc_fifo;
/*
* An algorithm that treats first and/or last fields in a sequence
diff --git a/libavfilter/yadif_common.c b/libavfilter/yadif_common.c
index a10cf7a..676853e 100644
--- a/libavfilter/yadif_common.c
+++ b/libavfilter/yadif_common.c
@@ -60,6 +60,8 @@ static int return_frame(AVFilterContext *ctx, int is_second)
yadif->out->pts = AV_NOPTS_VALUE;
}
}
+
+ ff_ccfifo_inject(yadif->cc_fifo, yadif->out);
ret = ff_filter_frame(ctx->outputs[0], yadif->out);
yadif->frame_pending = (yadif->mode&1) && !is_second;
@@ -96,6 +98,8 @@ int ff_yadif_filter_frame(AVFilterLink *link, AVFrame *frame)
av_assert0(frame);
+ ff_ccfifo_extract(yadif->cc_fifo, frame);
+
if (yadif->frame_pending)
return_frame(ctx, 1);
@@ -137,6 +141,7 @@ int ff_yadif_filter_frame(AVFilterLink *link, AVFrame *frame)
if (!yadif->out)
return AVERROR(ENOMEM);
+ ff_ccfifo_inject(yadif->cc_fifo, yadif->out);
av_frame_free(&yadif->prev);
if (yadif->out->pts != AV_NOPTS_VALUE)
yadif->out->pts *= 2;
--
1.8.3.1
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
^ permalink raw reply [flat|nested] 14+ messages in thread* [FFmpeg-devel] [PATCH v4 4/6] tinterlace: Properly preserve CEA-708 closed captions
2023-04-28 16:37 [FFmpeg-devel] [PATCH v4 0/6] Add support for Closed Caption FIFO Devin Heitmueller
` (2 preceding siblings ...)
2023-04-28 16:37 ` [FFmpeg-devel] [PATCH v4 3/6] yadif: Properly preserve CEA-708 closed captions Devin Heitmueller
@ 2023-04-28 16:37 ` Devin Heitmueller
2023-04-28 16:37 ` [FFmpeg-devel] [PATCH v4 5/6] vf_ccrepack: Add new filter to repack CEA-708 side data Devin Heitmueller
2023-04-28 16:37 ` [FFmpeg-devel] [PATCH v4 6/6] decklink_enc: add support for playout of 608 captions in MOV files Devin Heitmueller
5 siblings, 0 replies; 14+ messages in thread
From: Devin Heitmueller @ 2023-04-28 16:37 UTC (permalink / raw)
To: ffmpeg-devel; +Cc: Devin Heitmueller
Because the interlacing filter halves the effective framerate, we
need to ensure that no CEA-708 data is lost as frames are merged.
Make use of the new ccfifo mechanism to ensure that caption data
is properly preserved as frames pass through the filter.
Thanks to Thomas Mundt for review and noticing a couple of
missed codepaths for injection on output. Thanks to Lance Wang
for pointing out a memory leak.
Signed-off-by: Devin Heitmueller <dheitmueller@ltnglobal.com>
---
libavfilter/tinterlace.h | 2 ++
libavfilter/vf_tinterlace.c | 9 +++++++++
2 files changed, 11 insertions(+)
diff --git a/libavfilter/tinterlace.h b/libavfilter/tinterlace.h
index 37b6c10..9f5ce7e 100644
--- a/libavfilter/tinterlace.h
+++ b/libavfilter/tinterlace.h
@@ -32,6 +32,7 @@
#include "libavutil/pixdesc.h"
#include "drawutils.h"
#include "avfilter.h"
+#include "ccfifo.h"
#define TINTERLACE_FLAG_VLPF 01
#define TINTERLACE_FLAG_CVLPF 2
@@ -77,6 +78,7 @@ typedef struct TInterlaceContext {
const AVPixFmtDescriptor *csp;
void (*lowpass_line)(uint8_t *dstp, ptrdiff_t width, const uint8_t *srcp,
ptrdiff_t mref, ptrdiff_t pref, int clip_max);
+ AVCCFifo *cc_fifo;
} TInterlaceContext;
void ff_tinterlace_init_x86(TInterlaceContext *interlace);
diff --git a/libavfilter/vf_tinterlace.c b/libavfilter/vf_tinterlace.c
index 0326292..309bf83 100644
--- a/libavfilter/vf_tinterlace.c
+++ b/libavfilter/vf_tinterlace.c
@@ -203,6 +203,7 @@ static av_cold void uninit(AVFilterContext *ctx)
av_frame_free(&tinterlace->next);
av_freep(&tinterlace->black_data[0][0]);
av_freep(&tinterlace->black_data[1][0]);
+ ff_ccfifo_freep(&tinterlace->cc_fifo);
}
static int config_out_props(AVFilterLink *outlink)
@@ -291,6 +292,9 @@ static int config_out_props(AVFilterLink *outlink)
#endif
}
+ if (!(tinterlace->cc_fifo = ff_ccfifo_alloc(&outlink->frame_rate, ctx)))
+ av_log(ctx, AV_LOG_VERBOSE, "Failure to setup CC FIFO queue\n");
+
av_log(ctx, AV_LOG_VERBOSE, "mode:%d filter:%s h:%d -> h:%d\n", tinterlace->mode,
(tinterlace->flags & TINTERLACE_FLAG_CVLPF) ? "complex" :
(tinterlace->flags & TINTERLACE_FLAG_VLPF) ? "linear" : "off",
@@ -375,6 +379,8 @@ static int filter_frame(AVFilterLink *inlink, AVFrame *picref)
tinterlace->cur = tinterlace->next;
tinterlace->next = picref;
+ ff_ccfifo_extract(tinterlace->cc_fifo, picref);
+
cur = tinterlace->cur;
next = tinterlace->next;
/* we need at least two frames */
@@ -451,6 +457,7 @@ static int filter_frame(AVFilterLink *inlink, AVFrame *picref)
if (!out)
return AVERROR(ENOMEM);
out->pts /= 2; // adjust pts to new framerate
+ ff_ccfifo_inject(tinterlace->cc_fifo, out);
ret = ff_filter_frame(outlink, out);
return ret;
}
@@ -486,6 +493,7 @@ static int filter_frame(AVFilterLink *inlink, AVFrame *picref)
out->pts = cur->pts*2;
out->pts = av_rescale_q(out->pts, tinterlace->preout_time_base, outlink->time_base);
+ ff_ccfifo_inject(tinterlace->cc_fifo, out);
if ((ret = ff_filter_frame(outlink, out)) < 0)
return ret;
@@ -521,6 +529,7 @@ static int filter_frame(AVFilterLink *inlink, AVFrame *picref)
out->pts = av_rescale_q(out->pts, tinterlace->preout_time_base, outlink->time_base);
out->duration = av_rescale_q(1, av_inv_q(outlink->frame_rate), outlink->time_base);
+ ff_ccfifo_inject(tinterlace->cc_fifo, out);
ret = ff_filter_frame(outlink, out);
return ret;
--
1.8.3.1
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
^ permalink raw reply [flat|nested] 14+ messages in thread* [FFmpeg-devel] [PATCH v4 5/6] vf_ccrepack: Add new filter to repack CEA-708 side data
2023-04-28 16:37 [FFmpeg-devel] [PATCH v4 0/6] Add support for Closed Caption FIFO Devin Heitmueller
` (3 preceding siblings ...)
2023-04-28 16:37 ` [FFmpeg-devel] [PATCH v4 4/6] tinterlace: " Devin Heitmueller
@ 2023-04-28 16:37 ` Devin Heitmueller
2023-04-30 22:42 ` Lance Wang
2023-04-28 16:37 ` [FFmpeg-devel] [PATCH v4 6/6] decklink_enc: add support for playout of 608 captions in MOV files Devin Heitmueller
5 siblings, 1 reply; 14+ messages in thread
From: Devin Heitmueller @ 2023-04-28 16:37 UTC (permalink / raw)
To: ffmpeg-devel; +Cc: Devin Heitmueller
THis filter can correct certain issues seen from upstream sources
where the cc_count is not properly set or the CEA-608 tuples are
not at the start of the payload as expected.
Make use of the ccfifo to extract and immediately repack the CEA-708
side data, thereby removing any extra padding and ensuring the 608
tuples are at the front of the payload.
Signed-off-by: Devin Heitmueller <dheitmueller@ltnglobal.com>
---
doc/filters.texi | 10 +++++
libavfilter/Makefile | 1 +
libavfilter/allfilters.c | 1 +
libavfilter/vf_ccrepack.c | 100 ++++++++++++++++++++++++++++++++++++++++++++++
4 files changed, 112 insertions(+)
create mode 100644 libavfilter/vf_ccrepack.c
diff --git a/doc/filters.texi b/doc/filters.texi
index 5dde799..9f67a00 100644
--- a/doc/filters.texi
+++ b/doc/filters.texi
@@ -8936,6 +8936,16 @@ Only deinterlace frames marked as interlaced.
The default value is @code{all}.
@end table
+@section ccrepack
+
+Repack CEA-708 closed captioning side data
+
+This filter fixes various issues seen with commerical encoders
+related to upstream malformed CEA-708 payloads, specifically
+incorrect number of tuples (wrong cc_count for the target FPS),
+and incorrect ordering of tuples (i.e. the CEA-608 tuples are not at
+the first entries in the payload).
+
@section cas
Apply Contrast Adaptive Sharpen filter to video stream.
diff --git a/libavfilter/Makefile b/libavfilter/Makefile
index 628ade8..3d0b3ad 100644
--- a/libavfilter/Makefile
+++ b/libavfilter/Makefile
@@ -212,6 +212,7 @@ OBJS-$(CONFIG_BOXBLUR_OPENCL_FILTER) += vf_avgblur_opencl.o opencl.o \
opencl/avgblur.o boxblur.o
OBJS-$(CONFIG_BWDIF_FILTER) += vf_bwdif.o yadif_common.o
OBJS-$(CONFIG_CAS_FILTER) += vf_cas.o
+OBJS-$(CONFIG_CCREPACK_FILTER) += vf_ccrepack.o
OBJS-$(CONFIG_CHROMABER_VULKAN_FILTER) += vf_chromaber_vulkan.o vulkan.o vulkan_filter.o
OBJS-$(CONFIG_CHROMAHOLD_FILTER) += vf_chromakey.o
OBJS-$(CONFIG_CHROMAKEY_FILTER) += vf_chromakey.o
diff --git a/libavfilter/allfilters.c b/libavfilter/allfilters.c
index d7db46c..b38550b 100644
--- a/libavfilter/allfilters.c
+++ b/libavfilter/allfilters.c
@@ -196,6 +196,7 @@ extern const AVFilter ff_vf_boxblur;
extern const AVFilter ff_vf_boxblur_opencl;
extern const AVFilter ff_vf_bwdif;
extern const AVFilter ff_vf_cas;
+extern const AVFilter ff_vf_ccrepack;
extern const AVFilter ff_vf_chromaber_vulkan;
extern const AVFilter ff_vf_chromahold;
extern const AVFilter ff_vf_chromakey;
diff --git a/libavfilter/vf_ccrepack.c b/libavfilter/vf_ccrepack.c
new file mode 100644
index 0000000..fb9120c
--- /dev/null
+++ b/libavfilter/vf_ccrepack.c
@@ -0,0 +1,100 @@
+/*
+ * CEA-708 Closed Caption Repacker
+ * Copyright (c) 2023 LTN Global Communications
+ *
+ * Author: Devin Heitmueller <dheitmueller@ltnglobal.com>
+ *
+ * This file is part of FFmpeg.
+ *
+ * FFmpeg is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 2.1 of the License, or (at your option) any later version.
+ *
+ * FFmpeg is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with FFmpeg; if not, write to the Free Software
+ * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
+ */
+
+/*
+ * Repackage CEA-708 arrays, which deals with incorrect cc_count for a given
+ * output framerate, and incorrect 708 padding.
+ *
+ * See CEA CEA-10-A "EIA-708-B Implementation Guidance", Section 26.5
+ * "Grouping DTVCC Data Within user_data() Structure"
+ */
+
+#include "avfilter.h"
+#include "internal.h"
+#include "ccfifo.h"
+#include "libavutil/opt.h"
+
+typedef struct CCRepackContext
+{
+ const AVClass *class;
+ AVCCFifo *cc_fifo;
+} CCRepackContext;
+
+static const AVOption ccrepack_options[] = {
+ { NULL }
+};
+
+AVFILTER_DEFINE_CLASS(ccrepack);
+
+static int config_input(AVFilterLink *link)
+{
+ CCRepackContext *ctx = link->dst->priv;
+
+ if (!(ctx->cc_fifo = ff_ccfifo_alloc(&link->frame_rate, ctx)))
+ av_log(ctx, AV_LOG_VERBOSE, "Failure to setup CC FIFO queue\n");
+
+ return 0;
+}
+
+static int filter_frame(AVFilterLink *inlink, AVFrame *frame)
+{
+ CCRepackContext *ctx = inlink->dst->priv;
+ AVFilterLink *outlink = inlink->dst->outputs[0];
+
+ ff_ccfifo_extract(ctx->cc_fifo, frame);
+ ff_ccfifo_inject(ctx->cc_fifo, frame);
+
+ return ff_filter_frame(outlink, frame);
+}
+
+static av_cold void uninit(AVFilterContext *ctx)
+{
+ CCRepackContext *s = ctx->priv;
+ ff_ccfifo_freep(&s->cc_fifo);
+}
+
+static const AVFilterPad avfilter_vf_ccrepack_inputs[] = {
+ {
+ .name = "default",
+ .type = AVMEDIA_TYPE_VIDEO,
+ .filter_frame = filter_frame,
+ .config_props = config_input,
+ },
+};
+
+static const AVFilterPad avfilter_vf_ccrepack_outputs[] = {
+ {
+ .name = "default",
+ .type = AVMEDIA_TYPE_VIDEO,
+ },
+};
+
+AVFilter ff_vf_ccrepack = {
+ .name = "ccrepack",
+ .description = NULL_IF_CONFIG_SMALL("Repack CEA-708 closed caption metadata"),
+ .uninit = uninit,
+ .priv_size = sizeof(CCRepackContext),
+ .priv_class = &ccrepack_class,
+ FILTER_INPUTS(avfilter_vf_ccrepack_inputs),
+ FILTER_OUTPUTS(avfilter_vf_ccrepack_outputs),
+};
--
1.8.3.1
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
^ permalink raw reply [flat|nested] 14+ messages in thread* Re: [FFmpeg-devel] [PATCH v4 5/6] vf_ccrepack: Add new filter to repack CEA-708 side data
2023-04-28 16:37 ` [FFmpeg-devel] [PATCH v4 5/6] vf_ccrepack: Add new filter to repack CEA-708 side data Devin Heitmueller
@ 2023-04-30 22:42 ` Lance Wang
2023-05-02 14:17 ` Devin Heitmueller
0 siblings, 1 reply; 14+ messages in thread
From: Lance Wang @ 2023-04-30 22:42 UTC (permalink / raw)
To: FFmpeg development discussions and patches
On Fri, Apr 28, 2023 at 11:43 PM Devin Heitmueller <
devin.heitmueller@ltnglobal.com> wrote:
> THis filter can correct certain issues seen from upstream sources
> where the cc_count is not properly set or the CEA-608 tuples are
> not at the start of the payload as expected.
>
> Make use of the ccfifo to extract and immediately repack the CEA-708
> side data, thereby removing any extra padding and ensuring the 608
> tuples are at the front of the payload.
>
> Signed-off-by: Devin Heitmueller <dheitmueller@ltnglobal.com>
> ---
> doc/filters.texi | 10 +++++
> libavfilter/Makefile | 1 +
> libavfilter/allfilters.c | 1 +
> libavfilter/vf_ccrepack.c | 100
> ++++++++++++++++++++++++++++++++++++++++++++++
> 4 files changed, 112 insertions(+)
> create mode 100644 libavfilter/vf_ccrepack.c
>
> diff --git a/doc/filters.texi b/doc/filters.texi
> index 5dde799..9f67a00 100644
> --- a/doc/filters.texi
> +++ b/doc/filters.texi
> @@ -8936,6 +8936,16 @@ Only deinterlace frames marked as interlaced.
> The default value is @code{all}.
> @end table
>
> +@section ccrepack
> +
> +Repack CEA-708 closed captioning side data
> +
> +This filter fixes various issues seen with commerical encoders
> +related to upstream malformed CEA-708 payloads, specifically
> +incorrect number of tuples (wrong cc_count for the target FPS),
> +and incorrect ordering of tuples (i.e. the CEA-608 tuples are not at
> +the first entries in the payload).
> +
> @section cas
>
> Apply Contrast Adaptive Sharpen filter to video stream.
> diff --git a/libavfilter/Makefile b/libavfilter/Makefile
> index 628ade8..3d0b3ad 100644
> --- a/libavfilter/Makefile
> +++ b/libavfilter/Makefile
> @@ -212,6 +212,7 @@ OBJS-$(CONFIG_BOXBLUR_OPENCL_FILTER) +=
> vf_avgblur_opencl.o opencl.o \
> opencl/avgblur.o boxblur.o
> OBJS-$(CONFIG_BWDIF_FILTER) += vf_bwdif.o yadif_common.o
> OBJS-$(CONFIG_CAS_FILTER) += vf_cas.o
> +OBJS-$(CONFIG_CCREPACK_FILTER) += vf_ccrepack.o
> OBJS-$(CONFIG_CHROMABER_VULKAN_FILTER) += vf_chromaber_vulkan.o
> vulkan.o vulkan_filter.o
> OBJS-$(CONFIG_CHROMAHOLD_FILTER) += vf_chromakey.o
> OBJS-$(CONFIG_CHROMAKEY_FILTER) += vf_chromakey.o
> diff --git a/libavfilter/allfilters.c b/libavfilter/allfilters.c
> index d7db46c..b38550b 100644
> --- a/libavfilter/allfilters.c
> +++ b/libavfilter/allfilters.c
> @@ -196,6 +196,7 @@ extern const AVFilter ff_vf_boxblur;
> extern const AVFilter ff_vf_boxblur_opencl;
> extern const AVFilter ff_vf_bwdif;
> extern const AVFilter ff_vf_cas;
> +extern const AVFilter ff_vf_ccrepack;
> extern const AVFilter ff_vf_chromaber_vulkan;
> extern const AVFilter ff_vf_chromahold;
> extern const AVFilter ff_vf_chromakey;
> diff --git a/libavfilter/vf_ccrepack.c b/libavfilter/vf_ccrepack.c
> new file mode 100644
> index 0000000..fb9120c
> --- /dev/null
> +++ b/libavfilter/vf_ccrepack.c
> @@ -0,0 +1,100 @@
> +/*
> + * CEA-708 Closed Caption Repacker
> + * Copyright (c) 2023 LTN Global Communications
> + *
> + * Author: Devin Heitmueller <dheitmueller@ltnglobal.com>
> + *
> + * This file is part of FFmpeg.
> + *
> + * FFmpeg is free software; you can redistribute it and/or
> + * modify it under the terms of the GNU Lesser General Public
> + * License as published by the Free Software Foundation; either
> + * version 2.1 of the License, or (at your option) any later version.
> + *
> + * FFmpeg is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
> + * Lesser General Public License for more details.
> + *
> + * You should have received a copy of the GNU Lesser General Public
> + * License along with FFmpeg; if not, write to the Free Software
> + * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA
> 02110-1301 USA
> + */
> +
> +/*
> + * Repackage CEA-708 arrays, which deals with incorrect cc_count for a
> given
> + * output framerate, and incorrect 708 padding.
> + *
> + * See CEA CEA-10-A "EIA-708-B Implementation Guidance", Section 26.5
> + * "Grouping DTVCC Data Within user_data() Structure"
> + */
> +
> +#include "avfilter.h"
> +#include "internal.h"
> +#include "ccfifo.h"
> +#include "libavutil/opt.h"
> +
> +typedef struct CCRepackContext
> +{
> + const AVClass *class;
> + AVCCFifo *cc_fifo;
> +} CCRepackContext;
> +
> +static const AVOption ccrepack_options[] = {
> + { NULL }
> +};
> +
>
unused code as no option
> +AVFILTER_DEFINE_CLASS(ccrepack);
> +
> +static int config_input(AVFilterLink *link)
> +{
> + CCRepackContext *ctx = link->dst->priv;
> +
> + if (!(ctx->cc_fifo = ff_ccfifo_alloc(&link->frame_rate, ctx)))
> + av_log(ctx, AV_LOG_VERBOSE, "Failure to setup CC FIFO queue\n");
> +
>
it's better to return error and print error log here.
> + return 0;
> +}
> +
> +static int filter_frame(AVFilterLink *inlink, AVFrame *frame)
> +{
> + CCRepackContext *ctx = inlink->dst->priv;
> + AVFilterLink *outlink = inlink->dst->outputs[0];
> +
> + ff_ccfifo_extract(ctx->cc_fifo, frame);
> + ff_ccfifo_inject(ctx->cc_fifo, frame);
> +
>
It's better to check return of function here.
> + return ff_filter_frame(outlink, frame);
> +}
> +
> +static av_cold void uninit(AVFilterContext *ctx)
> +{
> + CCRepackContext *s = ctx->priv;
> + ff_ccfifo_freep(&s->cc_fifo);
> +}
> +
> +static const AVFilterPad avfilter_vf_ccrepack_inputs[] = {
> + {
> + .name = "default",
> + .type = AVMEDIA_TYPE_VIDEO,
> + .filter_frame = filter_frame,
> + .config_props = config_input,
> + },
> +};
> +
> +static const AVFilterPad avfilter_vf_ccrepack_outputs[] = {
> + {
> + .name = "default",
> + .type = AVMEDIA_TYPE_VIDEO,
> + },
> +};
> +
> +AVFilter ff_vf_ccrepack = {
> + .name = "ccrepack",
> + .description = NULL_IF_CONFIG_SMALL("Repack CEA-708 closed caption
> metadata"),
> + .uninit = uninit,
> + .priv_size = sizeof(CCRepackContext),
> + .priv_class = &ccrepack_class,
> + FILTER_INPUTS(avfilter_vf_ccrepack_inputs),
> + FILTER_OUTPUTS(avfilter_vf_ccrepack_outputs),
> +};
> --
> 1.8.3.1
>
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>
> To unsubscribe, visit link above, or email
> ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
>
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
^ permalink raw reply [flat|nested] 14+ messages in thread* Re: [FFmpeg-devel] [PATCH v4 5/6] vf_ccrepack: Add new filter to repack CEA-708 side data
2023-04-30 22:42 ` Lance Wang
@ 2023-05-02 14:17 ` Devin Heitmueller
0 siblings, 0 replies; 14+ messages in thread
From: Devin Heitmueller @ 2023-05-02 14:17 UTC (permalink / raw)
To: FFmpeg development discussions and patches
Hi Lance,
On Sun, Apr 30, 2023 at 6:42 PM Lance Wang <lance.lmwang@gmail.com> wrote:
>
> On Fri, Apr 28, 2023 at 11:43 PM Devin Heitmueller <
> devin.heitmueller@ltnglobal.com> wrote:
>
> > THis filter can correct certain issues seen from upstream sources
> > where the cc_count is not properly set or the CEA-608 tuples are
> > not at the start of the payload as expected.
> >
> > Make use of the ccfifo to extract and immediately repack the CEA-708
> > side data, thereby removing any extra padding and ensuring the 608
> > tuples are at the front of the payload.
> >
> > Signed-off-by: Devin Heitmueller <dheitmueller@ltnglobal.com>
> > ---
> > doc/filters.texi | 10 +++++
> > libavfilter/Makefile | 1 +
> > libavfilter/allfilters.c | 1 +
> > libavfilter/vf_ccrepack.c | 100
> > ++++++++++++++++++++++++++++++++++++++++++++++
> > 4 files changed, 112 insertions(+)
> > create mode 100644 libavfilter/vf_ccrepack.c
> >
> > diff --git a/doc/filters.texi b/doc/filters.texi
> > index 5dde799..9f67a00 100644
> > --- a/doc/filters.texi
> > +++ b/doc/filters.texi
> > @@ -8936,6 +8936,16 @@ Only deinterlace frames marked as interlaced.
> > The default value is @code{all}.
> > @end table
> >
> > +@section ccrepack
> > +
> > +Repack CEA-708 closed captioning side data
> > +
> > +This filter fixes various issues seen with commerical encoders
> > +related to upstream malformed CEA-708 payloads, specifically
> > +incorrect number of tuples (wrong cc_count for the target FPS),
> > +and incorrect ordering of tuples (i.e. the CEA-608 tuples are not at
> > +the first entries in the payload).
> > +
> > @section cas
> >
> > Apply Contrast Adaptive Sharpen filter to video stream.
> > diff --git a/libavfilter/Makefile b/libavfilter/Makefile
> > index 628ade8..3d0b3ad 100644
> > --- a/libavfilter/Makefile
> > +++ b/libavfilter/Makefile
> > @@ -212,6 +212,7 @@ OBJS-$(CONFIG_BOXBLUR_OPENCL_FILTER) +=
> > vf_avgblur_opencl.o opencl.o \
> > opencl/avgblur.o boxblur.o
> > OBJS-$(CONFIG_BWDIF_FILTER) += vf_bwdif.o yadif_common.o
> > OBJS-$(CONFIG_CAS_FILTER) += vf_cas.o
> > +OBJS-$(CONFIG_CCREPACK_FILTER) += vf_ccrepack.o
> > OBJS-$(CONFIG_CHROMABER_VULKAN_FILTER) += vf_chromaber_vulkan.o
> > vulkan.o vulkan_filter.o
> > OBJS-$(CONFIG_CHROMAHOLD_FILTER) += vf_chromakey.o
> > OBJS-$(CONFIG_CHROMAKEY_FILTER) += vf_chromakey.o
> > diff --git a/libavfilter/allfilters.c b/libavfilter/allfilters.c
> > index d7db46c..b38550b 100644
> > --- a/libavfilter/allfilters.c
> > +++ b/libavfilter/allfilters.c
> > @@ -196,6 +196,7 @@ extern const AVFilter ff_vf_boxblur;
> > extern const AVFilter ff_vf_boxblur_opencl;
> > extern const AVFilter ff_vf_bwdif;
> > extern const AVFilter ff_vf_cas;
> > +extern const AVFilter ff_vf_ccrepack;
> > extern const AVFilter ff_vf_chromaber_vulkan;
> > extern const AVFilter ff_vf_chromahold;
> > extern const AVFilter ff_vf_chromakey;
> > diff --git a/libavfilter/vf_ccrepack.c b/libavfilter/vf_ccrepack.c
> > new file mode 100644
> > index 0000000..fb9120c
> > --- /dev/null
> > +++ b/libavfilter/vf_ccrepack.c
> > @@ -0,0 +1,100 @@
> > +/*
> > + * CEA-708 Closed Caption Repacker
> > + * Copyright (c) 2023 LTN Global Communications
> > + *
> > + * Author: Devin Heitmueller <dheitmueller@ltnglobal.com>
> > + *
> > + * This file is part of FFmpeg.
> > + *
> > + * FFmpeg is free software; you can redistribute it and/or
> > + * modify it under the terms of the GNU Lesser General Public
> > + * License as published by the Free Software Foundation; either
> > + * version 2.1 of the License, or (at your option) any later version.
> > + *
> > + * FFmpeg is distributed in the hope that it will be useful,
> > + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
> > + * Lesser General Public License for more details.
> > + *
> > + * You should have received a copy of the GNU Lesser General Public
> > + * License along with FFmpeg; if not, write to the Free Software
> > + * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA
> > 02110-1301 USA
> > + */
> > +
> > +/*
> > + * Repackage CEA-708 arrays, which deals with incorrect cc_count for a
> > given
> > + * output framerate, and incorrect 708 padding.
> > + *
> > + * See CEA CEA-10-A "EIA-708-B Implementation Guidance", Section 26.5
> > + * "Grouping DTVCC Data Within user_data() Structure"
> > + */
> > +
> > +#include "avfilter.h"
> > +#include "internal.h"
> > +#include "ccfifo.h"
> > +#include "libavutil/opt.h"
> > +
> > +typedef struct CCRepackContext
> > +{
> > + const AVClass *class;
> > + AVCCFifo *cc_fifo;
> > +} CCRepackContext;
> > +
> > +static const AVOption ccrepack_options[] = {
> > + { NULL }
> > +};
> > +
> >
> unused code as no option
>
>
> > +AVFILTER_DEFINE_CLASS(ccrepack);
> > +
> > +static int config_input(AVFilterLink *link)
> > +{
> > + CCRepackContext *ctx = link->dst->priv;
> > +
> > + if (!(ctx->cc_fifo = ff_ccfifo_alloc(&link->frame_rate, ctx)))
> > + av_log(ctx, AV_LOG_VERBOSE, "Failure to setup CC FIFO queue\n");
> > +
> >
>
> it's better to return error and print error log here.
>
>
> > + return 0;
> > +}
> > +
> > +static int filter_frame(AVFilterLink *inlink, AVFrame *frame)
> > +{
> > + CCRepackContext *ctx = inlink->dst->priv;
> > + AVFilterLink *outlink = inlink->dst->outputs[0];
> > +
> > + ff_ccfifo_extract(ctx->cc_fifo, frame);
> > + ff_ccfifo_inject(ctx->cc_fifo, frame);
> > +
> >
> It's better to check return of function here.
>
>
> > + return ff_filter_frame(outlink, frame);
> > +}
> > +
> > +static av_cold void uninit(AVFilterContext *ctx)
> > +{
> > + CCRepackContext *s = ctx->priv;
> > + ff_ccfifo_freep(&s->cc_fifo);
> > +}
> > +
> > +static const AVFilterPad avfilter_vf_ccrepack_inputs[] = {
> > + {
> > + .name = "default",
> > + .type = AVMEDIA_TYPE_VIDEO,
> > + .filter_frame = filter_frame,
> > + .config_props = config_input,
> > + },
> > +};
> > +
> > +static const AVFilterPad avfilter_vf_ccrepack_outputs[] = {
> > + {
> > + .name = "default",
> > + .type = AVMEDIA_TYPE_VIDEO,
> > + },
> > +};
> > +
> > +AVFilter ff_vf_ccrepack = {
> > + .name = "ccrepack",
> > + .description = NULL_IF_CONFIG_SMALL("Repack CEA-708 closed caption
> > metadata"),
> > + .uninit = uninit,
> > + .priv_size = sizeof(CCRepackContext),
> > + .priv_class = &ccrepack_class,
> > + FILTER_INPUTS(avfilter_vf_ccrepack_inputs),
> > + FILTER_OUTPUTS(avfilter_vf_ccrepack_outputs),
> > +};
> > --
> > 1.8.3.1
> >
> > _______________________________________________
> > ffmpeg-devel mailing list
> > ffmpeg-devel@ffmpeg.org
> > https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
> >
> > To unsubscribe, visit link above, or email
> > ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
> >
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>
> To unsubscribe, visit link above, or email
> ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
These are all reasonable comments, and in fact largely the result of
my not reflecting changes I made to the API in the last round, where
we now have the ff_ccfifo_alloc() call succeed even if the output
framerate is not supported.
I'll roll these fixes into the next series.
Devin
--
Devin Heitmueller, Senior Software Engineer
LTN Global Communications
o: +1 (301) 363-1001
w: https://ltnglobal.com e: devin.heitmueller@ltnglobal.com
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
^ permalink raw reply [flat|nested] 14+ messages in thread
* [FFmpeg-devel] [PATCH v4 6/6] decklink_enc: add support for playout of 608 captions in MOV files
2023-04-28 16:37 [FFmpeg-devel] [PATCH v4 0/6] Add support for Closed Caption FIFO Devin Heitmueller
` (4 preceding siblings ...)
2023-04-28 16:37 ` [FFmpeg-devel] [PATCH v4 5/6] vf_ccrepack: Add new filter to repack CEA-708 side data Devin Heitmueller
@ 2023-04-28 16:37 ` Devin Heitmueller
2023-04-30 23:01 ` Lance Wang
5 siblings, 1 reply; 14+ messages in thread
From: Devin Heitmueller @ 2023-04-28 16:37 UTC (permalink / raw)
To: ffmpeg-devel; +Cc: Devin Heitmueller
Unlike other cases where the closed captions are embedded in the
video stream as MPEG-2 userdata or H.264 SEI data, with MOV files
the captions are often found on a separate "e608" subtitle track.
Add support for playout of such files, leveraging the new ccfifo
mechanism to ensure that they are embedded into VANC at the correct
rate (since e608 packets often contain batches of multiple 608 pairs).
Note this patch includes a new file named libavdevice/ccfifo.c, which
allows the ccfifo functionality in libavfilter to be reused even if
doing shared builds. This is the same approach used for log2_tab.c.
Signed-off-by: Devin Heitmueller <dheitmueller@ltnglobal.com>
---
libavdevice/Makefile | 1 +
libavdevice/ccfifo.c | 24 ++++++++++++++++
libavdevice/decklink_common.h | 3 ++
libavdevice/decklink_enc.cpp | 66 +++++++++++++++++++++++++++++++++++++++++++
libavdevice/decklink_enc_c.c | 2 +-
5 files changed, 95 insertions(+), 1 deletion(-)
create mode 100644 libavdevice/ccfifo.c
diff --git a/libavdevice/Makefile b/libavdevice/Makefile
index 8a62822..c304492 100644
--- a/libavdevice/Makefile
+++ b/libavdevice/Makefile
@@ -57,6 +57,7 @@ OBJS-$(CONFIG_LIBDC1394_INDEV) += libdc1394.o
# Objects duplicated from other libraries for shared builds
SHLIBOBJS-$(CONFIG_DECKLINK_INDEV) += reverse.o
+SHLIBOBJS-$(CONFIG_DECKLINK_OUTDEV) += ccfifo.o
# Windows resource file
SHLIBOBJS-$(HAVE_GNU_WINDRES) += avdeviceres.o
diff --git a/libavdevice/ccfifo.c b/libavdevice/ccfifo.c
new file mode 100644
index 0000000..9007094
--- /dev/null
+++ b/libavdevice/ccfifo.c
@@ -0,0 +1,24 @@
+/*
+ * CEA-708 Closed Captioning FIFO
+ * Copyright (c) 2023 LTN Global Communications
+ *
+ * Author: Devin Heitmueller <dheitmueller@ltnglobal.com>
+ *
+ * This file is part of FFmpeg.
+ *
+ * FFmpeg is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 2.1 of the License, or (at your option) any later version.
+ *
+ * FFmpeg is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with FFmpeg; if not, write to the Free Software
+ * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
+ */
+
+#include "libavfilter/ccfifo.c"
diff --git a/libavdevice/decklink_common.h b/libavdevice/decklink_common.h
index 088e165..0d33f94 100644
--- a/libavdevice/decklink_common.h
+++ b/libavdevice/decklink_common.h
@@ -31,6 +31,7 @@
extern "C" {
#include "libavcodec/packet_internal.h"
+#include "libavfilter/ccfifo.h"
}
#include "libavutil/thread.h"
#include "decklink_common_c.h"
@@ -112,6 +113,8 @@ struct decklink_ctx {
/* Capture buffer queue */
AVPacketQueue queue;
+ AVCCFifo *cc_fifo; ///< closed captions
+
/* Streams present */
int audio;
int video;
diff --git a/libavdevice/decklink_enc.cpp b/libavdevice/decklink_enc.cpp
index 9f1a8df..616e9c7 100644
--- a/libavdevice/decklink_enc.cpp
+++ b/libavdevice/decklink_enc.cpp
@@ -326,6 +326,25 @@ static int create_s337_payload(AVPacket *pkt, uint8_t **outbuf, int *outsize)
return 0;
}
+static int decklink_setup_subtitle(AVFormatContext *avctx, AVStream *st)
+{
+ int ret = -1;
+
+ switch(st->codecpar->codec_id) {
+#if CONFIG_LIBKLVANC
+ case AV_CODEC_ID_EIA_608:
+ /* No special setup required */
+ ret = 0;
+ break;
+#endif
+ default:
+ av_log(avctx, AV_LOG_ERROR, "Unsupported subtitle codec specified\n");
+ break;
+ }
+
+ return ret;
+}
+
av_cold int ff_decklink_write_trailer(AVFormatContext *avctx)
{
struct decklink_cctx *cctx = (struct decklink_cctx *)avctx->priv_data;
@@ -352,6 +371,7 @@ av_cold int ff_decklink_write_trailer(AVFormatContext *avctx)
klvanc_context_destroy(ctx->vanc_ctx);
#endif
+ ff_ccfifo_freep(&ctx->cc_fifo);
av_freep(&cctx->ctx);
return 0;
@@ -503,6 +523,23 @@ out:
free(afd_words);
}
+/* Parse any EIA-608 subtitles sitting on the queue, and write packet side data
+ that will later be handled by construct_cc... */
+static void parse_608subs(AVFormatContext *avctx, struct decklink_ctx *ctx, AVPacket *pkt)
+{
+ uint8_t *cc_data;
+ size_t cc_size;
+ int ret;
+
+ ret = ff_ccfifo_injectbytes(ctx->cc_fifo, &cc_data, &cc_size);
+ if (ret == 0) {
+ uint8_t *cc_buf = av_packet_new_side_data(pkt, AV_PKT_DATA_A53_CC, cc_size);
+ if (cc_buf)
+ memcpy(cc_buf, cc_data, cc_size);
+ av_freep(&cc_data);
+ }
+}
+
static int decklink_construct_vanc(AVFormatContext *avctx, struct decklink_ctx *ctx,
AVPacket *pkt, decklink_frame *frame,
AVStream *st)
@@ -513,6 +550,7 @@ static int decklink_construct_vanc(AVFormatContext *avctx, struct decklink_ctx *
if (!ctx->supports_vanc)
return 0;
+ parse_608subs(avctx, ctx, pkt);
construct_cc(avctx, ctx, pkt, &vanc_lines);
construct_afd(avctx, ctx, pkt, &vanc_lines, st);
@@ -704,12 +742,23 @@ static int decklink_write_audio_packet(AVFormatContext *avctx, AVPacket *pkt)
return ret;
}
+static int decklink_write_subtitle_packet(AVFormatContext *avctx, AVPacket *pkt)
+{
+ struct decklink_cctx *cctx = (struct decklink_cctx *)avctx->priv_data;
+ struct decklink_ctx *ctx = (struct decklink_ctx *)cctx->ctx;
+
+ ff_ccfifo_extractbytes(ctx->cc_fifo, pkt->data, pkt->size);
+
+ return 0;
+}
+
extern "C" {
av_cold int ff_decklink_write_header(AVFormatContext *avctx)
{
struct decklink_cctx *cctx = (struct decklink_cctx *)avctx->priv_data;
struct decklink_ctx *ctx;
+ AVRational frame_rate;
unsigned int n;
int ret;
@@ -768,12 +817,27 @@ av_cold int ff_decklink_write_header(AVFormatContext *avctx)
} else if (c->codec_type == AVMEDIA_TYPE_VIDEO) {
if (decklink_setup_video(avctx, st))
goto error;
+ } else if (c->codec_type == AVMEDIA_TYPE_SUBTITLE) {
+ if (decklink_setup_subtitle(avctx, st))
+ goto error;
} else {
av_log(avctx, AV_LOG_ERROR, "Unsupported stream type.\n");
goto error;
}
}
+ for (n = 0; n < avctx->nb_streams; n++) {
+ AVStream *st = avctx->streams[n];
+ AVCodecParameters *c = st->codecpar;
+
+ if(c->codec_type == AVMEDIA_TYPE_SUBTITLE)
+ avpriv_set_pts_info(st, 64, ctx->bmd_tb_num, ctx->bmd_tb_den);
+ }
+
+ frame_rate = av_make_q(ctx->bmd_tb_den, ctx->bmd_tb_num);
+ if (!(ctx->cc_fifo = ff_ccfifo_alloc(&frame_rate, ctx)))
+ av_log(ctx, AV_LOG_VERBOSE, "Failure to setup CC FIFO queue\n");
+
return 0;
error:
@@ -789,6 +853,8 @@ int ff_decklink_write_packet(AVFormatContext *avctx, AVPacket *pkt)
return decklink_write_video_packet(avctx, pkt);
else if (st->codecpar->codec_type == AVMEDIA_TYPE_AUDIO)
return decklink_write_audio_packet(avctx, pkt);
+ else if (st->codecpar->codec_type == AVMEDIA_TYPE_SUBTITLE)
+ return decklink_write_subtitle_packet(avctx, pkt);
return AVERROR(EIO);
}
diff --git a/libavdevice/decklink_enc_c.c b/libavdevice/decklink_enc_c.c
index f7e3150..0a3984b 100644
--- a/libavdevice/decklink_enc_c.c
+++ b/libavdevice/decklink_enc_c.c
@@ -77,7 +77,7 @@ const FFOutputFormat ff_decklink_muxer = {
.p.long_name = NULL_IF_CONFIG_SMALL("Blackmagic DeckLink output"),
.p.audio_codec = AV_CODEC_ID_PCM_S16LE,
.p.video_codec = AV_CODEC_ID_WRAPPED_AVFRAME,
- .p.subtitle_codec = AV_CODEC_ID_NONE,
+ .p.subtitle_codec = AV_CODEC_ID_EIA_608,
.p.flags = AVFMT_NOFILE,
.p.priv_class = &decklink_muxer_class,
.get_device_list = ff_decklink_list_output_devices,
--
1.8.3.1
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
^ permalink raw reply [flat|nested] 14+ messages in thread* Re: [FFmpeg-devel] [PATCH v4 6/6] decklink_enc: add support for playout of 608 captions in MOV files
2023-04-28 16:37 ` [FFmpeg-devel] [PATCH v4 6/6] decklink_enc: add support for playout of 608 captions in MOV files Devin Heitmueller
@ 2023-04-30 23:01 ` Lance Wang
2023-05-02 14:47 ` Devin Heitmueller
0 siblings, 1 reply; 14+ messages in thread
From: Lance Wang @ 2023-04-30 23:01 UTC (permalink / raw)
To: FFmpeg development discussions and patches
On Fri, Apr 28, 2023 at 11:43 PM Devin Heitmueller <
devin.heitmueller@ltnglobal.com> wrote:
> Unlike other cases where the closed captions are embedded in the
> video stream as MPEG-2 userdata or H.264 SEI data, with MOV files
> the captions are often found on a separate "e608" subtitle track.
>
> Add support for playout of such files, leveraging the new ccfifo
> mechanism to ensure that they are embedded into VANC at the correct
> rate (since e608 packets often contain batches of multiple 608 pairs).
>
> Note this patch includes a new file named libavdevice/ccfifo.c, which
> allows the ccfifo functionality in libavfilter to be reused even if
> doing shared builds. This is the same approach used for log2_tab.c.
>
>
This implementation is limited to decklink SDI output only, If possible,
can we implement the function from demuxer layer, and then passthrough
by SEI side data? By this way, we can convert such stream in streaming
to embedded CC to video stream easily also.
Signed-off-by: Devin Heitmueller <dheitmueller@ltnglobal.com>
> ---
> libavdevice/Makefile | 1 +
> libavdevice/ccfifo.c | 24 ++++++++++++++++
> libavdevice/decklink_common.h | 3 ++
> libavdevice/decklink_enc.cpp | 66
> +++++++++++++++++++++++++++++++++++++++++++
> libavdevice/decklink_enc_c.c | 2 +-
> 5 files changed, 95 insertions(+), 1 deletion(-)
> create mode 100644 libavdevice/ccfifo.c
>
> diff --git a/libavdevice/Makefile b/libavdevice/Makefile
> index 8a62822..c304492 100644
> --- a/libavdevice/Makefile
> +++ b/libavdevice/Makefile
> @@ -57,6 +57,7 @@ OBJS-$(CONFIG_LIBDC1394_INDEV) += libdc1394.o
>
> # Objects duplicated from other libraries for shared builds
> SHLIBOBJS-$(CONFIG_DECKLINK_INDEV) += reverse.o
> +SHLIBOBJS-$(CONFIG_DECKLINK_OUTDEV) += ccfifo.o
>
> # Windows resource file
> SHLIBOBJS-$(HAVE_GNU_WINDRES) += avdeviceres.o
> diff --git a/libavdevice/ccfifo.c b/libavdevice/ccfifo.c
> new file mode 100644
> index 0000000..9007094
> --- /dev/null
> +++ b/libavdevice/ccfifo.c
> @@ -0,0 +1,24 @@
> +/*
> + * CEA-708 Closed Captioning FIFO
> + * Copyright (c) 2023 LTN Global Communications
> + *
> + * Author: Devin Heitmueller <dheitmueller@ltnglobal.com>
> + *
> + * This file is part of FFmpeg.
> + *
> + * FFmpeg is free software; you can redistribute it and/or
> + * modify it under the terms of the GNU Lesser General Public
> + * License as published by the Free Software Foundation; either
> + * version 2.1 of the License, or (at your option) any later version.
> + *
> + * FFmpeg is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
> + * Lesser General Public License for more details.
> + *
> + * You should have received a copy of the GNU Lesser General Public
> + * License along with FFmpeg; if not, write to the Free Software
> + * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA
> 02110-1301 USA
> + */
> +
> +#include "libavfilter/ccfifo.c"
> diff --git a/libavdevice/decklink_common.h b/libavdevice/decklink_common.h
> index 088e165..0d33f94 100644
> --- a/libavdevice/decklink_common.h
> +++ b/libavdevice/decklink_common.h
> @@ -31,6 +31,7 @@
>
> extern "C" {
> #include "libavcodec/packet_internal.h"
> +#include "libavfilter/ccfifo.h"
> }
> #include "libavutil/thread.h"
> #include "decklink_common_c.h"
> @@ -112,6 +113,8 @@ struct decklink_ctx {
> /* Capture buffer queue */
> AVPacketQueue queue;
>
> + AVCCFifo *cc_fifo; ///< closed captions
> +
> /* Streams present */
> int audio;
> int video;
> diff --git a/libavdevice/decklink_enc.cpp b/libavdevice/decklink_enc.cpp
> index 9f1a8df..616e9c7 100644
> --- a/libavdevice/decklink_enc.cpp
> +++ b/libavdevice/decklink_enc.cpp
> @@ -326,6 +326,25 @@ static int create_s337_payload(AVPacket *pkt, uint8_t
> **outbuf, int *outsize)
> return 0;
> }
>
> +static int decklink_setup_subtitle(AVFormatContext *avctx, AVStream *st)
> +{
> + int ret = -1;
> +
> + switch(st->codecpar->codec_id) {
> +#if CONFIG_LIBKLVANC
> + case AV_CODEC_ID_EIA_608:
> + /* No special setup required */
> + ret = 0;
> + break;
> +#endif
> + default:
> + av_log(avctx, AV_LOG_ERROR, "Unsupported subtitle codec
> specified\n");
> + break;
> + }
> +
> + return ret;
> +}
> +
> av_cold int ff_decklink_write_trailer(AVFormatContext *avctx)
> {
> struct decklink_cctx *cctx = (struct decklink_cctx *)avctx->priv_data;
> @@ -352,6 +371,7 @@ av_cold int ff_decklink_write_trailer(AVFormatContext
> *avctx)
> klvanc_context_destroy(ctx->vanc_ctx);
> #endif
>
> + ff_ccfifo_freep(&ctx->cc_fifo);
> av_freep(&cctx->ctx);
>
> return 0;
> @@ -503,6 +523,23 @@ out:
> free(afd_words);
> }
>
> +/* Parse any EIA-608 subtitles sitting on the queue, and write packet
> side data
> + that will later be handled by construct_cc... */
> +static void parse_608subs(AVFormatContext *avctx, struct decklink_ctx
> *ctx, AVPacket *pkt)
> +{
> + uint8_t *cc_data;
> + size_t cc_size;
> + int ret;
> +
> + ret = ff_ccfifo_injectbytes(ctx->cc_fifo, &cc_data, &cc_size);
> + if (ret == 0) {
> + uint8_t *cc_buf = av_packet_new_side_data(pkt,
> AV_PKT_DATA_A53_CC, cc_size);
> + if (cc_buf)
> + memcpy(cc_buf, cc_data, cc_size);
> + av_freep(&cc_data);
> + }
> +}
> +
> static int decklink_construct_vanc(AVFormatContext *avctx, struct
> decklink_ctx *ctx,
> AVPacket *pkt, decklink_frame *frame,
> AVStream *st)
> @@ -513,6 +550,7 @@ static int decklink_construct_vanc(AVFormatContext
> *avctx, struct decklink_ctx *
> if (!ctx->supports_vanc)
> return 0;
>
> + parse_608subs(avctx, ctx, pkt);
> construct_cc(avctx, ctx, pkt, &vanc_lines);
> construct_afd(avctx, ctx, pkt, &vanc_lines, st);
>
> @@ -704,12 +742,23 @@ static int
> decklink_write_audio_packet(AVFormatContext *avctx, AVPacket *pkt)
> return ret;
> }
>
> +static int decklink_write_subtitle_packet(AVFormatContext *avctx,
> AVPacket *pkt)
> +{
> + struct decklink_cctx *cctx = (struct decklink_cctx *)avctx->priv_data;
> + struct decklink_ctx *ctx = (struct decklink_ctx *)cctx->ctx;
> +
> + ff_ccfifo_extractbytes(ctx->cc_fifo, pkt->data, pkt->size);
> +
> + return 0;
> +}
> +
> extern "C" {
>
> av_cold int ff_decklink_write_header(AVFormatContext *avctx)
> {
> struct decklink_cctx *cctx = (struct decklink_cctx *)avctx->priv_data;
> struct decklink_ctx *ctx;
> + AVRational frame_rate;
> unsigned int n;
> int ret;
>
> @@ -768,12 +817,27 @@ av_cold int ff_decklink_write_header(AVFormatContext
> *avctx)
> } else if (c->codec_type == AVMEDIA_TYPE_VIDEO) {
> if (decklink_setup_video(avctx, st))
> goto error;
> + } else if (c->codec_type == AVMEDIA_TYPE_SUBTITLE) {
> + if (decklink_setup_subtitle(avctx, st))
> + goto error;
> } else {
> av_log(avctx, AV_LOG_ERROR, "Unsupported stream type.\n");
> goto error;
> }
> }
>
> + for (n = 0; n < avctx->nb_streams; n++) {
> + AVStream *st = avctx->streams[n];
> + AVCodecParameters *c = st->codecpar;
> +
> + if(c->codec_type == AVMEDIA_TYPE_SUBTITLE)
> + avpriv_set_pts_info(st, 64, ctx->bmd_tb_num, ctx->bmd_tb_den);
> + }
> +
> + frame_rate = av_make_q(ctx->bmd_tb_den, ctx->bmd_tb_num);
> + if (!(ctx->cc_fifo = ff_ccfifo_alloc(&frame_rate, ctx)))
> + av_log(ctx, AV_LOG_VERBOSE, "Failure to setup CC FIFO queue\n");
> +
> return 0;
>
> error:
> @@ -789,6 +853,8 @@ int ff_decklink_write_packet(AVFormatContext *avctx,
> AVPacket *pkt)
> return decklink_write_video_packet(avctx, pkt);
> else if (st->codecpar->codec_type == AVMEDIA_TYPE_AUDIO)
> return decklink_write_audio_packet(avctx, pkt);
> + else if (st->codecpar->codec_type == AVMEDIA_TYPE_SUBTITLE)
> + return decklink_write_subtitle_packet(avctx, pkt);
>
> return AVERROR(EIO);
> }
> diff --git a/libavdevice/decklink_enc_c.c b/libavdevice/decklink_enc_c.c
> index f7e3150..0a3984b 100644
> --- a/libavdevice/decklink_enc_c.c
> +++ b/libavdevice/decklink_enc_c.c
> @@ -77,7 +77,7 @@ const FFOutputFormat ff_decklink_muxer = {
> .p.long_name = NULL_IF_CONFIG_SMALL("Blackmagic DeckLink
> output"),
> .p.audio_codec = AV_CODEC_ID_PCM_S16LE,
> .p.video_codec = AV_CODEC_ID_WRAPPED_AVFRAME,
> - .p.subtitle_codec = AV_CODEC_ID_NONE,
> + .p.subtitle_codec = AV_CODEC_ID_EIA_608,
> .p.flags = AVFMT_NOFILE,
> .p.priv_class = &decklink_muxer_class,
> .get_device_list = ff_decklink_list_output_devices,
> --
> 1.8.3.1
>
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>
> To unsubscribe, visit link above, or email
> ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
>
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
^ permalink raw reply [flat|nested] 14+ messages in thread* Re: [FFmpeg-devel] [PATCH v4 6/6] decklink_enc: add support for playout of 608 captions in MOV files
2023-04-30 23:01 ` Lance Wang
@ 2023-05-02 14:47 ` Devin Heitmueller
2023-05-03 7:36 ` Lance Wang
0 siblings, 1 reply; 14+ messages in thread
From: Devin Heitmueller @ 2023-05-02 14:47 UTC (permalink / raw)
To: FFmpeg development discussions and patches
Hi Lance,
On Sun, Apr 30, 2023 at 7:01 PM Lance Wang <lance.lmwang@gmail.com> wrote:
> This implementation is limited to decklink SDI output only, If possible,
> can we implement the function from demuxer layer, and then passthrough
> by SEI side data? By this way, we can convert such stream in streaming
> to embedded CC to video stream easily also.
I did consider this approach, and it does raise the more fundamental
issue about trying to minimize the number of ways we have to process
CC data depending on whether it originated in SEI metadata or in
separate packets. There are a number of problems with what you are
proposing though:
1. There could be multiple CC streams within an MOV file but only a
single CC stream can be embedded into AVFrame side data. Hence you
would have to specify some sort of argument to the demux to decide
which stream to embed. This makes it much more difficult to do things
like ingest a stream with multiple CC streams and have separate
outputs with different CC streams. Performing the work on the output
side allows you to use the standard "-map" mechanism to dictate which
CC streams are routed to which outputs, and to deliver the content to
different outputs with different CC streams.
2. I have use cases in mind where the captions originate from sources
other than MOV files, where the video framerate is not known (or there
is no video at all in the source). For example, I want to be able to
consume video from a TS source while simultaneously demuxing an SCC or
MCC file and sending the result in the output. In such cases the
correct rate control for the captions can only be implemented on the
output side, since in such cases the SCC/MCC demux doesn't have access
to the corresponding video stream (it won't know the video framerate,
nor is it able to embed the captions into the AVFrame side data).
I can indeed imagine there are use cases where doing it further up the
pipeline could be useful. For example, if you were taking in an MOV
file and wanting to produce a TS where the captions need to be
embedded as SEI metadata (hence you would need the e608 packets
converted to AVFrame side data prior to reaching the encoder).
However I don't see this as a substitute for being able to do it on
the output side when that is the most flexible approach for those
other use cases described above.
Much of this comes down to the fundamental limitations of the ffmpeg
framework related to being able to move data back/forth between data
packets and side data. You can't feed data packets into
AVFilterGraphs. You can't easily combine data from data packets into
AVFrames carrying video (or extract side data from AVFrames to
generate data packets), etc. You can't use BSF filters to combine
data from multiple inputs such as compressed video streams and data
streams after encoding. I've run across all these limitations over
the years, and at this point I'm trying to take the least invasive
approach possible that doesn't require changes to the fundamental
frameworks for handling data packets.
It's worth noting that nothing you have suggested is an "either/or"
situation. Because caption processing is inexpensive, there isn't any
significant overhead in having multiple AvCCFifo instances in the
pipeline. In other words, if you added a feature to the MOV demuxer,
it wouldn't prevent us from running the packets through an AvCCFifo
instance on the output side. The patch proposed doesn't preclude you
adding such a feature on the demux side in the future.
Devin
--
Devin Heitmueller, Senior Software Engineer
LTN Global Communications
o: +1 (301) 363-1001
w: https://ltnglobal.com e: devin.heitmueller@ltnglobal.com
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [FFmpeg-devel] [PATCH v4 6/6] decklink_enc: add support for playout of 608 captions in MOV files
2023-05-02 14:47 ` Devin Heitmueller
@ 2023-05-03 7:36 ` Lance Wang
0 siblings, 0 replies; 14+ messages in thread
From: Lance Wang @ 2023-05-03 7:36 UTC (permalink / raw)
To: FFmpeg development discussions and patches
On Tue, May 2, 2023 at 10:48 PM Devin Heitmueller <
devin.heitmueller@ltnglobal.com> wrote:
> Hi Lance,
>
> On Sun, Apr 30, 2023 at 7:01 PM Lance Wang <lance.lmwang@gmail.com> wrote:
> > This implementation is limited to decklink SDI output only, If
> possible,
> > can we implement the function from demuxer layer, and then passthrough
> > by SEI side data? By this way, we can convert such stream in streaming
> > to embedded CC to video stream easily also.
>
> I did consider this approach, and it does raise the more fundamental
> issue about trying to minimize the number of ways we have to process
> CC data depending on whether it originated in SEI metadata or in
> separate packets. There are a number of problems with what you are
> proposing though:
>
> 1. There could be multiple CC streams within an MOV file but only a
> single CC stream can be embedded into AVFrame side data. Hence you
> would have to specify some sort of argument to the demux to decide
> which stream to embed. This makes it much more difficult to do things
> like ingest a stream with multiple CC streams and have separate
> outputs with different CC streams. Performing the work on the output
> side allows you to use the standard "-map" mechanism to dictate which
> CC streams are routed to which outputs, and to deliver the content to
> different outputs with different CC streams.
>
> 2. I have use cases in mind where the captions originate from sources
> other than MOV files, where the video framerate is not known (or there
> is no video at all in the source). For example, I want to be able to
> consume video from a TS source while simultaneously demuxing an SCC or
> MCC file and sending the result in the output. In such cases the
> correct rate control for the captions can only be implemented on the
> output side, since in such cases the SCC/MCC demux doesn't have access
> to the corresponding video stream (it won't know the video framerate,
> nor is it able to embed the captions into the AVFrame side data).
>
> I can indeed imagine there are use cases where doing it further up the
> pipeline could be useful. For example, if you were taking in an MOV
> file and wanting to produce a TS where the captions need to be
> embedded as SEI metadata (hence you would need the e608 packets
> converted to AVFrame side data prior to reaching the encoder).
> However I don't see this as a substitute for being able to do it on
> the output side when that is the most flexible approach for those
> other use cases described above.
> Much of this comes down to the fundamental limitations of the ffmpeg
> framework related to being able to move data back/forth between data
> packets and side data. You can't feed data packets into
> AVFilterGraphs. You can't easily combine data from data packets into
> AVFrames carrying video (or extract side data from AVFrames to
> generate data packets), etc. You can't use BSF filters to combine
> data from multiple inputs such as compressed video streams and data
> streams after encoding. I've run across all these limitations over
> the years, and at this point I'm trying to take the least invasive
> approach possible that doesn't require changes to the fundamental
> frameworks for handling data packets.
>
It's worth noting that nothing you have suggested is an "either/or"
> situation. Because caption processing is inexpensive, there isn't any
> significant overhead in having multiple AvCCFifo instances in the
> pipeline. In other words, if you added a feature to the MOV demuxer,
> it wouldn't prevent us from running the packets through an AvCCFifo
> instance on the output side. The patch proposed doesn't preclude you
> adding such a feature on the demux side in the future.
>
>
Thanks for the answer. It's acceptable from my side.
Devin
>
> --
> Devin Heitmueller, Senior Software Engineer
> LTN Global Communications
> o: +1 (301) 363-1001
> w: https://ltnglobal.com e: devin.heitmueller@ltnglobal.com
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>
> To unsubscribe, visit link above, or email
> ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
>
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
^ permalink raw reply [flat|nested] 14+ messages in thread