[FFmpeg-devel] [PATCH] avdevice/decklink: Add option to align Capture start time

[Marton Balint]

On Mon, 24 Sep 2018, Karthick J wrote:

> From: Karthick Jeyapal <kjeyapal at akamai.com>
>
> This option is useful for maintaining input synchronization across N
> different hardware devices deployed for 'N-way' redundancy.
> The system time of different hardware devices should be synchronized
> with protocols such as NTP or PTP, before using this option.

Here is a review of the patch if you want to proceed with implementing 
the feature in decklink. The introduced code can be simple/small enough 
that I think it is OK to include it if you prefer that.


> ---
> doc/indevs.texi                 | 10 ++++++++++
> libavdevice/decklink_common_c.h |  1 +
> libavdevice/decklink_dec.cpp    | 20 ++++++++++++++++++++
> libavdevice/decklink_dec_c.c    |  1 +
> 4 files changed, 32 insertions(+)
>
> diff --git a/doc/indevs.texi b/doc/indevs.texi
> index ed2784b..dfd530a 100644
> --- a/doc/indevs.texi
> +++ b/doc/indevs.texi
> @@ -371,6 +371,16 @@ If set to @option{true}, timestamps are forwarded as they are without removing
> the initial offset.
> Defaults to @option{false}.
> 
> + at item timestamp_align
> +Capture start time alignment in seconds. If set to nonzero, input frames are
> +dropped till the system timestamp aligns with configured value.
> +Alignment difference of upto one frame duration is tolerated.
> +This is useful for maintaining input synchronization across N different
> +hardware devices deployed for 'N-way' redundancy. The system time of different
> +hardware devices should be synchronized with protocols such as NTP or PTP,
> +before using this option.
> +Defaults to @samp{0}.

This approach is not perfect. If frame callbacks are roughly at the same 
time as the alignment, then - because of the jitter of the callbacks - 
segments can be skipped or 1 frame difference is possible. At least 
mention this in the docs.

Also I remember having some burst callbacks in case of signal 
loss/return on newer DeckLink models, that might mess this up as well.

An always working synchornization method probably needs a continous SDI 
timecode which is unfortunately not an option in most cases.


> +
> @end table
> 
> @subsection Examples
> diff --git a/libavdevice/decklink_common_c.h b/libavdevice/decklink_common_c.h
> index 32a5d70..c4a8985 100644
> --- a/libavdevice/decklink_common_c.h
> +++ b/libavdevice/decklink_common_c.h
> @@ -56,6 +56,7 @@ struct decklink_cctx {
>     int raw_format;
>     int64_t queue_size;
>     int copyts;
> +    int timestamp_align;

Make this int64_t and the option below AV_OPT_TYPE_DURATION.

> };
> 
> #endif /* AVDEVICE_DECKLINK_COMMON_C_H */
> diff --git a/libavdevice/decklink_dec.cpp b/libavdevice/decklink_dec.cpp
> index 7fabef2..24f5ca9 100644
> --- a/libavdevice/decklink_dec.cpp
> +++ b/libavdevice/decklink_dec.cpp
> @@ -703,6 +703,26 @@ HRESULT decklink_input_callback::VideoInputFrameArrived(
>         return S_OK;
>     }
> 
> +    // Drop the frames till system's timestamp aligns with the configured value.
> +    if (0 == ctx->frameCount && cctx->timestamp_align) {
> +        int64_t current_time_us = av_gettime();
> +        int64_t align_factor_us = (cctx->timestamp_align * 1000000);
> +        int remainder = current_time_us % align_factor_us;
> +        if (videoFrame) {
> +            videoFrame->GetStreamTime(&frameTime, &frameDuration, 1000000);
> +        } else if (audioFrame) {
> +            long sample_count = audioFrame->GetSampleFrameCount();
> +            frameDuration = (long)(sample_count * 1000000) / bmdAudioSampleRate48kHz;
> +        } else {
> +            frameDuration = 0;
> +        }
> +        // threshold of one frameDuration
> +        if(remainder > frameDuration) {
> +            ++ctx->dropped;
> +            return S_OK;
> +        }

You already know the frame rate, so I'd simply write something like:

if (ctx->frameCount == 0 && ctx->timestamp_align) {
     AVRational remainder = av_make_q(av_gettime() % cctx->timestamp_align, 1000000);
     if (av_cmp_q(remainder, st->video_st->time_base) > 0) {
         ctx->dropped++;
         return S_OK;
     }
}

> +    }
> +
>     ctx->frameCount++;
>     if (ctx->audio_pts_source == PTS_SRC_WALLCLOCK || ctx->video_pts_source == PTS_SRC_WALLCLOCK)
>         wallclock = av_gettime_relative();
> diff --git a/libavdevice/decklink_dec_c.c b/libavdevice/decklink_dec_c.c
> index 6ab3819..bef9c14 100644
> --- a/libavdevice/decklink_dec_c.c
> +++ b/libavdevice/decklink_dec_c.c
> @@ -84,6 +84,7 @@ static const AVOption options[] = {
>     { "queue_size",    "input queue buffer size",   OFFSET(queue_size),   AV_OPT_TYPE_INT64, { .i64 = (1024 * 1024 * 1024)}, 0, INT64_MAX, DEC },
>     { "audio_depth",   "audio bitdepth (16 or 32)", OFFSET(audio_depth),  AV_OPT_TYPE_INT,   { .i64 = 16}, 16, 32, DEC },
>     { "decklink_copyts", "copy timestamps, do not remove the initial offset", OFFSET(copyts), AV_OPT_TYPE_BOOL, { .i64 = 0 }, 0, 1, DEC },
> +    { "timestamp_align", "Capture start time alignment (in seconds)", OFFSET(timestamp_align), AV_OPT_TYPE_INT, { .i64 = 0 }, 0, INT_MAX, DEC },

AV_OPT_TYPE_DURATION.

Regards,
Marton