Re: [FFmpeg-devel] [PATCH v3 1/5] ccfifo: Properly handle CEA-708 captions through framerate conversion

[Lance Wang <lance.lmwang@gmail.com>]

On Fri, Apr 21, 2023 at 9:29 PM Devin Heitmueller <
devin.heitmueller@ltnglobal.com> wrote:

> 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>
> ---
>  libavfilter/Makefile |   1 +
>  libavfilter/ccfifo.c | 191
> +++++++++++++++++++++++++++++++++++++++++++++++++++
>  libavfilter/ccfifo.h |  85 +++++++++++++++++++++++
>  3 files changed, 277 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..3f8be57
> --- /dev/null
> +++ b/libavfilter/ccfifo.c
> @@ -0,0 +1,191 @@
> +/*
> + * 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 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 < (sizeof(cc_lookup_vals) / sizeof(struct cc_lookup));
> i++) {
>

I prefer to use FF_ARRAY_ELEMS here.


> +        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;
>

why not use goto error?  I feel ccf should be freed.


> +    }
> +
> +    return ccf;
> +
> +error:
> +    ff_ccfifo_freep(&ccf);
> +    return NULL;
> +}
> +
> +int ff_ccfifo_inject(AVCCFifo *ccf, AVFrame *frame)
> +{
> +    AVFrameSideData *sd;
> +    int cc_filled = 0;
> +    int i;
> +
> +    if (!ccf)
> +        return 0;
>

+ * @return            Zero on success, or negative AVERROR
+ *                    code on failure.

 why not return error code?  the same to other failure condition.

+
> +    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;
>

same.


> +
> +    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++;
> +    }
> +
> +    return 0;
> +}
> +
> +int ff_ccfifo_extract(AVCCFifo *ccf, AVFrame *frame)
> +{
> +    int i;
> +
> +    if (!ccf)
> +        return 0;
>

+ * @return            Zero on success, or negative AVERROR
+ *                    code on failure.
same question.


> +
> +    /* 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);
> +                }
> +            }
> +
> +            /* 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..70e8b9b
> --- /dev/null
> +++ b/libavfilter/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
>

AVUTIL is wrong here


> +
> +#include "libavutil/avutil.h"
> +#include "libavutil/frame.h"
> +#include "libavutil/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
>

This is mismatch comments


> + * @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);
> +
> +
> +/**
> + * Read a frame into a CC Fifo
>

It's not clear I think.


> + *
> + * 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 *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 ff_ccfifo_inject(AVCCFifo *af, AVFrame *frame);
> +
> +#endif /* AVUTIL_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".
>
_______________________________________________
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".