[FFmpeg-devel] [PATCH 1/8] libavdevice/decklink: Add support for EIA-708 output over SDI

Aaron Levinson alevinsn_dev at levland.net
Sat Dec 30 08:23:25 EET 2017


On 12/29/2017 10:12 AM, Devin Heitmueller wrote:
> Hook in libklvanc and use it for output of EIA-708 captions over
> SDI.  The bulk of this patch is just general support for ancillary
> data for the Decklink SDI module - the real work for construction
> of the EIA-708 CDP and VANC line construction is done by libklvanc.
> 
> Libklvanc can be found at: https://github.com/stoth68000/libklvanc
> 
> Updated to reflect feedback from Marton Balint <cus at passwd.hu>
> 
> Signed-off-by: Devin Heitmueller <dheitmueller at ltnglobal.com>
> ---
>   configure                       |   4 ++
>   libavcodec/v210enc.c            |  10 +++
>   libavdevice/decklink_common.cpp |  17 +++--
>   libavdevice/decklink_common.h   |  10 +++
>   libavdevice/decklink_enc.cpp    | 154 ++++++++++++++++++++++++++++++++++++++--
>   5 files changed, 185 insertions(+), 10 deletions(-)
> 
> diff --git a/configure b/configure
> index 6748ef8bc9..19fd04bb6f 100755
> --- a/configure
> +++ b/configure
> @@ -238,6 +238,7 @@ External library support:
>     --enable-libiec61883     enable iec61883 via libiec61883 [no]
>     --enable-libilbc         enable iLBC de/encoding via libilbc [no]
>     --enable-libjack         enable JACK audio sound server [no]
> +  --enable-libklvanc       enable Kernel Labs VANC processing [no]
>     --enable-libkvazaar      enable HEVC encoding via libkvazaar [no]
>     --enable-libmodplug      enable ModPlug via libmodplug [no]
>     --enable-libmp3lame      enable MP3 encoding via libmp3lame [no]
> @@ -1602,6 +1603,7 @@ EXTERNAL_LIBRARY_LIST="
>       libiec61883
>       libilbc
>       libjack
> +    libklvanc
>       libkvazaar
>       libmodplug
>       libmp3lame
> @@ -3074,6 +3076,7 @@ decklink_deps_any="libdl LoadLibrary"
>   decklink_indev_deps="decklink threads"
>   decklink_indev_extralibs="-lstdc++"
>   decklink_outdev_deps="decklink threads"
> +decklink_outdev_suggest="libklvanc"
>   decklink_outdev_extralibs="-lstdc++"
>   libndi_newtek_indev_deps="libndi_newtek"
>   libndi_newtek_indev_extralibs="-lndi"
> @@ -5835,6 +5838,7 @@ enabled libgsm            && { for gsm_hdr in "gsm.h" "gsm/gsm.h"; do
>                                      check_lib libgsm "${gsm_hdr}" gsm_create -lgsm && break;
>                                  done || die "ERROR: libgsm not found"; }
>   enabled libilbc           && require libilbc ilbc.h WebRtcIlbcfix_InitDecode -lilbc $pthreads_extralibs
> +enabled libklvanc         && require libklvanc libklvanc/vanc.h klvanc_context_create -lklvanc
>   enabled libkvazaar        && require_pkg_config libkvazaar "kvazaar >= 0.8.1" kvazaar.h kvz_api_get
>   # While it may appear that require is being used as a pkg-config
>   # fallback for libmfx, it is actually being used to detect a different
> diff --git a/libavcodec/v210enc.c b/libavcodec/v210enc.c
> index a6afbbfc41..51804e3bac 100644
> --- a/libavcodec/v210enc.c
> +++ b/libavcodec/v210enc.c
> @@ -123,6 +123,7 @@ static int encode_frame(AVCodecContext *avctx, AVPacket *pkt,
>       int aligned_width = ((avctx->width + 47) / 48) * 48;
>       int stride = aligned_width * 8 / 3;
>       int line_padding = stride - ((avctx->width * 8 + 11) / 12) * 4;
> +    AVFrameSideData *side_data;
>       int h, w, ret;
>       uint8_t *dst;
>   
> @@ -233,6 +234,15 @@ static int encode_frame(AVCodecContext *avctx, AVPacket *pkt,
>           }
>       }
>   
> +    side_data = av_frame_get_side_data(pic, AV_FRAME_DATA_A53_CC);
> +    if (side_data && side_data->size) {
> +        uint8_t *buf = av_packet_new_side_data(pkt, AV_PKT_DATA_A53_CC, side_data->size);
> +        if (buf)
> +            memcpy(buf, side_data->data, side_data->size);
> +        else
> +            return AVERROR(ENOMEM);
> +    }
> +
>       pkt->flags |= AV_PKT_FLAG_KEY;
>       *got_packet = 1;
>       return 0;
> diff --git a/libavdevice/decklink_common.cpp b/libavdevice/decklink_common.cpp
> index 6ef2c529f4..ba091dadea 100644
> --- a/libavdevice/decklink_common.cpp
> +++ b/libavdevice/decklink_common.cpp
> @@ -254,10 +254,19 @@ int ff_decklink_set_format(AVFormatContext *avctx,
>                                              &support, NULL) != S_OK)
>               return -1;
>       } else {
> -        if (ctx->dlo->DoesSupportVideoMode(ctx->bmd_mode, bmdFormat8BitYUV,
> -                                           bmdVideoOutputFlagDefault,
> -                                           &support, NULL) != S_OK)
> -        return -1;
> +        ctx->supports_vanc = 1;
> +        if (ctx->dlo->DoesSupportVideoMode(ctx->bmd_mode, ctx->raw_format,
> +                                           bmdVideoOutputVANC,
> +                                           &support, NULL) != S_OK) {
> +            /* Try again, but without VANC enabled */
> +            if (ctx->dlo->DoesSupportVideoMode(ctx->bmd_mode, ctx->raw_format,
> +                                               bmdVideoOutputFlagDefault,
> +                                               &support, NULL) != S_OK) {
> +                return -1;
> +            }
> +            ctx->supports_vanc = 0;
> +        }
> +
>       }
>       if (support == bmdDisplayModeSupported)
>           return 0;
> diff --git a/libavdevice/decklink_common.h b/libavdevice/decklink_common.h
> index 57ee7d1d68..143bbb951a 100644
> --- a/libavdevice/decklink_common.h
> +++ b/libavdevice/decklink_common.h
> @@ -27,6 +27,9 @@
>   
>   #include "libavutil/thread.h"
>   #include "decklink_common_c.h"
> +#if CONFIG_LIBKLVANC
> +#include "libklvanc/vanc.h"
> +#endif
>   
>   #ifdef _WIN32
>   #define DECKLINK_BOOL BOOL
> @@ -67,6 +70,7 @@ struct decklink_ctx {
>       int bmd_width;
>       int bmd_height;
>       int bmd_field_dominance;
> +    int supports_vanc;
>   
>       /* Capture buffer queue */
>       AVPacketQueue queue;
> @@ -84,6 +88,7 @@ struct decklink_ctx {
>       AVStream *audio_st;
>       AVStream *video_st;
>       AVStream *teletext_st;
> +    uint16_t cdp_sequence_num;
>   
>       /* Options */
>       int list_devices;
> @@ -94,6 +99,7 @@ struct decklink_ctx {
>       DecklinkPtsSource audio_pts_source;
>       DecklinkPtsSource video_pts_source;
>       int draw_bars;
> +    BMDPixelFormat raw_format;
>   
>       int frames_preroll;
>       int frames_buffer;
> @@ -103,6 +109,10 @@ struct decklink_ctx {
>       int frames_buffer_available_spots;
>       int autodetect;
>   
> +#if CONFIG_LIBKLVANC
> +    struct klvanc_context_s *vanc_ctx;
> +#endif
> +
>       int channels;
>       int audio_depth;
>   };
> diff --git a/libavdevice/decklink_enc.cpp b/libavdevice/decklink_enc.cpp
> index c06ca4668f..ded47108d6 100644
> --- a/libavdevice/decklink_enc.cpp
> +++ b/libavdevice/decklink_enc.cpp
> @@ -38,16 +38,20 @@ extern "C" {
>   
>   #include "decklink_common.h"
>   #include "decklink_enc.h"
> -
> +#if CONFIG_LIBKLVANC
> +#include "libklvanc/vanc.h"
> +#include "libklvanc/vanc-lines.h"
> +#include "libklvanc/pixels.h"
> +#endif
>   
>   /* DeckLink callback class declaration */
>   class decklink_frame : public IDeckLinkVideoFrame
>   {
>   public:
>       decklink_frame(struct decklink_ctx *ctx, AVFrame *avframe, AVCodecID codec_id, int height, int width) :
> -        _ctx(ctx), _avframe(avframe), _avpacket(NULL), _codec_id(codec_id), _height(height), _width(width),  _refs(1) { }
> +        _ctx(ctx), _avframe(avframe), _avpacket(NULL), _codec_id(codec_id), _ancillary(NULL), _height(height), _width(width),  _refs(1) { }
>       decklink_frame(struct decklink_ctx *ctx, AVPacket *avpacket, AVCodecID codec_id, int height, int width) :
> -        _ctx(ctx), _avframe(NULL), _avpacket(avpacket), _codec_id(codec_id), _height(height), _width(width), _refs(1) { }
> +        _ctx(ctx), _avframe(NULL), _avpacket(avpacket), _codec_id(codec_id), _ancillary(NULL), _height(height), _width(width), _refs(1) { }
>   
>       virtual long           STDMETHODCALLTYPE GetWidth      (void)          { return _width; }
>       virtual long           STDMETHODCALLTYPE GetHeight     (void)          { return _height; }
> @@ -87,8 +91,13 @@ public:
>       }
>   
>       virtual HRESULT STDMETHODCALLTYPE GetTimecode     (BMDTimecodeFormat format, IDeckLinkTimecode **timecode) { return S_FALSE; }
> -    virtual HRESULT STDMETHODCALLTYPE GetAncillaryData(IDeckLinkVideoFrameAncillary **ancillary)               { return S_FALSE; }
> -
> +    virtual HRESULT STDMETHODCALLTYPE GetAncillaryData(IDeckLinkVideoFrameAncillary **ancillary)
> +    {
> +        *ancillary = _ancillary;
> +        return _ancillary ? S_OK : S_FALSE;
> +    }

As a general rule, when returning interface pointers in a COM method, 
AddRef() should be called on them before returning.  This is alluded to 
in the SDK documentation for the GetAncillaryData() method:  "Pointer to 
a new IDeckLinkVideoFrameAncillary object. This object must be released 
by the caller when no longer required."  In addition, in the 
implementation of SetAncillaryData(), it is also appropriate to call 
AddRef() on ancillary, because you don't want the underlying object to 
self-destruct out from under you.  Further, now that the decklink_frame 
class has an actual COM object as a member variable, you'll want to add 
a destructor to the class to call Release() on _ancillary if it isn't 
null.  Alternatively, you could modify the implementation of Release() 
in the case that the reference count becomes 0, although that isn't the 
"C++ way".

> +    virtual HRESULT STDMETHODCALLTYPE SetAncillaryData(IDeckLinkVideoFrameAncillary
> +                                                       *ancillary) { _ancillary = ancillary; return S_OK; }
>       virtual HRESULT STDMETHODCALLTYPE QueryInterface(REFIID iid, LPVOID *ppv) { return E_NOINTERFACE; }
>       virtual ULONG   STDMETHODCALLTYPE AddRef(void)                            { return ++_refs; }
>       virtual ULONG   STDMETHODCALLTYPE Release(void)
> @@ -106,6 +115,7 @@ public:
>       AVFrame *_avframe;
>       AVPacket *_avpacket;
>       AVCodecID _codec_id;
> +    IDeckLinkVideoFrameAncillary *_ancillary;
>       int _height;
>       int _width;
>   
> @@ -156,10 +166,13 @@ static int decklink_setup_video(AVFormatContext *avctx, AVStream *st)
>                      " Only AV_PIX_FMT_UYVY422 is supported.\n");
>               return -1;
>           }
> +        ctx->raw_format = bmdFormat8BitYUV;
>       } else if (c->codec_id != AV_CODEC_ID_V210) {
>           av_log(avctx, AV_LOG_ERROR, "Unsupported codec type!"
>                  " Only V210 and wrapped frame with AV_PIX_FMT_UYVY422 are supported.\n");
>           return -1;
> +    } else {
> +        ctx->raw_format = bmdFormat10BitYUV;
>       }
>   
>       if (ff_decklink_set_configs(avctx, DIRECTION_OUT) < 0) {
> @@ -173,7 +186,7 @@ static int decklink_setup_video(AVFormatContext *avctx, AVStream *st)
>           return -1;
>       }
>       if (ctx->dlo->EnableVideoOutput(ctx->bmd_mode,
> -                                    bmdVideoOutputFlagDefault) != S_OK) {
> +                                    ctx->supports_vanc ? bmdVideoOutputVANC : bmdVideoOutputFlagDefault) != S_OK) {

Most of the changes in this set of patches appear to only be relevant in 
the case that libklvanc is enabled.  Nevertheless, a number of the 
changes are not wrapped in #if CONFIG_LIBKLVANC, which means that they 
will be exercised even if libklvanc is not available.  Most of these 
changes are benign, but the change to use bmdVideoOutputVANC by default 
may not be benign.  I'm unclear on any side effects of specifying 
bmdVideoOutputVANC for the output flags instead of 
bmdVideoOutputFlagDefault.  If this change, for example, exposes a bug 
in Blackmagic on Windows, where libklvanc is almost certainly not 
currently available, then this code, which has previously been working 
on Windows, will suddenly stop working, and for no good reason other 
than the code is a bit cleaner without #if CONFIG_LIBKLVANC.  I'm not 
saying that such a Blackmagic bug exists (although it wouldn't surprise 
me if such a bug does exist), but if the user doesn't have libklvanc 
available when building, as will be the case for almost all users out 
there, it seems preferable not to expose them to potential issues.  I 
think this is also a good argument for having the libklvanc 
functionality in FFmpeg proper.

I'd also argue that, even if CONFIG_LIBKLVANC is available, it should 
only specify the VANC output flag in the case that VANC will actually be 
used.  Just because VANC is supported doesn't mean it should be used.

>           av_log(avctx, AV_LOG_ERROR, "Could not enable video output!\n");
>           return -1;
>       }
> @@ -269,6 +282,122 @@ av_cold int ff_decklink_write_trailer(AVFormatContext *avctx)
>       return 0;
>   }
>   
> +#if CONFIG_LIBKLVANC
> +static int decklink_construct_vanc(AVFormatContext *avctx, struct decklink_ctx *ctx,
> +                                   AVPacket *pkt, decklink_frame *frame)
> +{
> +    struct klvanc_line_set_s vanc_lines = { 0 };
> +    int ret, size;
> +
> +    if (ctx->supports_vanc == 0)
> +        return 0;
> +
> +    const uint8_t *data = av_packet_get_side_data(pkt, AV_PKT_DATA_A53_CC, &size);
> +    if (data) {
> +        struct klvanc_packet_eia_708b_s *pkt;
> +        uint16_t *cdp;
> +        uint16_t len;
> +        uint8_t cc_count = size / 3;
> +
> +        ret = klvanc_create_eia708_cdp(&pkt);
> +        if (ret != 0)
> +            return AVERROR(ENOMEM);
> +
> +        ret = klvanc_set_framerate_EIA_708B(pkt, ctx->bmd_tb_num, ctx->bmd_tb_den);
> +        if (ret != 0) {
> +            av_log(avctx, AV_LOG_ERROR, "Invalid framerate specified: %lld/%lld\n",
> +                   ctx->bmd_tb_num, ctx->bmd_tb_den);
> +            klvanc_destroy_eia708_cdp(pkt);
> +            return AVERROR(EINVAL);
> +        }
> +
> +        if (cc_count > KLVANC_MAX_CC_COUNT) {
> +            av_log(avctx, AV_LOG_ERROR, "Illegal cc_count received: %d\n", cc_count);
> +            cc_count = KLVANC_MAX_CC_COUNT;
> +        }
> +
> +        /* CC data */
> +        pkt->header.ccdata_present = 1;
> +        pkt->ccdata.cc_count = cc_count;
> +        for (size_t i = 0; i < cc_count; i++) {
> +            if (data [3*i] & 0x40)
> +                pkt->ccdata.cc[i].cc_valid = 1;
> +            pkt->ccdata.cc[i].cc_type = data[3*i] & 0x03;
> +            pkt->ccdata.cc[i].cc_data[0] = data[3*i+1];
> +            pkt->ccdata.cc[i].cc_data[1] = data[3*i+2];
> +        }
> +
> +        klvanc_finalize_EIA_708B(pkt, ctx->cdp_sequence_num++);
> +        ret = klvanc_convert_EIA_708B_to_words(pkt, &cdp, &len);
> +        if (ret != 0) {
> +            av_log(avctx, AV_LOG_ERROR, "Failed converting 708 packet to words\n");
> +            return AVERROR(ENOMEM);
> +        }
> +        klvanc_destroy_eia708_cdp(pkt);
> +
> +        ret = klvanc_line_insert(ctx->vanc_ctx, &vanc_lines, cdp, len, 11, 0);
> +        if (ret != 0) {
> +            av_log(avctx, AV_LOG_ERROR, "VANC line insertion failed\n");
> +            return AVERROR(ENOMEM);
> +        }
> +    }
> +
> +    IDeckLinkVideoFrameAncillary *vanc;
> +    int result = ctx->dlo->CreateAncillaryData(bmdFormat10BitYUV, &vanc);

Since the reference count changes are missing in the code above, I think 
it would be useful to describe what is happening here.  If 
CreateAncillaryData() returns successfully, the vanc object will have a 
reference count of 1.  This code "owns" this particular reference to the 
object.

> +    if (result != S_OK) {
> +        av_log(avctx, AV_LOG_ERROR, "Failed to create vanc\n");
> +        return -1;
> +    }
> +
> +    /* Now that we've got all the VANC lines in a nice orderly manner, generate the
> +       final VANC sections for the Decklink output */
> +    for (int i = 0; i < vanc_lines.num_lines; i++) {
> +        struct klvanc_line_s *line = vanc_lines.lines[i];
> +        uint16_t *out_line;
> +        int real_line;
> +        int out_len;
> +        void *buf;
> +
> +        if (line == NULL)
> +            break;
> +
> +        real_line = line->line_number;
> +#if 0
> +        /* FIXME: include hack for certain Decklink cards which mis-represent
> +           line numbers for pSF frames */
> +        if (decklink_sys->b_psf_interlaced)
> +            real_line = Calculate1080psfVancLine(line->line_number);
> +#endif
> +        result = vanc->GetBufferForVerticalBlankingLine(real_line, &buf);
> +        if (result != S_OK) {
> +            av_log(avctx, AV_LOG_ERROR, "Failed to get VANC line %d: %d", real_line, result);
> +            klvanc_line_free(line);
> +            continue;
> +        }
> +
> +        /* Generate the full line taking into account all VANC packets on that line */
> +        result = klvanc_generate_vanc_line(ctx->vanc_ctx, line, &out_line, &out_len, ctx->bmd_width);
> +        if (result != 0) {
> +            av_log(avctx, AV_LOG_ERROR, "Failed to generate VANC line\n");
> +            klvanc_line_free(line);
> +            continue;
> +        }
> +
> +        /* Repack the 16-bit ints into 10-bit, and push into final buffer */
> +        klvanc_y10_to_v210(out_line, (uint8_t *) buf, out_len);
> +        free(out_line);
> +        klvanc_line_free(line);
> +    }
> +
> +    result = frame->SetAncillaryData(vanc);

After this point, assuming that SetAncillaryData() is implemented 
properly, the reference count for vanc should be 2.  This code owns the 
"first" reference, and the frame object owns the "second" reference. 
This code should call Release() on vanc after this point, regardless of 
the return value of SetAncillaryData().

> +    if (result != S_OK) {
> +        av_log(avctx, AV_LOG_ERROR, "Failed to set vanc: %d", result);
> +        return AVERROR(EIO);
> +    }
> +    return 0;
> +}
> +#endif
> +
>   static int decklink_write_video_packet(AVFormatContext *avctx, AVPacket *pkt)
>   {
>       struct decklink_cctx *cctx = (struct decklink_cctx *)avctx->priv_data;
> @@ -279,6 +408,9 @@ static int decklink_write_video_packet(AVFormatContext *avctx, AVPacket *pkt)
>       decklink_frame *frame;
>       buffercount_type buffered;
>       HRESULT hr;
> +#if CONFIG_LIBKLVANC
> +    int ret;
> +#endif
>   
>       if (st->codecpar->codec_id == AV_CODEC_ID_WRAPPED_AVFRAME) {
>           if (tmp->format != AV_PIX_FMT_UYVY422 ||
> @@ -303,6 +435,13 @@ static int decklink_write_video_packet(AVFormatContext *avctx, AVPacket *pkt)
>           }
>   
>           frame = new decklink_frame(ctx, avpacket, st->codecpar->codec_id, ctx->bmd_height, ctx->bmd_width);
> +
> +#if CONFIG_LIBKLVANC
> +        ret = decklink_construct_vanc(avctx, ctx, pkt, frame);
> +        if (ret != 0) {
> +            av_log(avctx, AV_LOG_ERROR, "Failed to construct VANC\n");
> +        }
> +#endif
>       }
>   
>       if (!frame) {
> @@ -393,6 +532,9 @@ av_cold int ff_decklink_write_header(AVFormatContext *avctx)
>       ctx->list_formats = cctx->list_formats;
>       ctx->preroll      = cctx->preroll;
>       cctx->ctx = ctx;
> +#if CONFIG_LIBKLVANC
> +    klvanc_context_create(&ctx->vanc_ctx);
> +#endif

Based on my perusal of the implementation of klvanc_context_create(), 
the function may fail, yet any failure conditions are not handled here. 
Admittedly, the only failure case I see in an out of memory error.

>   
>       /* List available devices and exit. */
>       if (ctx->list_devices) {
> 


More information about the ffmpeg-devel mailing list