[FFmpeg-devel,6/8] Add suppoort for using libklvanc from within decklink 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>
and James Almer <jamrial@gmail.com>

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

Patch title:  "suppoort" -> "support", also "decklink" -> "DeckLink"

On 12/29/2017 10:12 AM, 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>
> and James Almer <jamrial@gmail.com>
> 
> Signed-off-by: Devin Heitmueller <dheitmueller@ltnglobal.com>
> ---
>   libavdevice/decklink_dec.cpp | 133 +++++++++++++++++++++++++++++++++++++++++++
>   1 file changed, 133 insertions(+)
> 
> diff --git a/libavdevice/decklink_dec.cpp b/libavdevice/decklink_dec.cpp
> index 8d4070eecd..86db6d8fbd 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.
>    *
> @@ -671,10 +672,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) {
> +        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? */
> +    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)));
> +        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;
> @@ -785,10 +899,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
>                               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;
> @@ -950,6 +1071,7 @@ av_cold int ff_decklink_read_close(AVFormatContext *avctx)
>   
>       ff_decklink_cleanup(avctx);
>       avpacket_queue_end(&ctx->queue);
> +    klvanc_context_destroy(ctx->vanc_ctx);

Shouldn't this last line be wrapped in #if CONFIG_LIBKLVANC?  I suggest 
building this on Linux with and without libklvanc, and I also suggest 
building it (and ideally testing it) on Windows without libklvanc as 
well.  DeckLink is also supported on OS/X.

>   
>       av_freep(&cctx->ctx);
>   
> @@ -1193,6 +1315,17 @@ 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);

Perhaps appropriate to use the return value of klvanc_context_create() 
as input to AVERROR().

> +        goto error;
> +    } else {
> +        ctx->vanc_ctx->verbose = 0;
> +        ctx->vanc_ctx->callbacks = &callbacks;
> +    }
> +#endif
> +
>       if (ctx->dli->StartStreams() != S_OK) {
>           av_log(avctx, AV_LOG_ERROR, "Cannot start input stream\n");
>           ret = AVERROR(EIO);
> 

Aaron Levinson

Hi Aaron,

> On Dec 30, 2017, at 2:34 AM, Aaron Levinson <alevinsn_dev@levland.net> wrote:
> 
> Patch title:  "suppoort" -> "support", also "decklink" -> “DeckLink"

Ok.

>>        ff_decklink_cleanup(avctx);
>>      avpacket_queue_end(&ctx->queue);
>> +    klvanc_context_destroy(ctx->vanc_ctx);
> 
> Shouldn't this last line be wrapped in #if CONFIG_LIBKLVANC?  I suggest building this on Linux with and without libklvanc, and I also suggest building it (and ideally testing it) on Windows without libklvanc as well.  DeckLink is also supported on OS/X.

Yup, that was a mistake I made preparing the latest patch.  The #ifdef guard got left out.

I do typically test-compile on both Linux and OS X both with and without the library present, but that process got skipped this time around (which was an error on my part).

I have confirmed though that was the only build error when the library isn’t present.

> 
>>        av_freep(&cctx->ctx);
>>  @@ -1193,6 +1315,17 @@ 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);
> 
> Perhaps appropriate to use the return value of klvanc_context_create() as input to AVERROR().

I think it’s usually bad practice to blindly return what a third party library returns.  Usually the error should be converted into some ffmpeg specific error code which the caller might actually know what to do with.  In this case I just return ENOMEM since that’s the only actual failure case possible in the call.

Devin