On 9/28/18 2:58 AM, Marton Balint wrote:
>
>
> On Mon, 24 Sep 2018, Karthick J wrote:
>
> > From: Karthick Jeyapal <kjeya...@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.
Thanks for review comments. Please find my relevant replies inlined.
I have submitted a new v2 patch.
>
>
> > ---
> > 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}.
> >
> > +@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.
Done.
>
> 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.
Done
>
> > };
> >
> > #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:
Yes, that makes it very simple. Thanks for pointing it out with an example.
That's really helpful.
>
> 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) {
A minor change here. Timebase is reset to 1000000 units. Hence, I am using
r_frame_rate for this.
> 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.
Done
>
> Regards,
> Marton
Thanks and regards,
Karthick
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
