[FFmpeg-devel] [RFC PATCH 0/5] Properly handle CEA-708 caption data when transcoding

[FFmpeg-devel] [RFC PATCH 0/5] Properly handle CEA-708 caption data when transcoding

[Devin Heitmueller]

This patch series is intended to address long-standing known issues where
CEA-708 caption data is either duplicated or corrupted (i.e. 50% of the data
is lost).  We employ an intermediate queue, which is available to filters that
change the frame rate of the video, which stashes the 608/708 data and
reconstructs properly formed 708 tuples on the output side (injecting
appropriate padding as needed).

The common functions are implemented in libavutil, but I am open to
suggestions.  While all the users in this patch series are within
libavfilter, there are use cases where this functionality would
be useful within libavformat (which is why libavutil was chosen).

Comments/feedback are welcomed.

Thanks,

Devin

Devin Heitmueller (5):
  ccfifo: Properly handle CEA-708 captions through framerate conversion
  vf_fps: properly preserve CEA-708 captions
  yadif: Properly preserve CEA-708 closed captions
  tinterlace: Properly preserve CEA-708 closed captions
  vf_ccrepack: Add new filter to repack CEA-708 side data

 doc/filters.texi            |  10 ++
 libavfilter/Makefile        |   1 +
 libavfilter/allfilters.c    |   1 +
 libavfilter/tinterlace.h    |   2 +
 libavfilter/vf_bwdif.c      |   7 ++
 libavfilter/vf_ccrepack.c   |  95 ++++++++++++++++++
 libavfilter/vf_fps.c        |   9 +-
 libavfilter/vf_tinterlace.c |  10 ++
 libavfilter/vf_yadif.c      |   6 ++
 libavfilter/vf_yadif_cuda.c |   8 ++
 libavfilter/yadif.h         |   2 +
 libavfilter/yadif_common.c  |   5 +
 libavutil/Makefile          |   2 +
 libavutil/ccfifo.c          | 192 ++++++++++++++++++++++++++++++++++++
 libavutil/ccfifo.h          |  85 ++++++++++++++++
 15 files changed, 434 insertions(+), 1 deletion(-)
 create mode 100644 libavfilter/vf_ccrepack.c
 create mode 100644 libavutil/ccfifo.c
 create mode 100644 libavutil/ccfifo.h

-- 
2.35.1.655.ga68dfadae5

_______________________________________________
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] [RFC PATCH 1/5] ccfifo: Properly handle CEA-708 captions through framerate conversion

[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.

Signed-off-by: Devin Heitmueller <dheitmueller@ltnglobal.com>
---
 libavutil/Makefile |   2 +
 libavutil/ccfifo.c | 192 +++++++++++++++++++++++++++++++++++++++++++++
 libavutil/ccfifo.h |  85 ++++++++++++++++++++
 3 files changed, 279 insertions(+)
 create mode 100644 libavutil/ccfifo.c
 create mode 100644 libavutil/ccfifo.h

diff --git a/libavutil/Makefile b/libavutil/Makefile
index dc9012f9a8..b6115859fa 100644
--- a/libavutil/Makefile
+++ b/libavutil/Makefile
@@ -17,6 +17,7 @@ HEADERS = adler32.h                                                     \
           buffer.h                                                      \
           cast5.h                                                       \
           camellia.h                                                    \
+          ccfifo.h                                                      \
           channel_layout.h                                              \
           common.h                                                      \
           cpu.h                                                         \
@@ -113,6 +114,7 @@ OBJS = adler32.o                                                        \
        buffer.o                                                         \
        cast5.o                                                          \
        camellia.o                                                       \
+       ccfifo.o                                                         \
        channel_layout.o                                                 \
        cpu.o                                                            \
        crc.o                                                            \
diff --git a/libavutil/ccfifo.c b/libavutil/ccfifo.c
new file mode 100644
index 0000000000..98ee365eee
--- /dev/null
+++ b/libavutil/ccfifo.c
@@ -0,0 +1,192 @@
+/*
+ * 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;
+    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 av_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 *av_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;
+
+    printf("cc_fifo framerate=%d/%d\n", framerate->num, framerate->den);
+    /* 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 < (sizeof(cc_lookup_vals) / sizeof(struct cc_lookup)); 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) {
+        av_log(ccf->log_ctx, AV_LOG_WARNING, "cc_fifo cannot transcode captions fps=%d/%d\n",
+               framerate->num, framerate->den);
+        return NULL;
+    }
+
+    return ccf;
+
+error:
+    av_ccfifo_freep(&ccf);
+    return NULL;
+}
+
+int av_ccfifo_inject(AVCCFifo *ccf, AVFrame *frame)
+{
+    AVFrameSideData *sd;
+    int cc_filled = 0;
+    int i;
+
+    if (!ccf)
+        return 0;
+
+    if (ccf->cc_detected == 0 || ccf->expected_cc_count == 0)
+        return 0;
+
+    sd = av_frame_new_side_data(frame, AV_FRAME_DATA_A53_CC,
+                                ccf->expected_cc_count * CC_BYTES_PER_ENTRY);
+    if (!sd)
+        return 0;
+
+    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, &sd->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, &sd->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) {
+        sd->data[cc_filled * CC_BYTES_PER_ENTRY]     = 0xfa;
+        sd->data[cc_filled * CC_BYTES_PER_ENTRY + 1] = 0x00;
+        sd->data[cc_filled * CC_BYTES_PER_ENTRY + 2] = 0x00;
+        cc_filled++;
+    }
+    printf("enqueue cc_filled=%d\n", cc_filled);
+    return 0;
+}
+
+int av_ccfifo_extract(AVCCFifo *ccf, AVFrame *frame)
+{
+    int i;
+
+    if (!ccf)
+        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) {
+            uint8_t *cc_bytes = side_data->data;
+            int cc_count = side_data->size / CC_BYTES_PER_ENTRY;
+            ccf->cc_detected = 1;
+
+            for (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);
+                }
+            }
+            printf("dequeue cc_filled=%d\n", cc_count);
+            /* 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/libavutil/ccfifo.h b/libavutil/ccfifo.h
new file mode 100644
index 0000000000..ad48c0a39c
--- /dev/null
+++ b/libavutil/ccfifo.h
@@ -0,0 +1,85 @@
+/*
+ * 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 AVUTIL_CCFIFO_H
+#define AVUTIL_CCFIFO_H
+
+#include "avutil.h"
+#include "frame.h"
+#include "fifo.h"
+
+typedef struct AVCCFifo AVCCFifo;
+
+/**
+ * Allocate an AVCCFifo.
+ *
+ * @param sample_fmt  sample format
+ * @param channels    number of channels
+ * @param nb_samples  initial allocation size, in samples
+ * @return            newly allocated AVCCFifo, or NULL on error
+ */
+AVCCFifo *av_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 av_ccfifo_freep(AVCCFifo **ccf);
+
+
+/**
+ * Read a frame into a CC Fifo
+ *
+ * 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 av_ccfifo_extract(AVCCFifo *af, AVFrame *frame);
+
+/**
+ * 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 av_ccfifo_inject(AVCCFifo *af, AVFrame *frame);
+
+#endif /* AVUTIL_CCFIFO_H */
-- 
2.35.1.655.ga68dfadae5

_______________________________________________
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] [RFC PATCH 2/5] vf_fps: properly preserve CEA-708 captions

[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 051d278f54..d2fab09c66 100644
--- a/libavfilter/vf_fps.c
+++ b/libavfilter/vf_fps.c
@@ -30,6 +30,7 @@
 #include <stdint.h>
 
 #include "libavutil/avassert.h"
+#include "libavutil/ccfifo.h"
 #include "libavutil/eval.h"
 #include "libavutil/mathematics.h"
 #include "libavutil/opt.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);
     }
+    av_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 = av_ccfifo_alloc(&outlink->frame_rate, ctx)))
+        av_log(ctx, AV_LOG_VERBOSE, "Failure to setup CC FIFO queue.  Captions will be passed through\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);
 
+    av_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);
+        av_ccfifo_inject(s->cc_fifo, frame);
         frame->pts = s->next_pts++;
         frame->duration = 1;
 
-- 
2.35.1.655.ga68dfadae5

_______________________________________________
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] [RFC PATCH 3/5] yadif: Properly preserve CEA-708 closed captions

[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 65c617ebb3..de937d83cf 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);
+    av_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 = av_ccfifo_alloc(&link->frame_rate, ctx)))
+        av_log(ctx, AV_LOG_VERBOSE, "Failure to setup CC FIFO queue.  Captions will be passed through\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 1be02de1a9..b51d21f6ff 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);
+    av_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 = av_ccfifo_alloc(&outlink->frame_rate, ctx)))
+        av_log(ctx, AV_LOG_VERBOSE, "Failure to setup CC FIFO queue.  Captions will be passed through\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 685b8a2035..d96ec391a0 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)
+        av_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 = av_cc_fifo_alloc(&outlink->frame_rate, ctx)))
+        av_log(ctx, AV_LOG_VERBOSE, "Failure to setup CC FIFO queue.  Captions will be passed through\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 c928911b35..ccd4304860 100644
--- a/libavfilter/yadif.h
+++ b/libavfilter/yadif.h
@@ -21,6 +21,7 @@
 
 #include "libavutil/opt.h"
 #include "libavutil/pixdesc.h"
+#include "libavutil/ccfifo.h"
 #include "avfilter.h"
 
 enum YADIFMode {
@@ -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 a10cf7a17f..f95b8c10d3 100644
--- a/libavfilter/yadif_common.c
+++ b/libavfilter/yadif_common.c
@@ -43,6 +43,7 @@ static int return_frame(AVFilterContext *ctx, int is_second)
             return AVERROR(ENOMEM);
 
         av_frame_copy_props(yadif->out, yadif->cur);
+        av_ccfifo_inject(yadif->cc_fifo, yadif->out);
         yadif->out->interlaced_frame = 0;
         if (yadif->current_field == YADIF_FIELD_BACK_END)
             yadif->current_field = YADIF_FIELD_END;
@@ -96,6 +97,8 @@ int ff_yadif_filter_frame(AVFilterLink *link, AVFrame *frame)
 
     av_assert0(frame);
 
+    av_ccfifo_extract(yadif->cc_fifo, frame);
+
     if (yadif->frame_pending)
         return_frame(ctx, 1);
 
@@ -137,6 +140,7 @@ int ff_yadif_filter_frame(AVFilterLink *link, AVFrame *frame)
         if (!yadif->out)
             return AVERROR(ENOMEM);
 
+        av_ccfifo_inject(yadif->cc_fifo, yadif->out);
         av_frame_free(&yadif->prev);
         if (yadif->out->pts != AV_NOPTS_VALUE)
             yadif->out->pts *= 2;
@@ -148,6 +152,7 @@ int ff_yadif_filter_frame(AVFilterLink *link, AVFrame *frame)
         return AVERROR(ENOMEM);
 
     av_frame_copy_props(yadif->out, yadif->cur);
+    av_ccfifo_inject(yadif->cc_fifo, yadif->out);
     yadif->out->interlaced_frame = 0;
 
     if (yadif->out->pts != AV_NOPTS_VALUE)
-- 
2.35.1.655.ga68dfadae5

_______________________________________________
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".

Re: [FFmpeg-devel] [RFC PATCH 3/5] yadif: Properly preserve CEA-708 closed captions

[Dennis Mungai]

On Fri, 17 Mar 2023, 23:11 Devin Heitmueller, <
devin.heitmueller@ltnglobal.com> wrote:

> 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 65c617ebb3..de937d83cf 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);
> +    av_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 = av_ccfifo_alloc(&link->frame_rate, ctx)))
> +        av_log(ctx, AV_LOG_VERBOSE, "Failure to setup CC FIFO queue.
> Captions will be passed through\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 1be02de1a9..b51d21f6ff 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);
> +    av_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 = av_ccfifo_alloc(&outlink->frame_rate, ctx)))
> +        av_log(ctx, AV_LOG_VERBOSE, "Failure to setup CC FIFO queue.
> Captions will be passed through\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 685b8a2035..d96ec391a0 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)
> +        av_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 = av_cc_fifo_alloc(&outlink->frame_rate, ctx)))
> +        av_log(ctx, AV_LOG_VERBOSE, "Failure to setup CC FIFO queue.
> Captions will be passed through\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 c928911b35..ccd4304860 100644
> --- a/libavfilter/yadif.h
> +++ b/libavfilter/yadif.h
> @@ -21,6 +21,7 @@
>
>  #include "libavutil/opt.h"
>  #include "libavutil/pixdesc.h"
> +#include "libavutil/ccfifo.h"
>  #include "avfilter.h"
>
>  enum YADIFMode {
> @@ -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 a10cf7a17f..f95b8c10d3 100644
> --- a/libavfilter/yadif_common.c
> +++ b/libavfilter/yadif_common.c
> @@ -43,6 +43,7 @@ static int return_frame(AVFilterContext *ctx, int
> is_second)
>              return AVERROR(ENOMEM);
>
>          av_frame_copy_props(yadif->out, yadif->cur);
> +        av_ccfifo_inject(yadif->cc_fifo, yadif->out);
>          yadif->out->interlaced_frame = 0;
>          if (yadif->current_field == YADIF_FIELD_BACK_END)
>              yadif->current_field = YADIF_FIELD_END;
> @@ -96,6 +97,8 @@ int ff_yadif_filter_frame(AVFilterLink *link, AVFrame
> *frame)
>
>      av_assert0(frame);
>
> +    av_ccfifo_extract(yadif->cc_fifo, frame);
> +
>      if (yadif->frame_pending)
>          return_frame(ctx, 1);
>
> @@ -137,6 +140,7 @@ int ff_yadif_filter_frame(AVFilterLink *link, AVFrame
> *frame)
>          if (!yadif->out)
>              return AVERROR(ENOMEM);
>
> +        av_ccfifo_inject(yadif->cc_fifo, yadif->out);
>          av_frame_free(&yadif->prev);
>          if (yadif->out->pts != AV_NOPTS_VALUE)
>              yadif->out->pts *= 2;
> @@ -148,6 +152,7 @@ int ff_yadif_filter_frame(AVFilterLink *link, AVFrame
> *frame)
>          return AVERROR(ENOMEM);
>
>      av_frame_copy_props(yadif->out, yadif->cur);
> +    av_ccfifo_inject(yadif->cc_fifo, yadif->out);
>      yadif->out->interlaced_frame = 0;
>
>      if (yadif->out->pts != AV_NOPTS_VALUE)
> --
> 2.35.1.655.ga68dfadae5
>


Hello Devin,

How would this be propagated (or replicated) in other deinterlacers, namely
Intel's OneVPL based deinterlace_qsv, vpp_qsv and VAAPI's
deinterlace_vaapi?

>
_______________________________________________
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".

Re: [FFmpeg-devel] [RFC PATCH 3/5] yadif: Properly preserve CEA-708 closed captions

[Devin Heitmueller]

Hello Dennis,

On Sat, Mar 18, 2023 at 11:48 AM Dennis Mungai <dmngaie@gmail.com> wrote:
> Hello Devin,
>
> How would this be propagated (or replicated) in other deinterlacers, namely Intel's OneVPL based deinterlace_qsv, vpp_qsv and VAAPI's deinterlace_vaapi?

So I was focused mainly on the video filters that I use regularly,
which didn't include the hardware accelerated ones.  That said, if you
look at the patches themselves, they're actually only a few lines of
code that need to be added to the individual filters, so the
maintainers of those filters should have little difficulty reusing the
framework.

I would be quite happy to see other filters reuse the same mechanism,
rather than reinventing the wheel, and I intentionally tried to
minimize the amount of code required by modules to make use of the
framework.

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] [RFC PATCH 4/5] tinterlace: Properly preserve CEA-708 closed captions

[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.

Signed-off-by: Devin Heitmueller <dheitmueller@ltnglobal.com>
---
 libavfilter/tinterlace.h    |  2 ++
 libavfilter/vf_tinterlace.c | 10 ++++++++++
 2 files changed, 12 insertions(+)

diff --git a/libavfilter/tinterlace.h b/libavfilter/tinterlace.h
index 37b6c10c08..30574c2ebf 100644
--- a/libavfilter/tinterlace.h
+++ b/libavfilter/tinterlace.h
@@ -30,6 +30,7 @@
 #include "libavutil/bswap.h"
 #include "libavutil/opt.h"
 #include "libavutil/pixdesc.h"
+#include "libavutil/ccfifo.h"
 #include "drawutils.h"
 #include "avfilter.h"
 
@@ -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 032629279a..260386a889 100644
--- a/libavfilter/vf_tinterlace.c
+++ b/libavfilter/vf_tinterlace.c
@@ -291,6 +291,9 @@ static int config_out_props(AVFilterLink *outlink)
 #endif
     }
 
+    if (!(tinterlace->cc_fifo = av_ccfifo_alloc(&outlink->frame_rate, ctx)))
+        av_log(ctx, AV_LOG_VERBOSE, "Failure to setup CC FIFO queue.  Captions will be passed through\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 +378,8 @@ static int filter_frame(AVFilterLink *inlink, AVFrame *picref)
     tinterlace->cur  = tinterlace->next;
     tinterlace->next = picref;
 
+    av_ccfifo_extract(tinterlace->cc_fifo, picref);
+
     cur = tinterlace->cur;
     next = tinterlace->next;
     /* we need at least two frames */
@@ -390,6 +395,7 @@ static int filter_frame(AVFilterLink *inlink, AVFrame *picref)
         if (!out)
             return AVERROR(ENOMEM);
         av_frame_copy_props(out, cur);
+        av_ccfifo_inject(tinterlace->cc_fifo, out);
         out->height = outlink->h;
         out->interlaced_frame = 1;
         out->top_field_first = 1;
@@ -423,6 +429,7 @@ static int filter_frame(AVFilterLink *inlink, AVFrame *picref)
         if (!out)
             return AVERROR(ENOMEM);
         av_frame_copy_props(out, cur);
+        av_ccfifo_inject(tinterlace->cc_fifo, out);
         out->height = outlink->h;
         out->sample_aspect_ratio = av_mul_q(cur->sample_aspect_ratio, av_make_q(2, 1));
 
@@ -459,6 +466,7 @@ static int filter_frame(AVFilterLink *inlink, AVFrame *picref)
         if (!out)
             return AVERROR(ENOMEM);
         av_frame_copy_props(out, cur);
+        av_ccfifo_inject(tinterlace->cc_fifo, out);
         out->interlaced_frame = 1;
         out->top_field_first = tff;
 
@@ -481,6 +489,7 @@ static int filter_frame(AVFilterLink *inlink, AVFrame *picref)
         out = av_frame_clone(cur);
         if (!out)
             return AVERROR(ENOMEM);
+        av_ccfifo_inject(tinterlace->cc_fifo, out);
         out->interlaced_frame = 1;
         if (cur->pts != AV_NOPTS_VALUE)
             out->pts = cur->pts*2;
@@ -495,6 +504,7 @@ static int filter_frame(AVFilterLink *inlink, AVFrame *picref)
         if (!out)
             return AVERROR(ENOMEM);
         av_frame_copy_props(out, next);
+        av_ccfifo_inject(tinterlace->cc_fifo, out);
         out->interlaced_frame = 1;
         out->top_field_first = !tff;
 
-- 
2.35.1.655.ga68dfadae5

_______________________________________________
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".

Re: [FFmpeg-devel] [RFC PATCH 4/5] tinterlace: Properly preserve CEA-708 closed captions

[Thomas Mundt]

Hi Devin,

Am Fr., 17. März 2023 um 21:10 Uhr schrieb Devin Heitmueller <
devin.heitmueller@ltnglobal.com>:

> Because the interlacing filter halves the effective framerate, we
> need to ensure that no CEA-708 data is lost as frames are merged.
>

Not all modes of tinterlace halve the frame rate. mergex2 and pad keep it,
interlacex2 even doubles it.


> Make use of the new ccfifo mechanism to ensure that caption data
> is properly preserved as frames pass through the filter.
>
> Signed-off-by: Devin Heitmueller <dheitmueller@ltnglobal.com>
> ---
>  libavfilter/tinterlace.h    |  2 ++
>  libavfilter/vf_tinterlace.c | 10 ++++++++++
>  2 files changed, 12 insertions(+)
>
> diff --git a/libavfilter/tinterlace.h b/libavfilter/tinterlace.h
> index 37b6c10c08..30574c2ebf 100644
> --- a/libavfilter/tinterlace.h
> +++ b/libavfilter/tinterlace.h
> @@ -30,6 +30,7 @@
>  #include "libavutil/bswap.h"
>  #include "libavutil/opt.h"
>  #include "libavutil/pixdesc.h"
> +#include "libavutil/ccfifo.h"
>  #include "drawutils.h"
>  #include "avfilter.h"
>
> @@ -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 032629279a..260386a889 100644
> --- a/libavfilter/vf_tinterlace.c
> +++ b/libavfilter/vf_tinterlace.c
> @@ -291,6 +291,9 @@ static int config_out_props(AVFilterLink *outlink)
>  #endif
>      }
>
> +    if (!(tinterlace->cc_fifo = av_ccfifo_alloc(&outlink->frame_rate,
> ctx)))
> +        av_log(ctx, AV_LOG_VERBOSE, "Failure to setup CC FIFO queue.
> Captions will be passed through\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 +378,8 @@ static int filter_frame(AVFilterLink *inlink, AVFrame
> *picref)
>      tinterlace->cur  = tinterlace->next;
>      tinterlace->next = picref;
>
> +    av_ccfifo_extract(tinterlace->cc_fifo, picref);
> +
>      cur = tinterlace->cur;
>      next = tinterlace->next;
>      /* we need at least two frames */
> @@ -390,6 +395,7 @@ static int filter_frame(AVFilterLink *inlink, AVFrame
> *picref)
>          if (!out)
>              return AVERROR(ENOMEM);
>          av_frame_copy_props(out, cur);
> +        av_ccfifo_inject(tinterlace->cc_fifo, out);
>          out->height = outlink->h;
>          out->interlaced_frame = 1;
>          out->top_field_first = 1;
> @@ -423,6 +429,7 @@ static int filter_frame(AVFilterLink *inlink, AVFrame
> *picref)
>

Are the drop modes missing?
Would it perhaps be clearer to put av_ccfifo_inject before ff_filter_frame?


>          if (!out)
>              return AVERROR(ENOMEM);
>          av_frame_copy_props(out, cur);
> +        av_ccfifo_inject(tinterlace->cc_fifo, out);
>          out->height = outlink->h;
>          out->sample_aspect_ratio = av_mul_q(cur->sample_aspect_ratio,
> av_make_q(2, 1));
>
> @@ -459,6 +466,7 @@ static int filter_frame(AVFilterLink *inlink, AVFrame
> *picref)
>          if (!out)
>              return AVERROR(ENOMEM);
>          av_frame_copy_props(out, cur);
> +        av_ccfifo_inject(tinterlace->cc_fifo, out);
>          out->interlaced_frame = 1;
>          out->top_field_first = tff;
>
> @@ -481,6 +489,7 @@ static int filter_frame(AVFilterLink *inlink, AVFrame
> *picref)
>          out = av_frame_clone(cur);
>          if (!out)
>              return AVERROR(ENOMEM);
> +        av_ccfifo_inject(tinterlace->cc_fifo, out);
>          out->interlaced_frame = 1;
>          if (cur->pts != AV_NOPTS_VALUE)
>              out->pts = cur->pts*2;
> @@ -495,6 +504,7 @@ static int filter_frame(AVFilterLink *inlink, AVFrame
> *picref)
>          if (!out)
>              return AVERROR(ENOMEM);
>          av_frame_copy_props(out, next);
> +        av_ccfifo_inject(tinterlace->cc_fifo, out);
>          out->interlaced_frame = 1;
>          out->top_field_first = !tff;
>
> --
>

Regards,
Thomas
_______________________________________________
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".

Re: [FFmpeg-devel] [RFC PATCH 4/5] tinterlace: Properly preserve CEA-708 closed captions

[Devin Heitmueller]

On Sat, Mar 18, 2023 at 1:12 PM Thomas Mundt <tmundt75@gmail.com> wrote:
> Not all modes of tinterlace halve the frame rate. mergex2 and pad keep it, interlacex2 even doubles it.

Sorry, I should have been more clear with the patch description in
that I realize that interlacing doesn't necessarily halve the
framerate, depending on the mode.  That said, I was pretty sure I got
it working with all the code paths, but if you think I've missed one
or more then please let me know.  Or if you have an alternative patch
which you think is cleaner for the tinterlace filter, I would be happy
to review it.

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".

Re: [FFmpeg-devel] [RFC PATCH 4/5] tinterlace: Properly preserve CEA-708 closed captions

[Thomas Mundt]

Am So., 19. März 2023 um 00:22 Uhr schrieb Devin Heitmueller <
devin.heitmueller@ltnglobal.com>:

> On Sat, Mar 18, 2023 at 1:12 PM Thomas Mundt <tmundt75@gmail.com> wrote:
> > Not all modes of tinterlace halve the frame rate. mergex2 and pad keep
> it, interlacex2 even doubles it.
>
> Sorry, I should have been more clear with the patch description in
> that I realize that interlacing doesn't necessarily halve the
> framerate, depending on the mode.  That said, I was pretty sure I got
> it working with all the code paths, but if you think I've missed one
> or more then please let me know.  Or if you have an alternative patch
> which you think is cleaner for the tinterlace filter, I would be happy
> to review it.
>
>
I haven't worked with closed captions yet and also don't have the expertise
to test them reliably.
I just would like to roughly understand as maintainer of the tinterlace
filter how your patch works in order not to break the functionality in
future changes.
Looking through it, I wonder for example why av_ccfifo_inject is set for
the interleave modes and not for the drop modes.
Under which conditions must av_ccfifo_inject be set? Whenever a frame is
created at the output of the filter, or are there other conditions?

Regards,
Thomas
_______________________________________________
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".

Re: [FFmpeg-devel] [RFC PATCH 4/5] tinterlace: Properly preserve CEA-708 closed captions

[Devin Heitmueller]

On Sun, Mar 19, 2023 at 12:50 PM Thomas Mundt <tmundt75@gmail.com> wrote:
> Looking through it, I wonder for example why av_ccfifo_inject is set for the interleave modes and not for the drop modes.
> Under which conditions must av_ccfifo_inject be set? Whenever a frame is created at the output of the filter, or are there other conditions?

I'll have to double check the logic in tinterlace.  The intention is
that whenever a frame arrives on the input we should be calling
extract, and whenever we generate a frame on the output we should be
calling inject.  If I've missed a case, then I will have to take care
of that.

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] [RFC PATCH 5/5] vf_ccrepack: Add new filter to repack CEA-708 side data

[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 | 95 +++++++++++++++++++++++++++++++++++++++
 4 files changed, 107 insertions(+)
 create mode 100644 libavfilter/vf_ccrepack.c

diff --git a/doc/filters.texi b/doc/filters.texi
index 77b594f69c..a84fef0e4d 100644
--- a/doc/filters.texi
+++ b/doc/filters.texi
@@ -8904,6 +8904,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 b3d3d981dd..eed2460ffc 100644
--- a/libavfilter/Makefile
+++ b/libavfilter/Makefile
@@ -211,6 +211,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 d7db46c2af..b38550b358 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 0000000000..ab8473e7c1
--- /dev/null
+++ b/libavfilter/vf_ccrepack.c
@@ -0,0 +1,95 @@
+/*
+ * 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 "libavutil/opt.h"
+#include "libavutil/ccfifo.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 = av_ccfifo_alloc(&link->frame_rate, ctx)))
+        av_log(ctx, AV_LOG_VERBOSE, "Failure to setup CC FIFO queue.  Captions will be passed through\n");
+
+    return 0;
+}
+
+static int filter_frame(AVFilterLink *inlink, AVFrame *frame)
+{
+    CCRepackContext *ctx = inlink->dst->priv;
+    AVFilterLink *outlink = inlink->dst->outputs[0];
+
+    av_ccfifo_extract(ctx->cc_fifo, frame);
+    av_ccfifo_inject(ctx->cc_fifo, frame);
+
+    return ff_filter_frame(outlink, frame);
+}
+
+static av_cold void uninit(AVFilterContext *ctx)
+{
+    CCRepackContext *s = ctx->priv;
+    av_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),
+};
-- 
2.35.1.655.ga68dfadae5

_______________________________________________
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".

Re: [FFmpeg-devel] [RFC PATCH 0/5] Properly handle CEA-708 caption data when transcoding

[BUCCIANTINI Francesco - ADECCO]

This is indeed very welcome.
Being able to re-encode from 25i to 50p while preserving subtitles has been something I've been trying to do for a while, so seeing this patch for CEA-708 is definitely a nice thing.

I have a question, though: will something similar be done for OP-47 subtitles too?
Currently they suffer the same issue when a re-encode changes the frame rate... :(

On Mar 17, 2023 21:10, Devin Heitmueller <devin.heitmueller@ltnglobal.com> wrote:
ATTENZIONE: mail esterna. Se il messaggio è sospetto, invialo come allegato a bedatasmart@skytv.it<mailto:bedatasmart@skytv.it>


This patch series is intended to address long-standing known issues where
CEA-708 caption data is either duplicated or corrupted (i.e. 50% of the data
is lost).  We employ an intermediate queue, which is available to filters that
change the frame rate of the video, which stashes the 608/708 data and
reconstructs properly formed 708 tuples on the output side (injecting
appropriate padding as needed).

The common functions are implemented in libavutil, but I am open to
suggestions.  While all the users in this patch series are within
libavfilter, there are use cases where this functionality would
be useful within libavformat (which is why libavutil was chosen).

Comments/feedback are welcomed.

Thanks,

Devin

Devin Heitmueller (5):
  ccfifo: Properly handle CEA-708 captions through framerate conversion
  vf_fps: properly preserve CEA-708 captions
  yadif: Properly preserve CEA-708 closed captions
  tinterlace: Properly preserve CEA-708 closed captions
  vf_ccrepack: Add new filter to repack CEA-708 side data

 doc/filters.texi            |  10 ++
 libavfilter/Makefile        |   1 +
 libavfilter/allfilters.c    |   1 +
 libavfilter/tinterlace.h    |   2 +
 libavfilter/vf_bwdif.c      |   7 ++
 libavfilter/vf_ccrepack.c   |  95 ++++++++++++++++++
 libavfilter/vf_fps.c        |   9 +-
 libavfilter/vf_tinterlace.c |  10 ++
 libavfilter/vf_yadif.c      |   6 ++
 libavfilter/vf_yadif_cuda.c |   8 ++
 libavfilter/yadif.h         |   2 +
 libavfilter/yadif_common.c  |   5 +
 libavutil/Makefile          |   2 +
 libavutil/ccfifo.c          | 192 ++++++++++++++++++++++++++++++++++++
 libavutil/ccfifo.h          |  85 ++++++++++++++++
 15 files changed, 434 insertions(+), 1 deletion(-)
 create mode 100644 libavfilter/vf_ccrepack.c
 create mode 100644 libavutil/ccfifo.c
 create mode 100644 libavutil/ccfifo.h

--
2.35.1.655.ga68dfadae5

_______________________________________________
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".

Re: [FFmpeg-devel] [RFC PATCH 0/5] Properly handle CEA-708 caption data when transcoding

[Devin Heitmueller]

On Fri, Mar 17, 2023 at 7:23 PM BUCCIANTINI Francesco - ADECCO
<francesco.bucciantini.adecco@skytv.it> wrote:
>
> This is indeed very welcome.
> Being able to re-encode from 25i to 50p while preserving subtitles has been something I've been trying to do for a while, so seeing this patch for CEA-708 is definitely a nice thing.
>
> I have a question, though: will something similar be done for OP-47 subtitles too?
> Currently they suffer the same issue when a re-encode changes the frame rate... :(

I don't have any plans to work on OP-47, and I'm not sure if anyone
else does.  While I've done a bit of work with it in the SDI domain,
I'm not super familiar with how it's treated by ffmpeg, or whether a
similar approach is appropriate for dealing with it.  I can say that
the FIFO here is highly specific to the CEA-708 standard, as we have
to disassemble and reassemble the payloads to meet the specification
(i.e. not applicable to OP-47).

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".