diff mbox series

[FFmpeg-devel,v3] ffmpeg: add option -isync

Message ID 20220704162912.5577-1-ffmpeg@gyani.pro
State New
Headers show
Series [FFmpeg-devel,v3] ffmpeg: add option -isync | expand

Checks

Context Check Description
yinshiyou/make_loongarch64 success Make finished
yinshiyou/make_fate_loongarch64 success Make fate finished
andriy/make_x86 success Make finished
andriy/make_fate_x86 success Make fate finished
andriy/make_armv7_RPi4 success Make finished
andriy/make_fate_armv7_RPi4 success Make fate finished

Commit Message

Gyan Doshi July 4, 2022, 4:29 p.m. UTC 
This is a per-file input option that adjusts an input's timestamps
with reference to another input, so that emitted packet timestamps
account for the difference between the start times of the two inputs.

Typical use case is to sync two or more live inputs such as from capture
devices. Both the target and reference input source timestamps should be
based on the same clock source.

If not all inputs have timestamps, the wallclock times at the time of
reception of inputs shall be used. FFmpeg must have been compiled with
thread support for this last case.
---
This is a single patch with the reception wallclock stored only in
fftools. I find this sub-optimal because find_stream_info will have
demuxed an indeterminate number of frames by the time the wallclock is
stored. But it's better than nothing.

 doc/ffmpeg.texi      | 16 ++++++++++
 fftools/ffmpeg.c     |  4 +++
 fftools/ffmpeg.h     |  3 ++
 fftools/ffmpeg_opt.c | 69 ++++++++++++++++++++++++++++++++++++++++++++
 4 files changed, 92 insertions(+)

Comments

Anton Khirnov July 7, 2022, 9:41 a.m. UTC | #1 
Quoting Gyan Doshi (2022-07-04 18:29:12)
> This is a per-file input option that adjusts an input's timestamps
> with reference to another input, so that emitted packet timestamps
> account for the difference between the start times of the two inputs.
> 
> Typical use case is to sync two or more live inputs such as from capture
> devices. Both the target and reference input source timestamps should be
> based on the same clock source.
> 
> If not all inputs have timestamps, the wallclock times at the time of
> reception of inputs shall be used. FFmpeg must have been compiled with
> thread support for this last case.

I'm wondering if simply using the other input's InputFile.ts_offset
wouldn't achieve the same effect with much less complexity.
Gyan Doshi July 8, 2022, 3:56 a.m. UTC | #2 
On 2022-07-07 03:11 pm, Anton Khirnov wrote:
> Quoting Gyan Doshi (2022-07-04 18:29:12)
>> This is a per-file input option that adjusts an input's timestamps
>> with reference to another input, so that emitted packet timestamps
>> account for the difference between the start times of the two inputs.
>>
>> Typical use case is to sync two or more live inputs such as from capture
>> devices. Both the target and reference input source timestamps should be
>> based on the same clock source.
>>
>> If not all inputs have timestamps, the wallclock times at the time of
>> reception of inputs shall be used. FFmpeg must have been compiled with
>> thread support for this last case.
> I'm wondering if simply using the other input's InputFile.ts_offset
> wouldn't achieve the same effect with much less complexity.

That's what I initially did. But since the code can also use two other 
sources for start times (start_time_realtime, first_pkt_wallclock),
those intervals may not exactly match the difference between 
fmctx->start_times so I use a generic calculation.

Regards,
Gyan
Gyan Doshi July 9, 2022, 6:27 p.m. UTC | #3 
On 2022-07-08 09:26 am, Gyan Doshi wrote:
>
>
> On 2022-07-07 03:11 pm, Anton Khirnov wrote:
>> Quoting Gyan Doshi (2022-07-04 18:29:12)
>>> This is a per-file input option that adjusts an input's timestamps
>>> with reference to another input, so that emitted packet timestamps
>>> account for the difference between the start times of the two inputs.
>>>
>>> Typical use case is to sync two or more live inputs such as from 
>>> capture
>>> devices. Both the target and reference input source timestamps 
>>> should be
>>> based on the same clock source.
>>>
>>> If not all inputs have timestamps, the wallclock times at the time of
>>> reception of inputs shall be used. FFmpeg must have been compiled with
>>> thread support for this last case.
>> I'm wondering if simply using the other input's InputFile.ts_offset
>> wouldn't achieve the same effect with much less complexity.
>
> That's what I initially did. But since the code can also use two other 
> sources for start times (start_time_realtime, first_pkt_wallclock),
> those intervals may not exactly match the difference between 
> fmctx->start_times so I use a generic calculation.

Plan to push on Monday, if no further changes. 5.1 is to be cut soon.

Regards,
Gyan
Paul B Mahol July 9, 2022, 7:43 p.m. UTC | #4 
On Sat, Jul 9, 2022 at 8:28 PM Gyan Doshi <ffmpeg@gyani.pro> wrote:

>
>
> On 2022-07-08 09:26 am, Gyan Doshi wrote:
> >
> >
> > On 2022-07-07 03:11 pm, Anton Khirnov wrote:
> >> Quoting Gyan Doshi (2022-07-04 18:29:12)
> >>> This is a per-file input option that adjusts an input's timestamps
> >>> with reference to another input, so that emitted packet timestamps
> >>> account for the difference between the start times of the two inputs.
> >>>
> >>> Typical use case is to sync two or more live inputs such as from
> >>> capture
> >>> devices. Both the target and reference input source timestamps
> >>> should be
> >>> based on the same clock source.
> >>>
> >>> If not all inputs have timestamps, the wallclock times at the time of
> >>> reception of inputs shall be used. FFmpeg must have been compiled with
> >>> thread support for this last case.
> >> I'm wondering if simply using the other input's InputFile.ts_offset
> >> wouldn't achieve the same effect with much less complexity.
> >
> > That's what I initially did. But since the code can also use two other
> > sources for start times (start_time_realtime, first_pkt_wallclock),
> > those intervals may not exactly match the difference between
> > fmctx->start_times so I use a generic calculation.
>
> Plan to push on Monday, if no further changes. 5.1 is to be cut soon.
>
>
Why big rush, its not so critical.


Regards,
> Gyan
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>
> To unsubscribe, visit link above, or email
> ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
>
Gyan Doshi July 9, 2022, 7:56 p.m. UTC | #5 
On 2022-07-10 01:13 am, Paul B Mahol wrote:
> On Sat, Jul 9, 2022 at 8:28 PM Gyan Doshi <ffmpeg@gyani.pro> wrote:
>
>>
>> On 2022-07-08 09:26 am, Gyan Doshi wrote:
>>>
>>> On 2022-07-07 03:11 pm, Anton Khirnov wrote:
>>>> Quoting Gyan Doshi (2022-07-04 18:29:12)
>>>>> This is a per-file input option that adjusts an input's timestamps
>>>>> with reference to another input, so that emitted packet timestamps
>>>>> account for the difference between the start times of the two inputs.
>>>>>
>>>>> Typical use case is to sync two or more live inputs such as from
>>>>> capture
>>>>> devices. Both the target and reference input source timestamps
>>>>> should be
>>>>> based on the same clock source.
>>>>>
>>>>> If not all inputs have timestamps, the wallclock times at the time of
>>>>> reception of inputs shall be used. FFmpeg must have been compiled with
>>>>> thread support for this last case.
>>>> I'm wondering if simply using the other input's InputFile.ts_offset
>>>> wouldn't achieve the same effect with much less complexity.
>>> That's what I initially did. But since the code can also use two other
>>> sources for start times (start_time_realtime, first_pkt_wallclock),
>>> those intervals may not exactly match the difference between
>>> fmctx->start_times so I use a generic calculation.
>> Plan to push on Monday, if no further changes. 5.1 is to be cut soon.
>>
>>
> Why big rush, its not so critical.

Patch was first sent on 22nd June. Only one reviewer asked for changes.

Regards,
Gyan
Hendrik Leppkes July 9, 2022, 8:49 p.m. UTC | #6 
On Sat, Jul 9, 2022 at 9:56 PM Gyan Doshi <ffmpeg@gyani.pro> wrote:
>
>
>
> On 2022-07-10 01:13 am, Paul B Mahol wrote:
> > On Sat, Jul 9, 2022 at 8:28 PM Gyan Doshi <ffmpeg@gyani.pro> wrote:
> >
> >>
> >> On 2022-07-08 09:26 am, Gyan Doshi wrote:
> >>>
> >>> On 2022-07-07 03:11 pm, Anton Khirnov wrote:
> >>>> Quoting Gyan Doshi (2022-07-04 18:29:12)
> >>>>> This is a per-file input option that adjusts an input's timestamps
> >>>>> with reference to another input, so that emitted packet timestamps
> >>>>> account for the difference between the start times of the two inputs.
> >>>>>
> >>>>> Typical use case is to sync two or more live inputs such as from
> >>>>> capture
> >>>>> devices. Both the target and reference input source timestamps
> >>>>> should be
> >>>>> based on the same clock source.
> >>>>>
> >>>>> If not all inputs have timestamps, the wallclock times at the time of
> >>>>> reception of inputs shall be used. FFmpeg must have been compiled with
> >>>>> thread support for this last case.
> >>>> I'm wondering if simply using the other input's InputFile.ts_offset
> >>>> wouldn't achieve the same effect with much less complexity.
> >>> That's what I initially did. But since the code can also use two other
> >>> sources for start times (start_time_realtime, first_pkt_wallclock),
> >>> those intervals may not exactly match the difference between
> >>> fmctx->start_times so I use a generic calculation.
> >> Plan to push on Monday, if no further changes. 5.1 is to be cut soon.
> >>
> >>
> > Why big rush, its not so critical.
>
> Patch was first sent on 22nd June. Only one reviewer asked for changes.
>

It does however not seem like that reviewer has ultimately signed off, did he?
Lack of response for a day does not make previous objections just go away.

- Hendrik
Anton Khirnov July 10, 2022, 5:13 p.m. UTC | #7 
Quoting Gyan Doshi (2022-07-09 21:56:10)
> 
> 
> On 2022-07-10 01:13 am, Paul B Mahol wrote:
> > On Sat, Jul 9, 2022 at 8:28 PM Gyan Doshi <ffmpeg@gyani.pro> wrote:
> >
> >>
> >> On 2022-07-08 09:26 am, Gyan Doshi wrote:
> >>>
> >>> On 2022-07-07 03:11 pm, Anton Khirnov wrote:
> >>>> Quoting Gyan Doshi (2022-07-04 18:29:12)
> >>>>> This is a per-file input option that adjusts an input's timestamps
> >>>>> with reference to another input, so that emitted packet timestamps
> >>>>> account for the difference between the start times of the two inputs.
> >>>>>
> >>>>> Typical use case is to sync two or more live inputs such as from
> >>>>> capture
> >>>>> devices. Both the target and reference input source timestamps
> >>>>> should be
> >>>>> based on the same clock source.
> >>>>>
> >>>>> If not all inputs have timestamps, the wallclock times at the time of
> >>>>> reception of inputs shall be used. FFmpeg must have been compiled with
> >>>>> thread support for this last case.
> >>>> I'm wondering if simply using the other input's InputFile.ts_offset
> >>>> wouldn't achieve the same effect with much less complexity.
> >>> That's what I initially did. But since the code can also use two other
> >>> sources for start times (start_time_realtime, first_pkt_wallclock),
> >>> those intervals may not exactly match the difference between
> >>> fmctx->start_times so I use a generic calculation.
> >> Plan to push on Monday, if no further changes. 5.1 is to be cut soon.
> >>
> >>
> > Why big rush, its not so critical.
> 
> Patch was first sent on 22nd June.

Many patches wait for way longer than that.

> Only one reviewer asked for changes.

That entitles you to disregard me then?
Anton Khirnov July 10, 2022, 5:16 p.m. UTC | #8 
Quoting Gyan Doshi (2022-07-08 05:56:21)
> 
> 
> On 2022-07-07 03:11 pm, Anton Khirnov wrote:
> > Quoting Gyan Doshi (2022-07-04 18:29:12)
> >> This is a per-file input option that adjusts an input's timestamps
> >> with reference to another input, so that emitted packet timestamps
> >> account for the difference between the start times of the two inputs.
> >>
> >> Typical use case is to sync two or more live inputs such as from capture
> >> devices. Both the target and reference input source timestamps should be
> >> based on the same clock source.
> >>
> >> If not all inputs have timestamps, the wallclock times at the time of
> >> reception of inputs shall be used. FFmpeg must have been compiled with
> >> thread support for this last case.
> > I'm wondering if simply using the other input's InputFile.ts_offset
> > wouldn't achieve the same effect with much less complexity.
> 
> That's what I initially did. But since the code can also use two other 
> sources for start times (start_time_realtime, first_pkt_wallclock),
> those intervals may not exactly match the difference between 
> fmctx->start_times so I use a generic calculation.

In what cases is it better to use either of those two other sources?

As per the commit message, the timestamps of both inputs are supposed to
come from the same clock. Then it seems to me that offsetting each of
those streams by different amounts would break synchronization rather
than improve it.
Gyan Doshi July 10, 2022, 6:02 p.m. UTC | #9 
On 2022-07-10 10:46 pm, Anton Khirnov wrote:
> Quoting Gyan Doshi (2022-07-08 05:56:21)
>>
>> On 2022-07-07 03:11 pm, Anton Khirnov wrote:
>>> Quoting Gyan Doshi (2022-07-04 18:29:12)
>>>> This is a per-file input option that adjusts an input's timestamps
>>>> with reference to another input, so that emitted packet timestamps
>>>> account for the difference between the start times of the two inputs.
>>>>
>>>> Typical use case is to sync two or more live inputs such as from capture
>>>> devices. Both the target and reference input source timestamps should be
>>>> based on the same clock source.
>>>>
>>>> If not all inputs have timestamps, the wallclock times at the time of
>>>> reception of inputs shall be used. FFmpeg must have been compiled with
>>>> thread support for this last case.
>>> I'm wondering if simply using the other input's InputFile.ts_offset
>>> wouldn't achieve the same effect with much less complexity.
>> That's what I initially did. But since the code can also use two other
>> sources for start times (start_time_realtime, first_pkt_wallclock),
>> those intervals may not exactly match the difference between
>> fmctx->start_times so I use a generic calculation.
> In what cases is it better to use either of those two other sources?
>
> As per the commit message, the timestamps of both inputs are supposed to
> come from the same clock. Then it seems to me that offsetting each of
> those streams by different amounts would break synchronization rather
> than improve it.

The first preference, when available, stores the epoch time closest to 
time of capture. That would eliminate some jitter.
The 2nd preference is the fmctx->start_time. The 3rd is the reception 
wallclock. It is a fallback. It will likely lead to the worst sync.

Regards,
Gyan




>
Anton Khirnov July 10, 2022, 6:51 p.m. UTC | #10 
Quoting Gyan Doshi (2022-07-10 20:02:38)
> 
> 
> On 2022-07-10 10:46 pm, Anton Khirnov wrote:
> > Quoting Gyan Doshi (2022-07-08 05:56:21)
> >>
> >> On 2022-07-07 03:11 pm, Anton Khirnov wrote:
> >>> Quoting Gyan Doshi (2022-07-04 18:29:12)
> >>>> This is a per-file input option that adjusts an input's timestamps
> >>>> with reference to another input, so that emitted packet timestamps
> >>>> account for the difference between the start times of the two inputs.
> >>>>
> >>>> Typical use case is to sync two or more live inputs such as from capture
> >>>> devices. Both the target and reference input source timestamps should be
> >>>> based on the same clock source.
> >>>>
> >>>> If not all inputs have timestamps, the wallclock times at the time of
> >>>> reception of inputs shall be used. FFmpeg must have been compiled with
> >>>> thread support for this last case.
> >>> I'm wondering if simply using the other input's InputFile.ts_offset
> >>> wouldn't achieve the same effect with much less complexity.
> >> That's what I initially did. But since the code can also use two other
> >> sources for start times (start_time_realtime, first_pkt_wallclock),
> >> those intervals may not exactly match the difference between
> >> fmctx->start_times so I use a generic calculation.
> > In what cases is it better to use either of those two other sources?
> >
> > As per the commit message, the timestamps of both inputs are supposed to
> > come from the same clock. Then it seems to me that offsetting each of
> > those streams by different amounts would break synchronization rather
> > than improve it.
> 
> The first preference, when available, stores the epoch time closest to 
> time of capture. That would eliminate some jitter.
> The 2nd preference is the fmctx->start_time. The 3rd is the reception 
> wallclock. It is a fallback. It will likely lead to the worst sync.

You did not answer my question.
If both streams use the same clock, then how is offsetting them by
different amounts improve sync?
Gyan Doshi July 11, 2022, 6:46 a.m. UTC | #11 
On 2022-07-11 12:21 am, Anton Khirnov wrote:
> Quoting Gyan Doshi (2022-07-10 20:02:38)
>>
>> On 2022-07-10 10:46 pm, Anton Khirnov wrote:
>>> Quoting Gyan Doshi (2022-07-08 05:56:21)
>>>> On 2022-07-07 03:11 pm, Anton Khirnov wrote:
>>>>> Quoting Gyan Doshi (2022-07-04 18:29:12)
>>>>>> This is a per-file input option that adjusts an input's timestamps
>>>>>> with reference to another input, so that emitted packet timestamps
>>>>>> account for the difference between the start times of the two inputs.
>>>>>>
>>>>>> Typical use case is to sync two or more live inputs such as from capture
>>>>>> devices. Both the target and reference input source timestamps should be
>>>>>> based on the same clock source.
>>>>>>
>>>>>> If not all inputs have timestamps, the wallclock times at the time of
>>>>>> reception of inputs shall be used. FFmpeg must have been compiled with
>>>>>> thread support for this last case.
>>>>> I'm wondering if simply using the other input's InputFile.ts_offset
>>>>> wouldn't achieve the same effect with much less complexity.
>>>> That's what I initially did. But since the code can also use two other
>>>> sources for start times (start_time_realtime, first_pkt_wallclock),
>>>> those intervals may not exactly match the difference between
>>>> fmctx->start_times so I use a generic calculation.
>>> In what cases is it better to use either of those two other sources?
>>>
>>> As per the commit message, the timestamps of both inputs are supposed to
>>> come from the same clock. Then it seems to me that offsetting each of
>>> those streams by different amounts would break synchronization rather
>>> than improve it.
>> The first preference, when available, stores the epoch time closest to
>> time of capture. That would eliminate some jitter.
>> The 2nd preference is the fmctx->start_time. The 3rd is the reception
>> wallclock. It is a fallback. It will likely lead to the worst sync.
> You did not answer my question.
> If both streams use the same clock, then how is offsetting them by
> different amounts improve sync?

Because the clocks can be different at different stages of stream 
conveyance  i.e. capture -> encode -> network relay -> ffmpeg reception.
As long as both use the same clock at a given stage, they represent the 
same sync relation but with some jitter in the mix added with each stage.
The semantics of start_time_realtime is "pts=0 in the stream was 
captured at this real world time" (unix epoch).
The fmctx start will usually be system timestamps at encode or mux. We 
should prefer the earliest stage available, which is what the patch does.

Regards,
Gyan
Anton Khirnov July 13, 2022, 12:30 p.m. UTC | #12 
Quoting Gyan Doshi (2022-07-11 08:46:48)
> 
> 
> On 2022-07-11 12:21 am, Anton Khirnov wrote:
> > Quoting Gyan Doshi (2022-07-10 20:02:38)
> >>
> >> On 2022-07-10 10:46 pm, Anton Khirnov wrote:
> >>> Quoting Gyan Doshi (2022-07-08 05:56:21)
> >>>> On 2022-07-07 03:11 pm, Anton Khirnov wrote:
> >>>>> Quoting Gyan Doshi (2022-07-04 18:29:12)
> >>>>>> This is a per-file input option that adjusts an input's timestamps
> >>>>>> with reference to another input, so that emitted packet timestamps
> >>>>>> account for the difference between the start times of the two inputs.
> >>>>>>
> >>>>>> Typical use case is to sync two or more live inputs such as from capture
> >>>>>> devices. Both the target and reference input source timestamps should be
> >>>>>> based on the same clock source.
> >>>>>>
> >>>>>> If not all inputs have timestamps, the wallclock times at the time of
> >>>>>> reception of inputs shall be used. FFmpeg must have been compiled with
> >>>>>> thread support for this last case.
> >>>>> I'm wondering if simply using the other input's InputFile.ts_offset
> >>>>> wouldn't achieve the same effect with much less complexity.
> >>>> That's what I initially did. But since the code can also use two other
> >>>> sources for start times (start_time_realtime, first_pkt_wallclock),
> >>>> those intervals may not exactly match the difference between
> >>>> fmctx->start_times so I use a generic calculation.
> >>> In what cases is it better to use either of those two other sources?
> >>>
> >>> As per the commit message, the timestamps of both inputs are supposed to
> >>> come from the same clock. Then it seems to me that offsetting each of
> >>> those streams by different amounts would break synchronization rather
> >>> than improve it.
> >> The first preference, when available, stores the epoch time closest to
> >> time of capture. That would eliminate some jitter.
> >> The 2nd preference is the fmctx->start_time. The 3rd is the reception
> >> wallclock. It is a fallback. It will likely lead to the worst sync.
> > You did not answer my question.
> > If both streams use the same clock, then how is offsetting them by
> > different amounts improve sync?
> 
> Because the clocks can be different at different stages of stream 
> conveyance  i.e. capture -> encode -> network relay -> ffmpeg reception.
> As long as both use the same clock at a given stage, they represent the 
> same sync relation but with some jitter in the mix added with each stage.

Why would you send the streams separately and not synchronized before
network transmission?
Gyan Doshi July 13, 2022, 12:53 p.m. UTC | #13 
On 2022-07-13 06:00 pm, Anton Khirnov wrote:
> Quoting Gyan Doshi (2022-07-11 08:46:48)
>>
>> On 2022-07-11 12:21 am, Anton Khirnov wrote:
>>> Quoting Gyan Doshi (2022-07-10 20:02:38)
>>>> On 2022-07-10 10:46 pm, Anton Khirnov wrote:
>>>>> Quoting Gyan Doshi (2022-07-08 05:56:21)
>>>>>> On 2022-07-07 03:11 pm, Anton Khirnov wrote:
>>>>>>> Quoting Gyan Doshi (2022-07-04 18:29:12)
>>>>>>>> This is a per-file input option that adjusts an input's timestamps
>>>>>>>> with reference to another input, so that emitted packet timestamps
>>>>>>>> account for the difference between the start times of the two inputs.
>>>>>>>>
>>>>>>>> Typical use case is to sync two or more live inputs such as from capture
>>>>>>>> devices. Both the target and reference input source timestamps should be
>>>>>>>> based on the same clock source.
>>>>>>>>
>>>>>>>> If not all inputs have timestamps, the wallclock times at the time of
>>>>>>>> reception of inputs shall be used. FFmpeg must have been compiled with
>>>>>>>> thread support for this last case.
>>>>>>> I'm wondering if simply using the other input's InputFile.ts_offset
>>>>>>> wouldn't achieve the same effect with much less complexity.
>>>>>> That's what I initially did. But since the code can also use two other
>>>>>> sources for start times (start_time_realtime, first_pkt_wallclock),
>>>>>> those intervals may not exactly match the difference between
>>>>>> fmctx->start_times so I use a generic calculation.
>>>>> In what cases is it better to use either of those two other sources?
>>>>>
>>>>> As per the commit message, the timestamps of both inputs are supposed to
>>>>> come from the same clock. Then it seems to me that offsetting each of
>>>>> those streams by different amounts would break synchronization rather
>>>>> than improve it.
>>>> The first preference, when available, stores the epoch time closest to
>>>> time of capture. That would eliminate some jitter.
>>>> The 2nd preference is the fmctx->start_time. The 3rd is the reception
>>>> wallclock. It is a fallback. It will likely lead to the worst sync.
>>> You did not answer my question.
>>> If both streams use the same clock, then how is offsetting them by
>>> different amounts improve sync?
>> Because the clocks can be different at different stages of stream
>> conveyance  i.e. capture -> encode -> network relay -> ffmpeg reception.
>> As long as both use the same clock at a given stage, they represent the
>> same sync relation but with some jitter in the mix added with each stage.
> Why would you send the streams separately and not synchronized before
> network transmission?

Because they may arise from separate machines e.g. a video 
teleconference with multiple participants on the LAN, conveyed with NTP 
time of start of stream.

Regards,
Gyan
Anton Khirnov July 14, 2022, 6:46 a.m. UTC | #14 
Quoting Gyan Doshi (2022-07-10 20:02:38)
> 
> 
> On 2022-07-10 10:46 pm, Anton Khirnov wrote:
> > Quoting Gyan Doshi (2022-07-08 05:56:21)
> >>
> >> On 2022-07-07 03:11 pm, Anton Khirnov wrote:
> >>> Quoting Gyan Doshi (2022-07-04 18:29:12)
> >>>> This is a per-file input option that adjusts an input's timestamps
> >>>> with reference to another input, so that emitted packet timestamps
> >>>> account for the difference between the start times of the two inputs.
> >>>>
> >>>> Typical use case is to sync two or more live inputs such as from capture
> >>>> devices. Both the target and reference input source timestamps should be
> >>>> based on the same clock source.
> >>>>
> >>>> If not all inputs have timestamps, the wallclock times at the time of
> >>>> reception of inputs shall be used. FFmpeg must have been compiled with
> >>>> thread support for this last case.
> >>> I'm wondering if simply using the other input's InputFile.ts_offset
> >>> wouldn't achieve the same effect with much less complexity.
> >> That's what I initially did. But since the code can also use two other
> >> sources for start times (start_time_realtime, first_pkt_wallclock),
> >> those intervals may not exactly match the difference between
> >> fmctx->start_times so I use a generic calculation.
> > In what cases is it better to use either of those two other sources?
> >
> > As per the commit message, the timestamps of both inputs are supposed to
> > come from the same clock. Then it seems to me that offsetting each of
> > those streams by different amounts would break synchronization rather
> > than improve it.
> 
> The first preference, when available, stores the epoch time closest to 
> time of capture. That would eliminate some jitter.
> The 2nd preference is the fmctx->start_time. The 3rd is the reception 
> wallclock. It is a fallback. It will likely lead to the worst sync.

In which cases is this fallback useful? I would expect that all
transports you'd want to use this with will have working timestamps.
Gyan Doshi July 14, 2022, 7:47 a.m. UTC | #15 
On 2022-07-14 12:16 pm, Anton Khirnov wrote:
> Quoting Gyan Doshi (2022-07-10 20:02:38)
>>
>> On 2022-07-10 10:46 pm, Anton Khirnov wrote:
>>> Quoting Gyan Doshi (2022-07-08 05:56:21)
>>>> On 2022-07-07 03:11 pm, Anton Khirnov wrote:
>>>>> Quoting Gyan Doshi (2022-07-04 18:29:12)
>>>>>> This is a per-file input option that adjusts an input's timestamps
>>>>>> with reference to another input, so that emitted packet timestamps
>>>>>> account for the difference between the start times of the two inputs.
>>>>>>
>>>>>> Typical use case is to sync two or more live inputs such as from capture
>>>>>> devices. Both the target and reference input source timestamps should be
>>>>>> based on the same clock source.
>>>>>>
>>>>>> If not all inputs have timestamps, the wallclock times at the time of
>>>>>> reception of inputs shall be used. FFmpeg must have been compiled with
>>>>>> thread support for this last case.
>>>>> I'm wondering if simply using the other input's InputFile.ts_offset
>>>>> wouldn't achieve the same effect with much less complexity.
>>>> That's what I initially did. But since the code can also use two other
>>>> sources for start times (start_time_realtime, first_pkt_wallclock),
>>>> those intervals may not exactly match the difference between
>>>> fmctx->start_times so I use a generic calculation.
>>> In what cases is it better to use either of those two other sources?
>>>
>>> As per the commit message, the timestamps of both inputs are supposed to
>>> come from the same clock. Then it seems to me that offsetting each of
>>> those streams by different amounts would break synchronization rather
>>> than improve it.
>> The first preference, when available, stores the epoch time closest to
>> time of capture. That would eliminate some jitter.
>> The 2nd preference is the fmctx->start_time. The 3rd is the reception
>> wallclock. It is a fallback. It will likely lead to the worst sync.
> In which cases is this fallback useful? I would expect that all
> transports you'd want to use this with will have working timestamps.

Agreed. I just kept it as a last resort scenario. Can do without it.

Regards,
Gyan
Anton Khirnov July 14, 2022, 8:15 a.m. UTC | #16 
Quoting Gyan Doshi (2022-07-14 09:47:17)
> 
> 
> On 2022-07-14 12:16 pm, Anton Khirnov wrote:
> > Quoting Gyan Doshi (2022-07-10 20:02:38)
> >>
> >> On 2022-07-10 10:46 pm, Anton Khirnov wrote:
> >>> Quoting Gyan Doshi (2022-07-08 05:56:21)
> >>>> On 2022-07-07 03:11 pm, Anton Khirnov wrote:
> >>>>> Quoting Gyan Doshi (2022-07-04 18:29:12)
> >>>>>> This is a per-file input option that adjusts an input's timestamps
> >>>>>> with reference to another input, so that emitted packet timestamps
> >>>>>> account for the difference between the start times of the two inputs.
> >>>>>>
> >>>>>> Typical use case is to sync two or more live inputs such as from capture
> >>>>>> devices. Both the target and reference input source timestamps should be
> >>>>>> based on the same clock source.
> >>>>>>
> >>>>>> If not all inputs have timestamps, the wallclock times at the time of
> >>>>>> reception of inputs shall be used. FFmpeg must have been compiled with
> >>>>>> thread support for this last case.
> >>>>> I'm wondering if simply using the other input's InputFile.ts_offset
> >>>>> wouldn't achieve the same effect with much less complexity.
> >>>> That's what I initially did. But since the code can also use two other
> >>>> sources for start times (start_time_realtime, first_pkt_wallclock),
> >>>> those intervals may not exactly match the difference between
> >>>> fmctx->start_times so I use a generic calculation.
> >>> In what cases is it better to use either of those two other sources?
> >>>
> >>> As per the commit message, the timestamps of both inputs are supposed to
> >>> come from the same clock. Then it seems to me that offsetting each of
> >>> those streams by different amounts would break synchronization rather
> >>> than improve it.
> >> The first preference, when available, stores the epoch time closest to
> >> time of capture. That would eliminate some jitter.
> >> The 2nd preference is the fmctx->start_time. The 3rd is the reception
> >> wallclock. It is a fallback. It will likely lead to the worst sync.
> > In which cases is this fallback useful? I would expect that all
> > transports you'd want to use this with will have working timestamps.
> 
> Agreed. I just kept it as a last resort scenario. Can do without it.

No more objections to this patch, with this fallback dropped.
Gyan Doshi July 14, 2022, 8:18 a.m. UTC | #17 
On 2022-07-14 01:45 pm, Anton Khirnov wrote:
> Quoting Gyan Doshi (2022-07-14 09:47:17)
>>
>> On 2022-07-14 12:16 pm, Anton Khirnov wrote:
>>> Quoting Gyan Doshi (2022-07-10 20:02:38)
>>>> On 2022-07-10 10:46 pm, Anton Khirnov wrote:
>>>>> Quoting Gyan Doshi (2022-07-08 05:56:21)
>>>>>> On 2022-07-07 03:11 pm, Anton Khirnov wrote:
>>>>>>> Quoting Gyan Doshi (2022-07-04 18:29:12)
>>>>>>>> This is a per-file input option that adjusts an input's timestamps
>>>>>>>> with reference to another input, so that emitted packet timestamps
>>>>>>>> account for the difference between the start times of the two inputs.
>>>>>>>>
>>>>>>>> Typical use case is to sync two or more live inputs such as from capture
>>>>>>>> devices. Both the target and reference input source timestamps should be
>>>>>>>> based on the same clock source.
>>>>>>>>
>>>>>>>> If not all inputs have timestamps, the wallclock times at the time of
>>>>>>>> reception of inputs shall be used. FFmpeg must have been compiled with
>>>>>>>> thread support for this last case.
>>>>>>> I'm wondering if simply using the other input's InputFile.ts_offset
>>>>>>> wouldn't achieve the same effect with much less complexity.
>>>>>> That's what I initially did. But since the code can also use two other
>>>>>> sources for start times (start_time_realtime, first_pkt_wallclock),
>>>>>> those intervals may not exactly match the difference between
>>>>>> fmctx->start_times so I use a generic calculation.
>>>>> In what cases is it better to use either of those two other sources?
>>>>>
>>>>> As per the commit message, the timestamps of both inputs are supposed to
>>>>> come from the same clock. Then it seems to me that offsetting each of
>>>>> those streams by different amounts would break synchronization rather
>>>>> than improve it.
>>>> The first preference, when available, stores the epoch time closest to
>>>> time of capture. That would eliminate some jitter.
>>>> The 2nd preference is the fmctx->start_time. The 3rd is the reception
>>>> wallclock. It is a fallback. It will likely lead to the worst sync.
>>> In which cases is this fallback useful? I would expect that all
>>> transports you'd want to use this with will have working timestamps.
>> Agreed. I just kept it as a last resort scenario. Can do without it.
> No more objections to this patch, with this fallback dropped.

Thanks. Since n5.1 hasn't been tagged just yet; will cherry-pick.

Regards,
Gyan
diff mbox series

Patch

diff --git a/doc/ffmpeg.texi b/doc/ffmpeg.texi
index 1a534ff1cc..1fe0a559c0 100644
--- a/doc/ffmpeg.texi
+++ b/doc/ffmpeg.texi
@@ -518,6 +518,22 @@  see @ref{time duration syntax,,the Time duration section in the ffmpeg-utils(1)
 Like the @code{-ss} option but relative to the "end of file". That is negative
 values are earlier in the file, 0 is at EOF.
 
+@item -isync @var{input_index} (@emph{input})
+Assign an input as a sync source.
+
+This will take the difference between the start times of the target and referenced inputs and
+offset the timestamps of the target file by that difference. The source timestamps of the two
+inputs should derive from the same clock source for expected results. If @code{copyts} is set
+then @code{start_at_zero} must also be set. If at least one of the inputs has no starting
+timestamp then the wallclock time at time of reception of the inputs is used as a best-effort
+sync basis.
+
+Acceptable values are those that refer to a valid ffmpeg input index. If the sync reference is
+the target index itself or @var{-1}, then no adjustment is made to target timestamps. A sync
+reference may not itself be synced to any other input.
+
+Default value is @var{-1}.
+
 @item -itsoffset @var{offset} (@emph{input})
 Set the input time offset.
 
diff --git a/fftools/ffmpeg.c b/fftools/ffmpeg.c
index e7384f052a..290b8228b8 100644
--- a/fftools/ffmpeg.c
+++ b/fftools/ffmpeg.c
@@ -3658,6 +3658,10 @@  static void *input_thread(void *arg)
             av_thread_message_queue_set_err_recv(f->in_thread_queue, ret);
             break;
         }
+
+        if (f->first_pkt_wallclock == AV_NOPTS_VALUE)
+            f->first_pkt_wallclock = av_gettime();
+
         queue_pkt = av_packet_alloc();
         if (!queue_pkt) {
             av_packet_unref(pkt);
diff --git a/fftools/ffmpeg.h b/fftools/ffmpeg.h
index 99d31c346e..760e94d1e5 100644
--- a/fftools/ffmpeg.h
+++ b/fftools/ffmpeg.h
@@ -118,6 +118,7 @@  typedef struct OptionsContext {
     float readrate;
     int accurate_seek;
     int thread_queue_size;
+    int input_sync_ref;
 
     SpecifierOpt *ts_scale;
     int        nb_ts_scale;
@@ -409,7 +410,9 @@  typedef struct InputFile {
     int64_t duration;     /* actual duration of the longest stream in a file
                              at the moment when looping happens */
     AVRational time_base; /* time base of the duration */
+    int64_t first_pkt_wallclock;
     int64_t input_ts_offset;
+    int input_sync_ref;
 
     int64_t ts_offset;
     int64_t last_ts;
diff --git a/fftools/ffmpeg_opt.c b/fftools/ffmpeg_opt.c
index e08455478f..b586f3e6b0 100644
--- a/fftools/ffmpeg_opt.c
+++ b/fftools/ffmpeg_opt.c
@@ -235,6 +235,7 @@  static void init_options(OptionsContext *o)
     o->chapters_input_file = INT_MAX;
     o->accurate_seek  = 1;
     o->thread_queue_size = -1;
+    o->input_sync_ref = -1;
 }
 
 static int show_hwaccels(void *optctx, const char *opt, const char *arg)
@@ -287,6 +288,67 @@  static int parse_and_set_vsync(const char *arg, int *vsync_var, int file_idx, in
     return 0;
 }
 
+static int apply_sync_offsets(void)
+{
+    for (int i = 0; i < nb_input_files; i++) {
+        InputFile *ref, *self = input_files[i];
+        int64_t adjustment;
+        int64_t self_start_time, ref_start_time, self_seek_start, ref_seek_start;
+        int sync_fpw = 0, start_times_set = 1;
+
+        if (self->input_sync_ref == -1 || self->input_sync_ref == i) continue;
+        if (self->input_sync_ref >= nb_input_files || self->input_sync_ref < -1) {
+            av_log(NULL, AV_LOG_FATAL, "-isync for input %d references non-existent input %d.\n", i, self->input_sync_ref);
+            exit_program(1);
+        }
+
+        if (copy_ts && !start_at_zero) {
+            av_log(NULL, AV_LOG_FATAL, "Use of -isync requires that start_at_zero be set if copyts is set.\n");
+            exit_program(1);
+        }
+
+        ref = input_files[self->input_sync_ref];
+        if (ref->input_sync_ref != -1 && ref->input_sync_ref != self->input_sync_ref) {
+            av_log(NULL, AV_LOG_ERROR, "-isync for input %d references a resynced input %d. Sync not set.\n", i, self->input_sync_ref);
+            continue;
+        }
+
+        if (self->ctx->start_time_realtime != AV_NOPTS_VALUE && ref->ctx->start_time_realtime != AV_NOPTS_VALUE) {
+            self_start_time = self->ctx->start_time_realtime;
+            ref_start_time  =  ref->ctx->start_time_realtime;
+        } else if (self->ctx->start_time != AV_NOPTS_VALUE && ref->ctx->start_time != AV_NOPTS_VALUE) {
+            self_start_time = self->ctx->start_time;
+            ref_start_time  =  ref->ctx->start_time;
+        } else {
+#if HAVE_THREADS
+            self_start_time = self->first_pkt_wallclock;
+            ref_start_time  =  ref->first_pkt_wallclock;
+            sync_fpw = 1;
+#else
+            start_times_set = 0;
+#endif
+        }
+
+        if (start_times_set) {
+            self_seek_start = self->start_time == AV_NOPTS_VALUE ? 0 : self->start_time;
+            ref_seek_start  =  ref->start_time == AV_NOPTS_VALUE ? 0 :  ref->start_time;
+
+            adjustment = (self_start_time - ref_start_time) + !copy_ts*(self_seek_start - ref_seek_start) + ref->input_ts_offset;
+
+            self->ts_offset += adjustment;
+
+            av_log(NULL, AV_LOG_INFO, "Adjusted ts offset for Input #%d by %"PRId64" us to sync with Input #%d", i, adjustment, self->input_sync_ref);
+            if (sync_fpw) av_log(NULL, AV_LOG_INFO, " using reception wallclock time. Sync may not be obtained");
+            av_log(NULL, AV_LOG_INFO, ".\n");
+        } else {
+            av_log(NULL, AV_LOG_INFO, "Unable to identify start times for Inputs #%d and %d both. No sync adjustment made.\n",
+                   i, self->input_sync_ref);
+       }
+    }
+
+    return 0;
+}
+
 static int opt_filter_threads(void *optctx, const char *opt, const char *arg)
 {
     av_free(filter_nbthreads);
@@ -1305,6 +1367,8 @@  static int open_input_file(OptionsContext *o, const char *filename)
     f->ist_index  = nb_input_streams - ic->nb_streams;
     f->start_time = o->start_time;
     f->recording_time = o->recording_time;
+    f->first_pkt_wallclock = AV_NOPTS_VALUE;
+    f->input_sync_ref = o->input_sync_ref;
     f->input_ts_offset = o->input_ts_offset;
     f->ts_offset  = o->input_ts_offset - (copy_ts ? (start_at_zero && ic->start_time != AV_NOPTS_VALUE ? ic->start_time : 0) : timestamp);
     f->nb_streams = ic->nb_streams;
@@ -3489,6 +3553,8 @@  int ffmpeg_parse_options(int argc, char **argv)
         goto fail;
     }
 
+    apply_sync_offsets();
+
     /* create the complex filtergraphs */
     ret = init_complex_filters();
     if (ret < 0) {
@@ -3603,6 +3669,9 @@  const OptionDef options[] = {
     { "accurate_seek",  OPT_BOOL | OPT_OFFSET | OPT_EXPERT |
                         OPT_INPUT,                                   { .off = OFFSET(accurate_seek) },
         "enable/disable accurate seeking with -ss" },
+    { "isync",          HAS_ARG | OPT_INT | OPT_OFFSET |
+                        OPT_EXPERT | OPT_INPUT,                      { .off = OFFSET(input_sync_ref) },
+        "Indicate the input index for sync reference", "sync ref" },
     { "itsoffset",      HAS_ARG | OPT_TIME | OPT_OFFSET |
                         OPT_EXPERT | OPT_INPUT,                      { .off = OFFSET(input_ts_offset) },
         "set the input ts offset", "time_off" },