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

[Paul B Mahol <onemda@gmail.com>]

On Thu, May 11, 2023 at 4:14 PM Lance Wang <lance.lmwang@gmail.com> wrote:

> On Wed, May 10, 2023 at 7:08 PM Lance Wang <lance.lmwang@gmail.com> wrote:
>
> >
> >
> > On Sun, May 7, 2023 at 3:03 PM Lance Wang <lance.lmwang@gmail.com>
> wrote:
> >
> >> LGTM for the patchset.
> >>
> >>
> > I will help to apply the patchset tomorrow if no further comments. thx.
> >
>
> applied, thanks for the effort.
>

Breaks build.


>
>
> >
> >
> >> On Sat, May 6, 2023 at 2:13 AM 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.
> >>>
> >>> Thanks to Lance Wang <lance.lmwang@gmail.com>, Anton
> >>> Khirnov <anton@khirnov.net>, and Michael Niedermayer <
> >>> michael@niedermayer.cc>
> >>> for providing review/feedback.
> >>>
> >>> Signed-off-by: Devin Heitmueller <dheitmueller@ltnglobal.com>
> >>> ---
> >>>  libavfilter/Makefile |   1 +
> >>>  libavfilter/ccfifo.c | 224
> >>> +++++++++++++++++++++++++++++++++++++++++++++++++++
> >>>  libavfilter/ccfifo.h | 110 +++++++++++++++++++++++++
> >>>  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 482aeaf..68c8f14 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..5fb68ce
> >>> --- /dev/null
> >>> +++ b/libavfilter/ccfifo.c
> >>> @@ -0,0 +1,224 @@
> >>> +/*
> >>> + * 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;
> >>> +    AVRational framerate;
> >>> +    int expected_cc_count;
> >>> +    int expected_608;
> >>> +    int cc_detected;
> >>> +    int passthrough;
> >>> +    int passthrough_warning;
> >>> +    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)
> >>> +{
> >>> +    AVCCFifo *tmp = *ccf;
> >>> +    if (tmp) {
> >>> +        av_fifo_freep2(&tmp->cc_608_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;
> >>> +
> >>> +    ccf->log_ctx = log_ctx;
> >>> +    ccf->framerate = framerate;
> >>> +
> >>> +    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 */
> >>> +        ccf->passthrough = 1;
> >>> +    }
> >>> +
> >>> +    return ccf;
> >>> +
> >>> +error:
> >>> +    ff_ccfifo_freep(&ccf);
> >>> +    return NULL;
> >>> +}
> >>> +
> >>> +int ff_ccfifo_getoutputsize(AVCCFifo *ccf)
> >>> +{
> >>> +    return ccf->expected_cc_count * CC_BYTES_PER_ENTRY;
> >>> +}
> >>> +
> >>> +int ff_ccfifo_ccdetected(AVCCFifo *ccf)
> >>> +{
> >>> +    return ccf->cc_detected;
> >>> +}
> >>> +
> >>> +int ff_ccfifo_injectbytes(AVCCFifo *ccf, uint8_t *cc_data, size_t len)
> >>> +{
> >>> +    int cc_608_tuples = 0;
> >>> +    int cc_708_tuples = 0;
> >>> +    int cc_filled = 0;
> >>> +
> >>> +    if (ccf->passthrough) {
> >>> +        return 0;
> >>> +    }
> >>> +
> >>> +    if (len < ff_ccfifo_getoutputsize(ccf)) {
> >>> +        return AVERROR(EINVAL);
> >>> +    }
> >>> +
> >>> +    /* Insert any available data from the 608 FIFO */
> >>> +    if (ccf->expected_608 <= av_fifo_can_read(ccf->cc_608_fifo))
> >>> +        cc_608_tuples = ccf->expected_608;
> >>> +    else
> >>> +        cc_608_tuples = av_fifo_can_read(ccf->cc_608_fifo);
> >>> +    av_fifo_read(ccf->cc_608_fifo, cc_data, cc_608_tuples);
> >>> +    cc_filled += cc_608_tuples;
> >>> +
> >>> +    /* Insert any available data from the 708 FIFO */
> >>> +    if ((ccf->expected_cc_count - cc_filled) <=
> >>> av_fifo_can_read(ccf->cc_708_fifo))
> >>> +        cc_708_tuples = ccf->expected_cc_count - cc_filled;
> >>> +    else
> >>> +        cc_708_tuples = av_fifo_can_read(ccf->cc_708_fifo);
> >>> +    av_fifo_read(ccf->cc_708_fifo, &cc_data[cc_filled *
> >>> CC_BYTES_PER_ENTRY], cc_708_tuples);
> >>> +    cc_filled += cc_708_tuples;
> >>> +
> >>> +    /* 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++;
> >>> +    }
> >>> +
> >>> +    return 0;
> >>> +}
> >>> +
> >>> +int ff_ccfifo_inject(AVCCFifo *ccf, AVFrame *frame)
> >>> +{
> >>> +    AVFrameSideData *sd;
> >>> +    int ret;
> >>> +
> >>> +    if (ccf->passthrough == 1 || ccf->cc_detected == 0)
> >>> +        return 0;
> >>> +
> >>> +    sd = av_frame_new_side_data(frame, AV_FRAME_DATA_A53_CC,
> >>> +                                ff_ccfifo_getoutputsize(ccf));
> >>> +    if (sd) {
> >>> +        ret = ff_ccfifo_injectbytes(ccf, sd->data, sd->size);
> >>> +        if (ret < 0) {
> >>> +            av_frame_remove_side_data(frame, AV_FRAME_DATA_A53_CC);
> >>> +            return ret;
> >>> +        }
> >>> +    }
> >>> +
> >>> +    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->passthrough == 1) {
> >>> +        av_log_once(ccf->log_ctx, AV_LOG_WARNING, AV_LOG_DEBUG,
> >>> &ccf->passthrough_warning,
> >>> +                    "cc_fifo cannot transcode captions fps=%d/%d\n",
> >>> +                    ccf->framerate.num, ccf->framerate.den);
> >>> +        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], 1);
> >>> +        } else if (cc_valid && (cc_type == 0x02 || cc_type == 0x03)) {
> >>> +            av_fifo_write(ccf->cc_708_fifo,
> >>> &cc_bytes[CC_BYTES_PER_ENTRY*i], 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... */
> >>> +int ff_ccfifo_extract(AVCCFifo *ccf, AVFrame *frame)
> >>> +{
> >>> +    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 */
> >>> +        if (!ccf->passthrough)
> >>> +            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..44c9245
> >>> --- /dev/null
> >>> +++ b/libavfilter/ccfifo.h
> >>> @@ -0,0 +1,110 @@
> >>> +/*
> >>> + * 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);
> >>> +
> >>> +/**
> >>> + * Provide the size in bytes of an output buffer to allocate
> >>> + *
> >>> + * Ask for how many bytes the output will contain, so the caller can
> >>> allocate
> >>> + * an appropriately sized buffer and pass it to
> ff_ccfifo_injectbytes()
> >>> + *
> >>> + */
> >>> +int ff_ccfifo_getoutputsize(AVCCFifo *ccf);
> >>> +
> >>> +/**
> >>> + * 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 to insert the
> >>> CC data
> >>> + * int rather than an AVFrame
> >>> + */
> >>> +int ff_ccfifo_injectbytes(AVCCFifo *ccf, uint8_t *data, size_t len);
> >>> +
> >>> +/**
> >>> + * Returns 1 if captions have been found as a prior call
> >>> + * to ff_ccfifo_extract() or ff_ccfifo_extractbytes()
> >>> + */
> >>> +int ff_ccfifo_ccdetected(AVCCFifo *ccf);
> >>> +
> >>> +#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".
> >>>
> >>
> _______________________________________________
> 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".