[FFmpeg-devel] [PATCH 5/6] Add suppoort for using libklvanc from within decklink capture module

[Derek Buitenhuis]

On 11/16/2017 6:34 PM, 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.
> 
> Signed-off-by: Devin Heitmueller <dheitmueller at ltnglobal.com>
> ---
>  libavdevice/decklink_dec.cpp | 135 +++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 135 insertions(+)
> 
> diff --git a/libavdevice/decklink_dec.cpp b/libavdevice/decklink_dec.cpp
> index 11b7e60..bea9213 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.
>   *
> @@ -669,10 +670,128 @@ error:
>      return ret;
>  }
>  
> +    afd[0] = pkt->hdr.payload[0] >> 3;
> +    if (av_packet_add_side_data(cb_ctx->pkt, AV_PKT_DATA_AFD, afd, 1) < 0)
> +        av_free(afd);

Is there a reason we shouldn't fail hard here?

> +    if (av_packet_add_side_data(cb_ctx->pkt, AV_PKT_DATA_A53_CC, cc, pkt->ccdata.cc_count * 3) < 0)
> +        av_free(cc);

Ditto here.

> +static struct klvanc_callbacks_s callbacks =
> +{
> +    .afd               = cb_AFD,
> +    .eia_708b          = cb_EIA_708B,
> +    .eia_608           = NULL,
> +    .scte_104          = NULL,
> +    .all               = NULL,
> +    .kl_i64le_counter  = NULL,
> +};

I thought C++ didn't have designated initializers? Maybe my C++ is rusty.

Same for other occurrences.

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

Is buf guaranteed to be properly aligned for this, or will cause aliasing problems?

> +        vanc_ctx->callback_context = &cb_ctx;
> +        int ret = klvanc_packet_parse(vanc_ctx, lineNr, decoded_words, sizeof(decoded_words) / (sizeof(unsigned short)));

Nobody should be typing 'short' in any C/C++ code in 2017..

> +        if (ret < 0) {
> +            /* No VANC on this line */
> +        }

Huh?

> +#if CONFIG_LIBKLVANC
> +                            klvanc_handle_line(avctx, ctx->vanc_ctx,
> +                                               buf, videoFrame->GetWidth(), i, &pkt);
> +#else

No error checking possible?

>                  }
> +
>                  vanc->Release();

Stray change.

> +#if CONFIG_LIBKLVANC
> +    if (klvanc_context_create(&ctx->vanc_ctx) < 0) {
> +        av_log(avctx, AV_LOG_ERROR, "Cannot create VANC library context\n");
> +    } else {
> +        ctx->vanc_ctx->verbose = 0;
> +        ctx->vanc_ctx->callbacks = &callbacks;
> +    }
> +#endif

Should fail hard, no?

- Derek