diff mbox

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

Message ID 1537789792-6200-1-git-send-email-kjeyapal@akamai.com
State Superseded
Headers show

Commit Message

Jeyapal, Karthick Sept. 24, 2018, 11:49 a.m. UTC
From: Karthick Jeyapal <kjeyapal@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.
---
 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(+)

Comments

Devin Heitmueller Sept. 24, 2018, 2:12 p.m. UTC | #1
Hello Karthick,


> On Sep 24, 2018, at 7:49 AM, Karthick J <kjeyapal@akamai.com> wrote:
> 
> From: Karthick Jeyapal <kjeyapal@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.

I can certainly see the usefulness of such a feature, but is the decklink module really the right place for this?  This feels like something that should be done through a filter (either as a multimedia filter or a BSF).

Does anyone else have an suggestions as to a better place to do this?

Devin

---
Devin Heitmueller - LTN Global Communications
dheitmueller@ltnglobal.com
Jeyapal, Karthick Sept. 25, 2018, 8:43 a.m. UTC | #2
On 9/24/18 7:42 PM, Devin Heitmueller wrote:
> Hello Karthick,

>

>

>> On Sep 24, 2018, at 7:49 AM, Karthick J <kjeyapal@akamai.com> wrote:

>>

>> From: Karthick Jeyapal <kjeyapal@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.

>

> I can certainly see the usefulness of such a feature, but is the decklink module really the right place for this?  This feels like something that should be done through a filter (either as a multimedia filter or a BSF).

Hi Devin,

Thank you very much for the feedback. I agree with you that if it can be done through a filter, then it is certainly a better place to do it. 
But I as far I understand it can't be implemented reliably in a filter, without imposing additional restrictions and/or added complexities. This is primarily due to the fact the frames might take different times to pass through the pipeline thread in each hardware and reach the filter function at different times and hence losing some synchronization w.r.t system time. In other words, some modules in the pipeline contains CPU intensive code(such as video decode), before it reaches the filter function. The thread that needs to do this should be very lightweight without any CPU intensive operations. And for better reliability it needs to be done as soon as the frame is received from the driver.
For example, the video frame(captured by decklink device) could take different times to pass through V210 decoder due to HW differences and/or CPU load due to other encoder threads. This unpredictable decoder delay kind of rules out multimedia filters for this kind of operation. 
Now a bitstream filter(BSF) can mitigate this issue to some extent as it sits before a decoder. We will still need to insert a thread(and associated buffering) in the BSF, so that the decoder is decoupled from this time-sensitive thread. But still it doesn't provide any guarantee against CPU intensive operations performed in the capture plugin. For example, the Decklink plugin performs some VANC processing which could be CPU intensive in a low-end 2-core Intel processor. Or even if we assume Decklink plugin doesn't perform any CPU intensive operations, we cannot guarantee the same for other capture device plugins.
Another option to implement this in filters would be to use "copyts" and drop the frames based of PTS/DTS value instead of system time. But then this imposes a restriction that "copyts" needs to be used mandatorily. If somebody needs to use it without "copyts", then it won't work.
My understanding on ffmpeg is limited, and hence the above explanation may not be entirely correct. So, please feel free to correct me.

Regards,
Karthick
>

> Does anyone else have an suggestions as to a better place to do this?

>

> Devin

>

> ---

> Devin Heitmueller - LTN Global Communications

> dheitmueller@ltnglobal.com

>
Marton Balint Sept. 26, 2018, 9:14 p.m. UTC | #3
On Tue, 25 Sep 2018, Jeyapal, Karthick wrote:

>
> On 9/24/18 7:42 PM, Devin Heitmueller wrote:
>> Hello Karthick,
>>
>>
>>> On Sep 24, 2018, at 7:49 AM, Karthick J <kjeyapal@akamai.com> wrote:
>>>
>>> From: Karthick Jeyapal <kjeyapal@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.
>>
>> I can certainly see the usefulness of such a feature, but is the 
>> decklink module really the right place for this?  This feels like 
>> something that should be done through a filter (either as a multimedia 
>> filter or a BSF).
> Hi Devin,
>
> Thank you very much for the feedback. I agree with you that if it can be 
> done through a filter, then it is certainly a better place to do it. But 
> I as far I understand it can't be implemented reliably in a filter, 
> without imposing additional restrictions and/or added complexities. This 
> is primarily due to the fact the frames might take different times to 
> pass through the pipeline thread in each hardware and reach the filter 
> function at different times and hence losing some synchronization w.r.t 
> system time. In other words, some modules in the pipeline contains CPU 
> intensive code(such as video decode), before it reaches the filter 
> function. The thread that needs to do this should be very lightweight 
> without any CPU intensive operations. And for better reliability it 
> needs to be done as soon as the frame is received from the driver. For 
> example, the video frame(captured by decklink device) could take 
> different times to pass through V210 decoder due to HW differences 
> and/or CPU load due to other encoder threads. This unpredictable decoder 
> delay kind of rules out multimedia filters for this kind of operation. 
> Now a bitstream filter(BSF) can mitigate this issue to some extent as it 
> sits before a decoder. We will still need to insert a thread(and 
> associated buffering) in the BSF, so that the decoder is decoupled from 
> this time-sensitive thread. But still it doesn't provide any guarantee 
> against CPU intensive operations performed in the capture plugin. For 
> example, the Decklink plugin performs some VANC processing which could 
> be CPU intensive in a low-end 2-core Intel processor. Or even if we 
> assume Decklink plugin doesn't perform any CPU intensive operations, we 
> cannot guarantee the same for other capture device plugins. Another 
> option to implement this in filters would be to use "copyts" and drop 
> the frames based of PTS/DTS value instead of system time. But then this 
> imposes a restriction that "copyts" needs to be used mandatorily. If 
> somebody needs to use it without "copyts", then it won't work. My 
> understanding on ffmpeg is limited, and hence the above explanation may 
> not be entirely correct. So, please feel free to correct me.

How about adding such an option to ffmpeg.c? You still can use wallclock 
timestamps in decklink, and then drop the frames (packets) in ffmpeg.c 
before the timestamps are touched.

Another approch might be to store the wallclock frame time as some kind of 
metadata (as it is done for "timecode") and then add the possiblity to 
f_select to drop based on this. However the evaluation engine has no 
concept of complex objects (like frame, or frame metadata) so this 
probably needs additional work.

Regards,
Marton
Jeyapal, Karthick Sept. 27, 2018, 11:14 a.m. UTC | #4
On 9/27/18 2:44 AM, Marton Balint wrote:
>

>

> On Tue, 25 Sep 2018, Jeyapal, Karthick wrote:

>

>>

>> On 9/24/18 7:42 PM, Devin Heitmueller wrote:

>>> Hello Karthick,

>>>

>>>

>>>> On Sep 24, 2018, at 7:49 AM, Karthick J <kjeyapal@akamai.com> wrote:

>>>>

>>>> From: Karthick Jeyapal <kjeyapal@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. 

>>>

>>> I can certainly see the usefulness of such a feature, but is the decklink module really the right place for this?  This feels like something that should be done through a filter (either as a multimedia filter or a BSF). 

>> Hi Devin,

>>

>> Thank you very much for the feedback. I agree with you that if it can be done through a filter, then it is certainly a better place to do it. But I as far I understand it can't be implemented reliably in a filter, without imposing additional restrictions and/or added complexities. This is primarily due to the fact the frames might take different times to pass through the pipeline thread in each hardware and reach the filter function at different times and hence losing some synchronization w.r.t system time. In other words, some modules in the pipeline contains CPU intensive code(such as video decode), before it reaches the filter function. The thread that needs to do this should be very lightweight without any CPU intensive operations. And for better reliability it needs to be done as soon as the frame is received from the driver. For example, the video frame(captured by decklink device) could take different times to pass through V210 decoder due to HW differences and/or CPU load due to other encoder threads. This unpredictable decoder delay kind of rules out multimedia filters for this kind of operation. Now a bitstream filter(BSF) can mitigate this issue to some extent as it sits before a decoder. We will still need to insert a thread(and associated buffering) in the BSF, so that the decoder is decoupled from this time-sensitive thread. But still it doesn't provide any guarantee against CPU intensive operations performed in the capture plugin. For example, the Decklink plugin performs some VANC processing which could be CPU intensive in a low-end 2-core Intel processor. Or even if we assume Decklink plugin doesn't perform any CPU intensive operations, we cannot guarantee the same for other capture device plugins. Another option to implement this in filters would be to use "copyts" and drop the frames based of PTS/DTS value instead of system time. But then this imposes a restriction that "copyts" needs to be used mandatorily. If somebody needs to use it without "copyts", then it won't work. My understanding on ffmpeg is limited, and hence the above explanation may not be entirely correct. So, please feel free to correct me. 

>

> How about adding such an option to ffmpeg.c? You still can use wallclock timestamps in decklink, and then drop the frames (packets) in ffmpeg.c before the timestamps are touched.

Yes, that's true. But, I will have to set the decklink options abs_wallclock and decklink_copyts for it work correctly. This will be the restriction that we will impose on the user of this option.
If I have to set those two decklink options, then an additional copyts option along with a f_select expression that you had suggested below will do the job without any changes to the code. For example, I used the select expr "lte(mod(pts*1000000*tb, 6*1000000), 41666)+selected_n" where 6 is the segment size and 41666 is the frame duration of 24fps video. This seems like an easiest workaround for this issue. Thank you very much for suggesting the usage of f_select. 
>

> Another approch might be to store the wallclock frame time as some kind of metadata (as it is done for "timecode") and then add the possiblity to f_select to drop based on this. However the evaluation engine has no concept of complex objects (like frame, or frame metadata) so this probably needs additional work.

This involves a lot of extra work for a feature that can be implemented very easily on the capture plugin. And still other capture plugins will have to add the relevant metadata/sidedata for this feature to work for them. If you still think that decklink plugin is not the right place to add this feature, then I respect that decision. I will live with the f_select solution with extra restrictions on timestamping options (
Thanks again for your valuable suggestions.

Regards,
Karthick
>

> Regards,

> Marton

> _______________________________________________

> ffmpeg-devel mailing list

> ffmpeg-devel@ffmpeg.org

> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Devin Heitmueller Sept. 27, 2018, 1:19 p.m. UTC | #5
Hi Karthick,


>> 
>> Another approch might be to store the wallclock frame time as some kind of metadata (as it is done for "timecode") and then add the possiblity to f_select to drop based on this. However the evaluation engine has no concept of complex objects (like frame, or frame metadata) so this probably needs additional work.
> This involves a lot of extra work for a feature that can be implemented very easily on the capture plugin. And still other capture plugins will have to add the relevant metadata/sidedata for this feature to work for them. If you still think that decklink plugin is not the right place to add this feature, then I respect that decision. I will live with the f_select solution with extra restrictions on timestamping options (
> Thanks again for your valuable suggestions.
> 

After further discussion of the alternatives, putting it directly in the decklink module does seem like the least invasive option.  While it would be nice if this functionality could be shared with other realtime sources, the framework doesn’t really seem to lend itself to that and it seems like a good bit more trouble than it’s worth.

I withdrawal any objections I had to the original patch.

Devin

---
Devin Heitmueller - LTN Global Communications
dheitmueller@ltnglobal.com
Marton Balint Sept. 27, 2018, 9:28 p.m. UTC | #6
On Mon, 24 Sep 2018, Karthick J wrote:

> From: Karthick Jeyapal <kjeyapal@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}.
> 
> +@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
Jeyapal, Karthick Sept. 28, 2018, 7:32 a.m. UTC | #7
On 9/28/18 2:58 AM, Marton Balint wrote:
>

>

> On Mon, 24 Sep 2018, Karthick J wrote:

>

> > From: Karthick Jeyapal <kjeyapal@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
Jeyapal, Karthick Sept. 28, 2018, 7:34 a.m. UTC | #8
On 9/27/18 6:49 PM, Devin Heitmueller wrote:
> Hi Karthick,

>

>

>>>

>>> Another approch might be to store the wallclock frame time as some kind of metadata (as it is done for "timecode") and then add the possiblity to f_select to drop based on this. However the evaluation engine has no concept of complex objects (like frame, or frame metadata) so this probably needs additional work.

>> This involves a lot of extra work for a feature that can be implemented very easily on the capture plugin. And still other capture plugins will have to add the relevant metadata/sidedata for this feature to work for them. If you still think that decklink plugin is not the right place to add this feature, then I respect that decision. I will live with the f_select solution with extra restrictions on timestamping options (

>> Thanks again for your valuable suggestions.

>>

>

> After further discussion of the alternatives, putting it directly in the decklink module does seem like the least invasive option.  While it would be nice if this functionality could be shared with other realtime sources, the framework doesn’t really seem to lend itself to that and it seems like a good bit more trouble than it’s worth.

>

> I withdrawal any objections I had to the original patch.

Thanks for your comments. I have submitted a new v2 patch for this feature.

Regards,
Karthick
>

> Devin

>

> ---

> Devin Heitmueller - LTN Global Communications

> dheitmueller@ltnglobal.com

>

>

> _______________________________________________

> ffmpeg-devel mailing list

> ffmpeg-devel@ffmpeg.org

> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Marton Balint Sept. 30, 2018, 7:11 p.m. UTC | #9
On Fri, 28 Sep 2018, Jeyapal, Karthick wrote:

>
> On 9/28/18 2:58 AM, Marton Balint wrote:
>>
>>
>> On Mon, 24 Sep 2018, Karthick J wrote:
>>
>>> From: Karthick Jeyapal <kjeyapal@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.

Thanks, applied with some minor cosmetic changes.

Regards,
Marton
diff mbox

Patch

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}.
+
 @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;
 };
 
 #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;
+        }
+    }
+
     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 },
     { NULL },
 };