[FFmpeg-devel,06/11] decklink: Add support for using libklvanc from within capture module

Make use of libklvanc from within the DeckLink capture module,
initially for EIA-708 and AFD.  Support for other VANC types will
come in subsequent patches.

Incorporates feedback from Derek Buitenhuis <derek.buitenhuis@gmail.com>,
James Almer <jamrial@gmail.com>, and Aaron Levinson <alevinsn_dev@levland.net>

Signed-off-by: Devin Heitmueller <dheitmueller@ltnglobal.com>
---
 libavdevice/decklink_dec.cpp | 136 +++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 136 insertions(+)

On Mon, 8 Jan 2018, Devin Heitmueller wrote:

> Make use of libklvanc from within the DeckLink capture module,
> initially for EIA-708 and AFD.  Support for other VANC types will
> come in subsequent patches.
>
> Incorporates feedback from Derek Buitenhuis <derek.buitenhuis@gmail.com>,
> James Almer <jamrial@gmail.com>, and Aaron Levinson <alevinsn_dev@levland.net>
>
> Signed-off-by: Devin Heitmueller <dheitmueller@ltnglobal.com>
> ---
> libavdevice/decklink_dec.cpp | 136 +++++++++++++++++++++++++++++++++++++++++++
> 1 file changed, 136 insertions(+)
>
> diff --git a/libavdevice/decklink_dec.cpp b/libavdevice/decklink_dec.cpp
> index 6c1ff82..bab3588 100644
> --- a/libavdevice/decklink_dec.cpp
> +++ b/libavdevice/decklink_dec.cpp
> @@ -3,6 +3,7 @@
>  * Copyright (c) 2013-2014 Luca Barbato, Deti Fliegl
>  * Copyright (c) 2014 Rafaël Carré
>  * Copyright (c) 2017 Akamai Technologies, Inc.
> + * Copyright (c) 2017 LTN Global Communications, Inc.
>  *
>  * This file is part of FFmpeg.
>  *
> @@ -673,10 +674,123 @@ error:
>     return ret;
> }
> 
> +#if CONFIG_LIBKLVANC
> +/* VANC Callbacks */
> +struct vanc_cb_ctx {
> +    AVFormatContext *avctx;
> +    AVPacket *pkt;
> +};
> +static int cb_AFD(void *callback_context, struct klvanc_context_s *ctx,
> +                  struct klvanc_packet_afd_s *pkt)
> +{
> +    struct vanc_cb_ctx *cb_ctx = (struct vanc_cb_ctx *)callback_context;
> +    uint8_t *afd;
> +
> +    afd = av_packet_new_side_data(cb_ctx->pkt, AV_PKT_DATA_AFD, 1);
> +    if (afd == NULL) {

if !afd
     return...

> +        return AVERROR(ENOMEM);
> +    }
> +    afd[0] = pkt->hdr.payload[0] >> 3;
> +
> +    return 0;
> +}
> +
> +static int cb_EIA_708B(void *callback_context, struct klvanc_context_s *ctx,
> +                       struct klvanc_packet_eia_708b_s *pkt)
> +{
> +    struct vanc_cb_ctx *cb_ctx = (struct vanc_cb_ctx *)callback_context;
> +    decklink_cctx *cctx = (struct decklink_cctx *)cb_ctx->avctx->priv_data;
> +    struct decklink_ctx *decklink_ctx = (struct decklink_ctx *)cctx->ctx;
> +    uint16_t expected_cdp;
> +    uint8_t *cc;
> +
> +    if (!pkt->checksum_valid)
> +        return 0;
> +
> +    if (!pkt->header.ccdata_present)
> +        return 0;
> +
> +    expected_cdp = decklink_ctx->cdp_sequence_num + 1;
> +    decklink_ctx->cdp_sequence_num = pkt->header.cdp_hdr_sequence_cntr;
> +    if (pkt->header.cdp_hdr_sequence_cntr != expected_cdp) {
> +        av_log(cb_ctx->avctx, AV_LOG_DEBUG,
> +               "CDP counter inconsistent.  Received=0x%04x Expected=%04x\n",
> +               pkt->header.cdp_hdr_sequence_cntr, expected_cdp);
> +        return 0;
> +    }
> +
> +    cc = av_packet_new_side_data(cb_ctx->pkt, AV_PKT_DATA_A53_CC, pkt->ccdata.cc_count * 3);
> +    if (cc == NULL)
> +        return AVERROR(ENOMEM);
> +
> +    for (int i = 0; i < pkt->ccdata.cc_count; i++) {
> +        cc[3*i] = 0xf8 | (pkt->ccdata.cc[i].cc_valid ? 0x04 : 0x00) |
> +                  (pkt->ccdata.cc[i].cc_type & 0x03);
> +        cc[3*i+1] = pkt->ccdata.cc[i].cc_data[0];
> +        cc[3*i+2] = pkt->ccdata.cc[i].cc_data[1];
> +    }
> +
> +    return 0;
> +}
> +
> +static struct klvanc_callbacks_s callbacks =
> +{
> +    cb_AFD,
> +    cb_EIA_708B,
> +    NULL,
> +    NULL,
> +    NULL,
> +    NULL,
> +};
> +/* End: VANC Callbacks */
> +
> +/* Take one line of V210 from VANC, colorspace convert and feed it to the
> + * VANC parser. We'll expect our VANC message callbacks to happen on this
> + * same calling thread.
> + */
> +static int klvanc_handle_line(AVFormatContext *avctx, struct klvanc_context_s *vanc_ctx,
> +                              unsigned char *buf, unsigned int uiWidth, unsigned int lineNr,
> +                              AVPacket *pkt)
> +{
> +    /* Convert the vanc line from V210 to CrCB422, then vanc parse it */
> +
> +    /* We need two kinds of type pointers into the source vbi buffer */
> +    /* TODO: What the hell is this, two ptrs? */

Hmm, what?

> +    const uint32_t *src = (const uint32_t *)buf;
> +
> +    /* Convert Blackmagic pixel format to nv20.
> +     * src pointer gets mangled during conversion, hence we need its own
> +     * ptr instead of passing vbiBufferPtr.
> +     * decoded_words should be atleast 2 * uiWidth.
> +     */
> +    uint16_t decoded_words[16384];
> +
> +    /* On output each pixel will be decomposed into three 16-bit words (one for Y, U, V) */
> +    memset(&decoded_words[0], 0, sizeof(decoded_words));
> +    uint16_t *p_anc = decoded_words;
> +    if (klvanc_v210_line_to_nv20_c(src, p_anc, sizeof(decoded_words), (uiWidth / 6) * 6) < 0)
> +        return AVERROR(EINVAL);
> +
> +    if (vanc_ctx) {
> +        struct vanc_cb_ctx cb_ctx = {
> +            .avctx = avctx,
> +            .pkt = pkt
> +        };
> +        vanc_ctx->callback_context = &cb_ctx;
> +        int ret = klvanc_packet_parse(vanc_ctx, lineNr, decoded_words, sizeof(decoded_words) / (sizeof(uint16_t)));

A parity error also causes a negative return value? Or parity errors only 
makes the library ignore the packet, and error values are meaning ENOMEM?

What happens if multiple packets are in a single line and one packet has 
parity errors, but the other does not. We should be able to use the 
result from the packet without errors. So does this function returns 
success if any of the callbacks succeeded?

> +        if (ret < 0) {
> +            return AVERROR(EINVAL);
> +        }
> +    }
> +    return 0;
> +}
> +#endif
> +
> HRESULT decklink_input_callback::VideoInputFrameArrived(
>     IDeckLinkVideoInputFrame *videoFrame, IDeckLinkAudioInputPacket *audioFrame)
> {
>     decklink_cctx *cctx = (struct decklink_cctx *)avctx->priv_data;
> +    struct decklink_ctx *ctx = (struct decklink_ctx *)cctx->ctx;
>     void *frameBytes;
>     void *audioFrameBytes;
>     BMDTimeValue frameTime;
> @@ -787,10 +901,17 @@ HRESULT decklink_input_callback::VideoInputFrameArrived(
>                     for (i = vanc_line_numbers[idx].vanc_start; i <= vanc_line_numbers[idx].vanc_end; i++) {
>                         uint8_t *buf;
>                         if (vanc->GetBufferForVerticalBlankingLine(i, (void**)&buf) == S_OK) {
> +#if CONFIG_LIBKLVANC
> +                            int ret = klvanc_handle_line(avctx, ctx->vanc_ctx,
> +                                                         buf, videoFrame->GetWidth(), i, &pkt);
> +                            if (ret != 0)
> +                                av_log(avctx, AV_LOG_ERROR, "Error parsing VANC for line %d\n", i);
> +#else

For now, you should allow both klvanc and ffmpeg parsing of the VANC, so 
the #else does not seem right.


>                             uint16_t luma_vanc[MAX_WIDTH_VANC];
>                             extract_luma_from_v210(luma_vanc, buf, videoFrame->GetWidth());
>                             txt_buf = get_metadata(avctx, luma_vanc, videoFrame->GetWidth(),
>                                                    txt_buf, sizeof(txt_buf0) - (txt_buf - txt_buf0), &pkt);
> +#endif
>                         }
>                         if (i == vanc_line_numbers[idx].field0_vanc_end)
>                             i = vanc_line_numbers[idx].field1_vanc_start - 1;
> @@ -952,6 +1073,9 @@ av_cold int ff_decklink_read_close(AVFormatContext *avctx)
>
>     ff_decklink_cleanup(avctx);
>     avpacket_queue_end(&ctx->queue);
> +#if CONFIG_LIBKLVANC
> +    klvanc_context_destroy(ctx->vanc_ctx);
> +#endif
>
>     av_freep(&cctx->ctx);
> 
> @@ -1195,6 +1319,18 @@ av_cold int ff_decklink_read_header(AVFormatContext *avctx)
>
>     avpacket_queue_init (avctx, &ctx->queue);
> 
> +#if CONFIG_LIBKLVANC
> +    if (klvanc_context_create(&ctx->vanc_ctx) < 0) {
> +        av_log(avctx, AV_LOG_ERROR, "Cannot create VANC library context\n");
> +        ret = AVERROR(ENOMEM);
> +        goto error;
> +    } else {
> +        ctx->vanc_ctx->verbose = 0;
> +        ctx->vanc_ctx->callbacks = &callbacks;
> +        ctx->vanc_ctx->log_cb = NULL;
> +    }
> +#endif
> +
>     if (ctx->dli->StartStreams() != S_OK) {
>         av_log(avctx, AV_LOG_ERROR, "Cannot start input stream\n");
>         ret = AVERROR(EIO);

Regards,
Marton

Hello Marton,

Thank you for the feedback.  Comments inline.

>> +        vanc_ctx->callback_context = &cb_ctx;
>> +        int ret = klvanc_packet_parse(vanc_ctx, lineNr, decoded_words, sizeof(decoded_words) / (sizeof(uint16_t)));
> 
> A parity error also causes a negative return value? Or parity errors only makes the library ignore the packet, and error values are meaning ENOMEM?

A parity error will cause the CRC to fail on the given packet.  Packets that fail the CRC check won’t invoke the callback (there is a special variable to override this, as well as a counter that can be inspected for the number of checksum errors encountered).  We don’t make any effort to distinguish between parity errors and checksum failures, since checksum failure already detects parity errors.

> 
> What happens if multiple packets are in a single line and one packet has parity errors, but the other does not. We should be able to use the result from the packet without errors. So does this function returns success if any of the callbacks succeeded?

The klvanc_packet_parse() can invoke the callback multiple times in a single call, to handle cases that multiple VANC packets are found on the same line.  The return value is the number of packets found, regardless of whether they failed the checksum validation or have a callback registered for their VANC type.

>> 
>> +#if CONFIG_LIBKLVANC
>> +                            int ret = klvanc_handle_line(avctx, ctx->vanc_ctx,
>> +                                                         buf, videoFrame->GetWidth(), i, &pkt);
>> +                            if (ret != 0)
>> +                                av_log(avctx, AV_LOG_ERROR, "Error parsing VANC for line %d\n", i);
>> +#else
> 
> For now, you should allow both klvanc and ffmpeg parsing of the VANC, so the #else does not seem right.

I’m not against this, in particular given libklvanc doesn’t currently support OP47 (it’s on my todo list).  However, how do you propose we handle cases where functionality overlaps between the two?  In particular, we don’t want both klvanc and the internal routine creating EIA-708 side_data.

I’m about to leave the country for a week.  Please don’t interpret my failure to respond to email(s) as disinterest.  I still very much want to get this functionality merged (and I’ve got about a dozen patches queued up behind these for various features/functionality).

Devin