[FFmpeg-devel] [PATCH] avdevice/decklink_dec: extracting and outputing klv from vanc

Marton Balint cus at passwd.hu
Sun Jun 14 17:27:19 EEST 2020


On Tue, 9 Jun 2020, Zivkovic, Milos wrote:

> Hello,
>
> I've attached a patch that adds support for extraction of KLV data from SDI
> VANC.
> Code is enabled with a special *output_klv* option and is otherwise unused.
>
> This was tested with an off the shelf UDP -> SDI decoder that preserves KLV
> and inserts them in VANC, following the guidelines in MISB ST 0605.
>
> I'm working on another patch that will enable *avdevice/decklink_enc* to do
> that as well, and it will also require *output_klv* option to be used.

> From 7371747b86dd3de4889c28d8b6536e8aff08be2b Mon Sep 17 00:00:00 2001
> From: Milos Zivkovic <zivkovic at teralogics.com>
> Date: Mon, 8 Jun 2020 11:56:37 +0200
> Subject: [PATCH] avdevice/decklink_dec: extracting and outputing klv from vanc
> 
> Signed-off-by: Milos Zivkovic <zivkovic at teralogics.com>
> ---
>  libavdevice/decklink_common.h   |   2 +
>  libavdevice/decklink_common_c.h |   1 +
>  libavdevice/decklink_dec.cpp    | 160 +++++++++++++++++++++++++++++++++++++++-
>  libavdevice/decklink_dec_c.c    |   1 +
>  4 files changed, 163 insertions(+), 1 deletion(-)

Missing docs for the new option.

> 
> diff --git a/libavdevice/decklink_common.h b/libavdevice/decklink_common.h
> index 27ce6a8..2d57b85 100644
> --- a/libavdevice/decklink_common.h
> +++ b/libavdevice/decklink_common.h
> @@ -120,12 +120,14 @@ struct decklink_ctx {
>      unsigned int dropped;
>      AVStream *audio_st;
>      AVStream *video_st;
> +    AVStream *klv_st;
>      AVStream *teletext_st;
>      uint16_t cdp_sequence_num;
>
>      /* Options */
>      int list_devices;
>      int list_formats;
> +    int output_klv;
>      int64_t teletext_lines;
>      double preroll;
>      int duplex_mode;
> diff --git a/libavdevice/decklink_common_c.h b/libavdevice/decklink_common_c.h
> index 88b1eae..953aebe 100644
> --- a/libavdevice/decklink_common_c.h
> +++ b/libavdevice/decklink_common_c.h
> @@ -40,6 +40,7 @@ struct decklink_cctx {
>      /* Options */
>      int list_devices;
>      int list_formats;
> +    int output_klv;
>      int64_t teletext_lines;
>      double preroll;
>      int audio_channels;
> diff --git a/libavdevice/decklink_dec.cpp b/libavdevice/decklink_dec.cpp
> index 82106aa..7b39dd5 100644
> --- a/libavdevice/decklink_dec.cpp
> +++ b/libavdevice/decklink_dec.cpp
> @@ -22,6 +22,8 @@
>   */
>
>  #include <atomic>
> +#include <vector>
> +#include <algorithm>

Avoid these if possible. Yes, this is C++ code but only because the decklink
API requires it. Also it gives me a compile error, and I am not familiar enough
with C++ to figure out what is going on:

cc1plus: warning: command line option ‘-Wdeclaration-after-statement’ is valid for C/ObjC but not for C++
cc1plus: warning: command line option ‘-Wmissing-prototypes’ is valid for C/ObjC but not for C++
cc1plus: warning: command line option ‘-Wno-pointer-to-int-cast’ is valid for C/ObjC but not for C++
cc1plus: warning: command line option ‘-Wstrict-prototypes’ is valid for C/ObjC but not for C++
cc1plus: warning: command line option ‘-Wno-pointer-sign’ is valid for C/ObjC but not for C++
cc1plus: warning: ‘-Werror=’ argument ‘-Werror=implicit-function-declaration’ is not valid for C++
cc1plus: warning: ‘-Werror=’ argument ‘-Werror=missing-prototypes’ is not valid for C++
cc1plus: warning: command line option ‘-std=c11’ is valid for C/ObjC but not for C++
In file included from /usr/include/c++/7/ext/new_allocator.h:33:0,
                  from /usr/include/c++/7/x86_64-suse-linux/bits/c++allocator.h:33,
                  from /usr/include/c++/7/bits/allocator.h:46,
                  from /usr/include/c++/7/vector:61,
                  from libavdevice/decklink_dec.cpp:25:
./new:1:1: error: expected unqualified-id before ‘[’ token
  [PACKET]
  ^
In file included from /usr/include/c++/7/bits/allocator.h:46:0,
                  from /usr/include/c++/7/vector:61,
                  from libavdevice/decklink_dec.cpp:25:
/usr/include/c++/7/x86_64-suse-linux/bits/c++allocator.h:48:41: error: ‘new_allocator’ in namespace ‘__gnu_cxx’ does not name a template type
      using __allocator_base = __gnu_cxx::new_allocator<_Tp>;
                                          ^~~~~~~~~~~~~
In file included from /usr/include/c++/7/vector:61:0,
                  from libavdevice/decklink_dec.cpp:25:
/usr/include/c++/7/bits/allocator.h:108:45: error: expected template-name before ‘<’ token
      class allocator: public __allocator_base<_Tp>

[...]

>  using std::atomic;
>
>  /* Include internal.h first to avoid conflict between winsock.h (used by
> @@ -583,6 +585,143 @@ static int avpacket_queue_get(AVPacketQueue *q, AVPacket *pkt, int block)
>      return ret;
>  }
> 
> +namespace
> +{
> +    template<typename T>
> +    struct com_ptr
> +    {
> +        T* self;
> +
> +        com_ptr()
> +            : self(nullptr)
> +        {
> +        }
> +
> +        ~com_ptr()
> +        {
> +            destroy();
> +        }
> +
> +        void destroy()
> +        {
> +            if (self)
> +                self->Release();
> +
> +            self = nullptr;
> +        }
> +
> +        T* get()
> +        {
> +            return self;
> +        }
> +
> +        const T* get() const
> +        {
> +            return self;
> +        }
> +
> +        T* operator->()
> +        {
> +            return get();
> +        }
> +
> +        const T* operator->() const
> +        {
> +            return get();
> +        }
> +    };
> +}

You only use this to avoid calling ->Release() right? If that is the case, then
I'd avoid this as well, I am not sure about the compiler support of such
templating.

> +
> +static void handle_klv(AVFormatContext *avctx, decklink_ctx *ctx, IDeckLinkVideoInputFrame *videoFrame, int64_t pts)
> +{
> +    // defined by MISB ST 0605
> +    const uint8_t KLV_DID = 0x44;
> +    const uint8_t KLV_IN_VANC_SDID = 0x04;
> +    const uint8_t KLV_IN_HANC_SDID = 0x14;
> +
> +    struct KLVPacket
> +    {
> +        uint16_t sequence_counter;
> +        std::vector<uint8_t> data;
> +    };
> +
> +    size_t total_size = 0;
> +    std::vector<std::vector<KLVPacket>> klv_packets(256);
> +
> +    com_ptr<IDeckLinkVideoFrameAncillaryPackets> packets;
> +    if (videoFrame->QueryInterface(IID_IDeckLinkVideoFrameAncillaryPackets, (void**)&packets.self) != S_OK)
> +        return;

This interface is only available from DeckLink SDK 10.10 or so, but you have not
bumped the version requirement in configure.

> +
> +    com_ptr<IDeckLinkAncillaryPacketIterator> it;
> +    if (packets->GetPacketIterator(&it.self) != S_OK)
> +        return;
> +
> +    com_ptr<IDeckLinkAncillaryPacket> packet;
> +    while (it->Next(&packet.self) == S_OK) {
> +        uint8_t *data = nullptr;
> +        uint32_t size = 0;
> +
> +        if (packet->GetDID() == KLV_DID && packet->GetSDID() == KLV_IN_VANC_SDID) {
> +             av_log(avctx, AV_LOG_DEBUG, "Found KLV VANC packet on line: %d\n", packet->GetLineNumber());
> +
> +            if (packet->GetBytes(bmdAncillaryPacketFormatUInt8, (const void**) &data, &size) == S_OK) {
> +                // MID and PSC
> +                if (size > 3) {
> +                    uint8_t mid = data[0];
> +                    uint16_t psc = data[1] << 8 | data[2];
> +
> +                    av_log(avctx, AV_LOG_DEBUG, "KLV with MID: %d and PSC: %d\n", mid, psc);
> +
> +                    uint32_t data_len = size - 3;
> +                    total_size += data_len;
> +
> +                    KLVPacket packet{ psc };
> +                    packet.data.resize(data_len);
> +                    memcpy(packet.data.data(), data + 3, data_len);
> +
> +                    klv_packets[mid].push_back(std::move(packet));
> +                }
> +            }
> +        }
> +
> +        packet.destroy();
> +    }
> +
> +    if (total_size > 0) {
> +        std::vector<uint8_t> klv;
> +        klv.reserve(total_size);
> +
> +        for (size_t i = 0; i < klv_packets.size(); ++i) {
> +            auto& list = klv_packets[i];
> +
> +            if (list.empty())
> +                continue;
> +
> +            av_log(avctx, AV_LOG_DEBUG, "Joining MID: %d\n", i);
> +
> +            std::sort(list.begin(), list.end(), [](const KLVPacket& lhs, const KLVPacket& rhs) {
> +                return lhs.sequence_counter < rhs.sequence_counter;
> +            });

Is the sorting really needed? Aren't they supposed to be sorted in the VANC?

> +
> +            for (auto& packet : list)
> +                klv.insert(klv.end(), packet.data.begin(), packet.data.end());
> +        }
> +
> +        AVPacket klv_packet;
> +        av_init_packet(&klv_packet);
> +        klv_packet.pts = pts;
> +        klv_packet.dts = pts;
> +        klv_packet.flags |= AV_PKT_FLAG_KEY;
> +        klv_packet.stream_index = ctx->klv_st->index;
> +        klv_packet.data = klv.data();
> +        klv_packet.size = klv.size();
> +
> +        if (avpacket_queue_put(&ctx->queue, &klv_packet) < 0) {
> +            av_log(avctx, AV_LOG_INFO, "dropping KLV VANC packet\n");

Maybe avoid the error message and increase ctx->dropped like other similar code
does it.

> +        }
> +    }
> +}
> +
>  class decklink_input_callback : public IDeckLinkInputCallback
>  {
>  public:
> @@ -816,6 +955,10 @@ HRESULT decklink_input_callback::VideoInputFrameArrived(
>          //fprintf(stderr,"Video Frame size %d ts %d\n", pkt.size, pkt.pts);
>
>          if (!no_video) {
> +            if (ctx->output_klv) {
> +                handle_klv(avctx, ctx, videoFrame, pkt.pts);
> +            }
> +

Mixed code and declaration.

>              IDeckLinkVideoFrameAncillary *vanc;
>              AVPacket txt_pkt;
>              uint8_t txt_buf0[3531]; // 35 * 46 bytes decoded teletext lines + 1 byte data_identifier + 1920 bytes OP47 decode buffer
> @@ -973,7 +1116,6 @@ static int decklink_autodetect(struct decklink_cctx *cctx) {
>      } else {
>          return -1;
>      }
> -
>  }
>
>  extern "C" {
> @@ -1012,6 +1154,7 @@ av_cold int ff_decklink_read_header(AVFormatContext *avctx)
>          return AVERROR(ENOMEM);
>      ctx->list_devices = cctx->list_devices;
>      ctx->list_formats = cctx->list_formats;
> +    ctx->output_klv = cctx->output_klv;
>      ctx->teletext_lines = cctx->teletext_lines;
>      ctx->preroll      = cctx->preroll;
>      ctx->duplex_mode  = cctx->duplex_mode;
> @@ -1202,6 +1345,21 @@ av_cold int ff_decklink_read_header(AVFormatContext *avctx)
>
>      ctx->video_st=st;
> 
> +    if (ctx->output_klv) {
> +        st = avformat_new_stream(avctx, NULL);
> +        if (!st) {
> +            av_log(avctx, AV_LOG_ERROR, "Cannot add KLV stream\n");

Lose the error message for ENOMEM.

> +            ret = AVERROR(ENOMEM);
> +            goto error;
> +        }
> +        st->codecpar->codec_type = AVMEDIA_TYPE_DATA;
> +        st->time_base.den        = ctx->bmd_tb_den;
> +        st->time_base.num        = ctx->bmd_tb_num;
> +        st->codecpar->codec_id   = AV_CODEC_ID_SMPTE_KLV;
> +        avpriv_set_pts_info(st, 64, 1, 1000000);  /* 64 bits pts in us */
> +        ctx->klv_st = st;
> +    }
> +
>      if (ctx->teletext_lines) {
>          st = avformat_new_stream(avctx, NULL);
>          if (!st) {
> diff --git a/libavdevice/decklink_dec_c.c b/libavdevice/decklink_dec_c.c
> index b598769..8beb629 100644
> --- a/libavdevice/decklink_dec_c.c
> +++ b/libavdevice/decklink_dec_c.c
> @@ -39,6 +39,7 @@ static const AVOption options[] = {
>      { "argb",          NULL,   0,  AV_OPT_TYPE_CONST, { .i64 = 32                       }, 0, 0, DEC, "raw_format"},
>      { "bgra",          NULL,   0,  AV_OPT_TYPE_CONST, { .i64 = MKBETAG('B','G','R','A') }, 0, 0, DEC, "raw_format"},
>      { "rgb10",         NULL,   0,  AV_OPT_TYPE_CONST, { .i64 = MKBETAG('r','2','1','0') }, 0, 0, DEC, "raw_format"},
> +    { "output_klv",     "output klv if present in vanc", OFFSET(output_klv), AV_OPT_TYPE_INT,  { .i64 = 0}, 0, 1,   DEC },

AV_OPT_TYPE_BOOL

>      { "teletext_lines", "teletext lines bitmask", OFFSET(teletext_lines), AV_OPT_TYPE_INT64, { .i64 = 0   }, 0, 0x7ffffffffLL, DEC, "teletext_lines"},
>      { "standard",     NULL,                                           0,  AV_OPT_TYPE_CONST, { .i64 = 0x7fff9fffeLL}, 0, 0,    DEC, "teletext_lines"},
>      { "all",          NULL,                                           0,  AV_OPT_TYPE_CONST, { .i64 = 0x7ffffffffLL}, 0, 0,    DEC, "teletext_lines"},
> -- 
> 1.8.3.1
>

Thanks,
Marton


More information about the ffmpeg-devel mailing list