diff mbox series

[FFmpeg-devel] ffmpeg: apply discontinuity adjustment per-stream

Message ID 20230103102217.1074-1-ffmpeg@gyani.pro
State New
Headers show
Series [FFmpeg-devel] ffmpeg: apply discontinuity adjustment per-stream | 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

Commit Message

Gyan Doshi Jan. 3, 2023, 10:22 a.m. UTC 
At present, the offset for discontinuity adjustment is applied per-file but
the check for discontinuity is intra-stream so the same discontinuity when
seen in multiple streams with copyts, leads to compounded adjustment of the
discontinuity offset. This introduces gaps in streams, leading to loss of sync
or even de facto loss of stream.

The ts_offset_discont parameter is transferred to InputStream and adjusted
based on intra-stream gaps.
---
Here's a simulated example to demonstrate the issue:
1) generate input
ffmpeg -f lavfi -i nullsrc=s=32x32:r=1,format=gray -f lavfi -i anullsrc=cl=mono:r=8000 -c:v libx264 -preset superfast -bf 0 -crf 50 -c:a aac -b:a 64k -muxpreload 0 -muxdelay 0 -output_ts_offset 95000 -t 100000 src-100000s.ts

2) demo
ffmpeg -copyts -i "src-100000.ts" -vf "showinfo=checksum=0" -af "ashowinfo" -f null - -report

==Before patch==

timestamp discontinuity for stream #0:1 (id=257, type=audio): -95443717689, new offset= 95443717689
timestamp discontinuity for stream #0:0 (id=256, type=video): -95443717689, new offset= 190887435378

[Parsed_ashowinfo_0 @ 000002700af98800] n:0 pts:759998976 pts_time:94999.9
[Parsed_ashowinfo_0 @ 000002700af98800] n:1 pts:760000000 pts_time:95000
 ... 
[Parsed_ashowinfo_0 @ 000002700af98800] n:745202 pts:1523085824 pts_time:190386
[Parsed_ashowinfo_0 @ 000002700af98800] n:745203 pts:1523086848 pts_time:190386
[Parsed_ashowinfo_0 @ 000002700af98800] n:745204 pts:2286637614 pts_time:285830
[Parsed_ashowinfo_0 @ 000002700af98800] n:745205 pts:2286638638 pts_time:285830


[Parsed_showinfo_0 @ 000002700ec34700] n:   0 pts:8550000000 pts_time:95000
[Parsed_showinfo_0 @ 000002700ec34700] n:   1 pts:8550090000 pts_time:95001
 ... 
[Parsed_showinfo_0 @ 000002700ec34700] n:95380 pts:17134200000 pts_time:190380
[Parsed_showinfo_0 @ 000002700ec34700] n:95381 pts:17134290000 pts_time:190381
[Parsed_showinfo_0 @ 000002700ec34700] n:95382 pts:25724314592 pts_time:285826
[Parsed_showinfo_0 @ 000002700ec34700] n:95383 pts:25724404592 pts_time:285827

==After patch==

timestamp discontinuity for stream #0:1 (id=257, type=audio): -95443717689, new stream offset= 95443717689
timestamp discontinuity for stream #0:0 (id=256, type=video): -95443717689, new stream offset= 95443717689

[Parsed_ashowinfo_0 @ 0000023c773c8880] n:0 pts:759998976 pts_time:94999.9
[Parsed_ashowinfo_0 @ 0000023c773c8880] n:1 pts:760000000 pts_time:95000
 ... 
[Parsed_ashowinfo_0 @ 0000023c773c8880] n:745202 pts:1523085824 pts_time:190386
[Parsed_ashowinfo_0 @ 0000023c773c8880] n:745203 pts:1523086848 pts_time:190386
[Parsed_ashowinfo_0 @ 0000023c773c8880] n:745204 pts:1523087872 pts_time:190386
[Parsed_ashowinfo_0 @ 0000023c773c8880] n:745205 pts:1523088896 pts_time:190386


[Parsed_showinfo_0 @ 0000023c795d07c0] n:   0 pts:8550000000 pts_time:95000
[Parsed_showinfo_0 @ 0000023c795d07c0] n:   1 pts:8550090000 pts_time:95001
 ... 
[Parsed_showinfo_0 @ 0000023c795d07c0] n:95380 pts:17134200000 pts_time:190380
[Parsed_showinfo_0 @ 0000023c795d07c0] n:95381 pts:17134290000 pts_time:190381
[Parsed_showinfo_0 @ 0000023c795d07c0] n:95382 pts:17134380000 pts_time:190382
[Parsed_showinfo_0 @ 0000023c795d07c0] n:95383 pts:17134470000 pts_time:190383



 fftools/ffmpeg.c | 12 ++++++------
 fftools/ffmpeg.h |  8 ++++----
 2 files changed, 10 insertions(+), 10 deletions(-)

Comments

Gyan Doshi Jan. 7, 2023, 4:20 a.m. UTC | #1 
On 2023-01-03 03:52 pm, Gyan Doshi wrote:
> At present, the offset for discontinuity adjustment is applied per-file but
> the check for discontinuity is intra-stream so the same discontinuity when
> seen in multiple streams with copyts, leads to compounded adjustment of the
> discontinuity offset. This introduces gaps in streams, leading to loss of sync
> or even de facto loss of stream.
>
> The ts_offset_discont parameter is transferred to InputStream and adjusted
> based on intra-stream gaps.

Comments?

> ---
> Here's a simulated example to demonstrate the issue:
> 1) generate input
> ffmpeg -f lavfi -i nullsrc=s=32x32:r=1,format=gray -f lavfi -i anullsrc=cl=mono:r=8000 -c:v libx264 -preset superfast -bf 0 -crf 50 -c:a aac -b:a 64k -muxpreload 0 -muxdelay 0 -output_ts_offset 95000 -t 100000 src-100000s.ts
>
> 2) demo
> ffmpeg -copyts -i "src-100000.ts" -vf "showinfo=checksum=0" -af "ashowinfo" -f null - -report
>
> ==Before patch==
>
> timestamp discontinuity for stream #0:1 (id=257, type=audio): -95443717689, new offset= 95443717689
> timestamp discontinuity for stream #0:0 (id=256, type=video): -95443717689, new offset= 190887435378
>
> [Parsed_ashowinfo_0 @ 000002700af98800] n:0 pts:759998976 pts_time:94999.9
> [Parsed_ashowinfo_0 @ 000002700af98800] n:1 pts:760000000 pts_time:95000
>   ...
> [Parsed_ashowinfo_0 @ 000002700af98800] n:745202 pts:1523085824 pts_time:190386
> [Parsed_ashowinfo_0 @ 000002700af98800] n:745203 pts:1523086848 pts_time:190386
> [Parsed_ashowinfo_0 @ 000002700af98800] n:745204 pts:2286637614 pts_time:285830
> [Parsed_ashowinfo_0 @ 000002700af98800] n:745205 pts:2286638638 pts_time:285830
>
>
> [Parsed_showinfo_0 @ 000002700ec34700] n:   0 pts:8550000000 pts_time:95000
> [Parsed_showinfo_0 @ 000002700ec34700] n:   1 pts:8550090000 pts_time:95001
>   ...
> [Parsed_showinfo_0 @ 000002700ec34700] n:95380 pts:17134200000 pts_time:190380
> [Parsed_showinfo_0 @ 000002700ec34700] n:95381 pts:17134290000 pts_time:190381
> [Parsed_showinfo_0 @ 000002700ec34700] n:95382 pts:25724314592 pts_time:285826
> [Parsed_showinfo_0 @ 000002700ec34700] n:95383 pts:25724404592 pts_time:285827
>
> ==After patch==
>
> timestamp discontinuity for stream #0:1 (id=257, type=audio): -95443717689, new stream offset= 95443717689
> timestamp discontinuity for stream #0:0 (id=256, type=video): -95443717689, new stream offset= 95443717689
>
> [Parsed_ashowinfo_0 @ 0000023c773c8880] n:0 pts:759998976 pts_time:94999.9
> [Parsed_ashowinfo_0 @ 0000023c773c8880] n:1 pts:760000000 pts_time:95000
>   ...
> [Parsed_ashowinfo_0 @ 0000023c773c8880] n:745202 pts:1523085824 pts_time:190386
> [Parsed_ashowinfo_0 @ 0000023c773c8880] n:745203 pts:1523086848 pts_time:190386
> [Parsed_ashowinfo_0 @ 0000023c773c8880] n:745204 pts:1523087872 pts_time:190386
> [Parsed_ashowinfo_0 @ 0000023c773c8880] n:745205 pts:1523088896 pts_time:190386
>
>
> [Parsed_showinfo_0 @ 0000023c795d07c0] n:   0 pts:8550000000 pts_time:95000
> [Parsed_showinfo_0 @ 0000023c795d07c0] n:   1 pts:8550090000 pts_time:95001
>   ...
> [Parsed_showinfo_0 @ 0000023c795d07c0] n:95380 pts:17134200000 pts_time:190380
> [Parsed_showinfo_0 @ 0000023c795d07c0] n:95381 pts:17134290000 pts_time:190381
> [Parsed_showinfo_0 @ 0000023c795d07c0] n:95382 pts:17134380000 pts_time:190382
> [Parsed_showinfo_0 @ 0000023c795d07c0] n:95383 pts:17134470000 pts_time:190383
>
>
>
>   fftools/ffmpeg.c | 12 ++++++------
>   fftools/ffmpeg.h |  8 ++++----
>   2 files changed, 10 insertions(+), 10 deletions(-)
>
> diff --git a/fftools/ffmpeg.c b/fftools/ffmpeg.c
> index 881d6f0af2..b967055059 100644
> --- a/fftools/ffmpeg.c
> +++ b/fftools/ffmpeg.c
> @@ -3418,13 +3418,13 @@ static void ts_discontinuity_detect(InputFile *ifile, InputStream *ist,
>           if (fmt_is_discont) {
>               if (FFABS(delta) > 1LL * dts_delta_threshold * AV_TIME_BASE ||
>                   pkt_dts + AV_TIME_BASE/10 < FFMAX(ist->pts, ist->dts)) {
> -                ifile->ts_offset_discont -= delta;
> +                ist->ts_offset_discont -= delta;
>                   av_log(NULL, AV_LOG_DEBUG,
>                          "timestamp discontinuity for stream #%d:%d "
> -                       "(id=%d, type=%s): %"PRId64", new offset= %"PRId64"\n",
> +                       "(id=%d, type=%s): %"PRId64", new stream offset= %"PRId64"\n",
>                          ist->file_index, ist->st->index, ist->st->id,
>                          av_get_media_type_string(ist->par->codec_type),
> -                       delta, ifile->ts_offset_discont);
> +                       delta, ist->ts_offset_discont);
>                   pkt->dts -= av_rescale_q(delta, AV_TIME_BASE_Q, ist->st->time_base);
>                   if (pkt->pts != AV_NOPTS_VALUE)
>                       pkt->pts -= av_rescale_q(delta, AV_TIME_BASE_Q, ist->st->time_base);
> @@ -3447,10 +3447,10 @@ static void ts_discontinuity_detect(InputFile *ifile, InputStream *ist,
>                  fmt_is_discont && ifile->last_ts != AV_NOPTS_VALUE) {
>           int64_t delta = pkt_dts - ifile->last_ts;
>           if (FFABS(delta) > 1LL * dts_delta_threshold * AV_TIME_BASE) {
> -            ifile->ts_offset_discont -= delta;
> +            ist->ts_offset_discont -= delta;
>               av_log(NULL, AV_LOG_DEBUG,
>                      "Inter stream timestamp discontinuity %"PRId64", new offset= %"PRId64"\n",
> -                   delta, ifile->ts_offset_discont);
> +                   delta, ist->ts_offset_discont);
>               pkt->dts -= av_rescale_q(delta, AV_TIME_BASE_Q, ist->st->time_base);
>               if (pkt->pts != AV_NOPTS_VALUE)
>                   pkt->pts -= av_rescale_q(delta, AV_TIME_BASE_Q, ist->st->time_base);
> @@ -3463,7 +3463,7 @@ static void ts_discontinuity_detect(InputFile *ifile, InputStream *ist,
>   static void ts_discontinuity_process(InputFile *ifile, InputStream *ist,
>                                        AVPacket *pkt)
>   {
> -    int64_t offset = av_rescale_q(ifile->ts_offset_discont, AV_TIME_BASE_Q,
> +    int64_t offset = av_rescale_q(ist->ts_offset_discont, AV_TIME_BASE_Q,
>                                     ist->st->time_base);
>   
>       // apply previously-detected timestamp-discontinuity offset
> diff --git a/fftools/ffmpeg.h b/fftools/ffmpeg.h
> index 5527dbe49b..d7bba8a190 100644
> --- a/fftools/ffmpeg.h
> +++ b/fftools/ffmpeg.h
> @@ -352,6 +352,10 @@ typedef struct InputStream {
>       int64_t       next_pts;  ///< synthetic pts for the next decode frame (in AV_TIME_BASE units)
>       int64_t       pts;       ///< current pts of the decoded frame  (in AV_TIME_BASE units)
>       int           wrap_correction_done;
> +    /**
> +     * Extra timestamp offset added by discontinuity handling.
> +     */
> +    int64_t ts_offset_discont;
>   
>       // the value of AVCodecParserContext.repeat_pict from the AVStream parser
>       // for the last packet returned from ifile_get_packet()
> @@ -444,10 +448,6 @@ typedef struct InputFile {
>        */
>       int64_t start_time_effective;
>       int64_t ts_offset;
> -    /**
> -     * Extra timestamp offset added by discontinuity handling.
> -     */
> -    int64_t ts_offset_discont;
>       int64_t last_ts;
>       int64_t start_time;   /* user-specified start time in AV_TIME_BASE or AV_NOPTS_VALUE */
>       int64_t recording_time;
Gyan Doshi Jan. 10, 2023, 3:49 a.m. UTC | #2 
On 2023-01-07 09:50 am, Gyan Doshi wrote:
>
>
> On 2023-01-03 03:52 pm, Gyan Doshi wrote:
>> At present, the offset for discontinuity adjustment is applied 
>> per-file but
>> the check for discontinuity is intra-stream so the same discontinuity 
>> when
>> seen in multiple streams with copyts, leads to compounded adjustment 
>> of the
>> discontinuity offset. This introduces gaps in streams, leading to 
>> loss of sync
>> or even de facto loss of stream.
>>
>> The ts_offset_discont parameter is transferred to InputStream and 
>> adjusted
>> based on intra-stream gaps.
>
> Comments?

Plan to push in 48h.

Regards,
Gyan

>
>> ---
>> Here's a simulated example to demonstrate the issue:
>> 1) generate input
>> ffmpeg -f lavfi -i nullsrc=s=32x32:r=1,format=gray -f lavfi -i 
>> anullsrc=cl=mono:r=8000 -c:v libx264 -preset superfast -bf 0 -crf 50 
>> -c:a aac -b:a 64k -muxpreload 0 -muxdelay 0 -output_ts_offset 95000 
>> -t 100000 src-100000s.ts
>>
>> 2) demo
>> ffmpeg -copyts -i "src-100000.ts" -vf "showinfo=checksum=0" -af 
>> "ashowinfo" -f null - -report
>>
>> ==Before patch==
>>
>> timestamp discontinuity for stream #0:1 (id=257, type=audio): 
>> -95443717689, new offset= 95443717689
>> timestamp discontinuity for stream #0:0 (id=256, type=video): 
>> -95443717689, new offset= 190887435378
>>
>> [Parsed_ashowinfo_0 @ 000002700af98800] n:0 pts:759998976 
>> pts_time:94999.9
>> [Parsed_ashowinfo_0 @ 000002700af98800] n:1 pts:760000000 pts_time:95000
>>   ...
>> [Parsed_ashowinfo_0 @ 000002700af98800] n:745202 pts:1523085824 
>> pts_time:190386
>> [Parsed_ashowinfo_0 @ 000002700af98800] n:745203 pts:1523086848 
>> pts_time:190386
>> [Parsed_ashowinfo_0 @ 000002700af98800] n:745204 pts:2286637614 
>> pts_time:285830
>> [Parsed_ashowinfo_0 @ 000002700af98800] n:745205 pts:2286638638 
>> pts_time:285830
>>
>>
>> [Parsed_showinfo_0 @ 000002700ec34700] n:   0 pts:8550000000 
>> pts_time:95000
>> [Parsed_showinfo_0 @ 000002700ec34700] n:   1 pts:8550090000 
>> pts_time:95001
>>   ...
>> [Parsed_showinfo_0 @ 000002700ec34700] n:95380 pts:17134200000 
>> pts_time:190380
>> [Parsed_showinfo_0 @ 000002700ec34700] n:95381 pts:17134290000 
>> pts_time:190381
>> [Parsed_showinfo_0 @ 000002700ec34700] n:95382 pts:25724314592 
>> pts_time:285826
>> [Parsed_showinfo_0 @ 000002700ec34700] n:95383 pts:25724404592 
>> pts_time:285827
>>
>> ==After patch==
>>
>> timestamp discontinuity for stream #0:1 (id=257, type=audio): 
>> -95443717689, new stream offset= 95443717689
>> timestamp discontinuity for stream #0:0 (id=256, type=video): 
>> -95443717689, new stream offset= 95443717689
>>
>> [Parsed_ashowinfo_0 @ 0000023c773c8880] n:0 pts:759998976 
>> pts_time:94999.9
>> [Parsed_ashowinfo_0 @ 0000023c773c8880] n:1 pts:760000000 pts_time:95000
>>   ...
>> [Parsed_ashowinfo_0 @ 0000023c773c8880] n:745202 pts:1523085824 
>> pts_time:190386
>> [Parsed_ashowinfo_0 @ 0000023c773c8880] n:745203 pts:1523086848 
>> pts_time:190386
>> [Parsed_ashowinfo_0 @ 0000023c773c8880] n:745204 pts:1523087872 
>> pts_time:190386
>> [Parsed_ashowinfo_0 @ 0000023c773c8880] n:745205 pts:1523088896 
>> pts_time:190386
>>
>>
>> [Parsed_showinfo_0 @ 0000023c795d07c0] n:   0 pts:8550000000 
>> pts_time:95000
>> [Parsed_showinfo_0 @ 0000023c795d07c0] n:   1 pts:8550090000 
>> pts_time:95001
>>   ...
>> [Parsed_showinfo_0 @ 0000023c795d07c0] n:95380 pts:17134200000 
>> pts_time:190380
>> [Parsed_showinfo_0 @ 0000023c795d07c0] n:95381 pts:17134290000 
>> pts_time:190381
>> [Parsed_showinfo_0 @ 0000023c795d07c0] n:95382 pts:17134380000 
>> pts_time:190382
>> [Parsed_showinfo_0 @ 0000023c795d07c0] n:95383 pts:17134470000 
>> pts_time:190383
>>
>>
>>
>>   fftools/ffmpeg.c | 12 ++++++------
>>   fftools/ffmpeg.h |  8 ++++----
>>   2 files changed, 10 insertions(+), 10 deletions(-)
>>
>> diff --git a/fftools/ffmpeg.c b/fftools/ffmpeg.c
>> index 881d6f0af2..b967055059 100644
>> --- a/fftools/ffmpeg.c
>> +++ b/fftools/ffmpeg.c
>> @@ -3418,13 +3418,13 @@ static void ts_discontinuity_detect(InputFile 
>> *ifile, InputStream *ist,
>>           if (fmt_is_discont) {
>>               if (FFABS(delta) > 1LL * dts_delta_threshold * 
>> AV_TIME_BASE ||
>>                   pkt_dts + AV_TIME_BASE/10 < FFMAX(ist->pts, 
>> ist->dts)) {
>> -                ifile->ts_offset_discont -= delta;
>> +                ist->ts_offset_discont -= delta;
>>                   av_log(NULL, AV_LOG_DEBUG,
>>                          "timestamp discontinuity for stream #%d:%d "
>> -                       "(id=%d, type=%s): %"PRId64", new offset= 
>> %"PRId64"\n",
>> +                       "(id=%d, type=%s): %"PRId64", new stream 
>> offset= %"PRId64"\n",
>>                          ist->file_index, ist->st->index, ist->st->id,
>> av_get_media_type_string(ist->par->codec_type),
>> -                       delta, ifile->ts_offset_discont);
>> +                       delta, ist->ts_offset_discont);
>>                   pkt->dts -= av_rescale_q(delta, AV_TIME_BASE_Q, 
>> ist->st->time_base);
>>                   if (pkt->pts != AV_NOPTS_VALUE)
>>                       pkt->pts -= av_rescale_q(delta, AV_TIME_BASE_Q, 
>> ist->st->time_base);
>> @@ -3447,10 +3447,10 @@ static void ts_discontinuity_detect(InputFile 
>> *ifile, InputStream *ist,
>>                  fmt_is_discont && ifile->last_ts != AV_NOPTS_VALUE) {
>>           int64_t delta = pkt_dts - ifile->last_ts;
>>           if (FFABS(delta) > 1LL * dts_delta_threshold * AV_TIME_BASE) {
>> -            ifile->ts_offset_discont -= delta;
>> +            ist->ts_offset_discont -= delta;
>>               av_log(NULL, AV_LOG_DEBUG,
>>                      "Inter stream timestamp discontinuity %"PRId64", 
>> new offset= %"PRId64"\n",
>> -                   delta, ifile->ts_offset_discont);
>> +                   delta, ist->ts_offset_discont);
>>               pkt->dts -= av_rescale_q(delta, AV_TIME_BASE_Q, 
>> ist->st->time_base);
>>               if (pkt->pts != AV_NOPTS_VALUE)
>>                   pkt->pts -= av_rescale_q(delta, AV_TIME_BASE_Q, 
>> ist->st->time_base);
>> @@ -3463,7 +3463,7 @@ static void ts_discontinuity_detect(InputFile 
>> *ifile, InputStream *ist,
>>   static void ts_discontinuity_process(InputFile *ifile, InputStream 
>> *ist,
>>                                        AVPacket *pkt)
>>   {
>> -    int64_t offset = av_rescale_q(ifile->ts_offset_discont, 
>> AV_TIME_BASE_Q,
>> +    int64_t offset = av_rescale_q(ist->ts_offset_discont, 
>> AV_TIME_BASE_Q,
>>                                     ist->st->time_base);
>>         // apply previously-detected timestamp-discontinuity offset
>> diff --git a/fftools/ffmpeg.h b/fftools/ffmpeg.h
>> index 5527dbe49b..d7bba8a190 100644
>> --- a/fftools/ffmpeg.h
>> +++ b/fftools/ffmpeg.h
>> @@ -352,6 +352,10 @@ typedef struct InputStream {
>>       int64_t       next_pts;  ///< synthetic pts for the next decode 
>> frame (in AV_TIME_BASE units)
>>       int64_t       pts;       ///< current pts of the decoded frame  
>> (in AV_TIME_BASE units)
>>       int           wrap_correction_done;
>> +    /**
>> +     * Extra timestamp offset added by discontinuity handling.
>> +     */
>> +    int64_t ts_offset_discont;
>>         // the value of AVCodecParserContext.repeat_pict from the 
>> AVStream parser
>>       // for the last packet returned from ifile_get_packet()
>> @@ -444,10 +448,6 @@ typedef struct InputFile {
>>        */
>>       int64_t start_time_effective;
>>       int64_t ts_offset;
>> -    /**
>> -     * Extra timestamp offset added by discontinuity handling.
>> -     */
>> -    int64_t ts_offset_discont;
>>       int64_t last_ts;
>>       int64_t start_time;   /* user-specified start time in 
>> AV_TIME_BASE or AV_NOPTS_VALUE */
>>       int64_t recording_time;
>
> _______________________________________________
> 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".
Anton Khirnov Jan. 10, 2023, 11:02 a.m. UTC | #3 
Quoting Gyan Doshi (2023-01-03 11:22:17)
> At present, the offset for discontinuity adjustment is applied per-file but
> the check for discontinuity is intra-stream so the same discontinuity when
> seen in multiple streams with copyts, leads to compounded adjustment of the
> discontinuity offset. This introduces gaps in streams, leading to loss of sync
> or even de facto loss of stream.
> 
> The ts_offset_discont parameter is transferred to InputStream and adjusted
> based on intra-stream gaps.

I never had much use for this feature, but I wonder if adding different
offsets to different streams isn't going against the whole point of
timestamps, which is synchronizing them.

Also, this could REALLY use tests.
Gyan Doshi Jan. 10, 2023, 4:02 p.m. UTC | #4 
On 2023-01-10 04:32 pm, Anton Khirnov wrote:
> Quoting Gyan Doshi (2023-01-03 11:22:17)
>> At present, the offset for discontinuity adjustment is applied per-file but
>> the check for discontinuity is intra-stream so the same discontinuity when
>> seen in multiple streams with copyts, leads to compounded adjustment of the
>> discontinuity offset. This introduces gaps in streams, leading to loss of sync
>> or even de facto loss of stream.
>>
>> The ts_offset_discont parameter is transferred to InputStream and adjusted
>> based on intra-stream gaps.
> I never had much use for this feature, but I wonder if adding different
> offsets to different streams isn't going against the whole point of
> timestamps, which is synchronizing them.

A robust method would have to look at the inter-stream delta among first 
packets in each stream after the jump,
which neither the existing code nor my changes do.

The existing code compares dts against next_dts for the stream where the 
jump is first seen, and derives the offset from that.
That offset adjustment applied to all other streams may not preserve the 
inter-stream deltas for the packets after the jump
since inter-frame intervals in each stream will be different. In 
practice, the discrepancy is going to be minor, except for pathological
inputs.

But this patch avoids the compounded adjustment currently being applied, 
which makes the output defunct, or worse frozen and
a CPU/memory hog if video encoding is CFR.

Regards,
Gyan
Michael Niedermayer Jan. 10, 2023, 8:10 p.m. UTC | #5 
On Tue, Jan 03, 2023 at 03:52:17PM +0530, Gyan Doshi wrote:
> At present, the offset for discontinuity adjustment is applied per-file but
> the check for discontinuity is intra-stream so the same discontinuity when
> seen in multiple streams with copyts, leads to compounded adjustment of the
> discontinuity offset. This introduces gaps in streams, leading to loss of sync
> or even de facto loss of stream.
> 
> The ts_offset_discont parameter is transferred to InputStream and adjusted
> based on intra-stream gaps.
> ---
> Here's a simulated example to demonstrate the issue:
> 1) generate input
> ffmpeg -f lavfi -i nullsrc=s=32x32:r=1,format=gray -f lavfi -i anullsrc=cl=mono:r=8000 -c:v libx264 -preset superfast -bf 0 -crf 50 -c:a aac -b:a 64k -muxpreload 0 -muxdelay 0 -output_ts_offset 95000 -t 100000 src-100000s.ts
> 
> 2) demo
> ffmpeg -copyts -i "src-100000.ts" -vf "showinfo=checksum=0" -af "ashowinfo" -f null - -report
> 
> ==Before patch==
> 
> timestamp discontinuity for stream #0:1 (id=257, type=audio): -95443717689, new offset= 95443717689
> timestamp discontinuity for stream #0:0 (id=256, type=video): -95443717689, new offset= 190887435378
> 
> [Parsed_ashowinfo_0 @ 000002700af98800] n:0 pts:759998976 pts_time:94999.9
> [Parsed_ashowinfo_0 @ 000002700af98800] n:1 pts:760000000 pts_time:95000
>  ... 
> [Parsed_ashowinfo_0 @ 000002700af98800] n:745202 pts:1523085824 pts_time:190386
> [Parsed_ashowinfo_0 @ 000002700af98800] n:745203 pts:1523086848 pts_time:190386
> [Parsed_ashowinfo_0 @ 000002700af98800] n:745204 pts:2286637614 pts_time:285830
> [Parsed_ashowinfo_0 @ 000002700af98800] n:745205 pts:2286638638 pts_time:285830
> 
> 
> [Parsed_showinfo_0 @ 000002700ec34700] n:   0 pts:8550000000 pts_time:95000
> [Parsed_showinfo_0 @ 000002700ec34700] n:   1 pts:8550090000 pts_time:95001
>  ... 
> [Parsed_showinfo_0 @ 000002700ec34700] n:95380 pts:17134200000 pts_time:190380
> [Parsed_showinfo_0 @ 000002700ec34700] n:95381 pts:17134290000 pts_time:190381
> [Parsed_showinfo_0 @ 000002700ec34700] n:95382 pts:25724314592 pts_time:285826
> [Parsed_showinfo_0 @ 000002700ec34700] n:95383 pts:25724404592 pts_time:285827
> 
> ==After patch==
> 
> timestamp discontinuity for stream #0:1 (id=257, type=audio): -95443717689, new stream offset= 95443717689
> timestamp discontinuity for stream #0:0 (id=256, type=video): -95443717689, new stream offset= 95443717689
> 
> [Parsed_ashowinfo_0 @ 0000023c773c8880] n:0 pts:759998976 pts_time:94999.9
> [Parsed_ashowinfo_0 @ 0000023c773c8880] n:1 pts:760000000 pts_time:95000
>  ... 
> [Parsed_ashowinfo_0 @ 0000023c773c8880] n:745202 pts:1523085824 pts_time:190386
> [Parsed_ashowinfo_0 @ 0000023c773c8880] n:745203 pts:1523086848 pts_time:190386
> [Parsed_ashowinfo_0 @ 0000023c773c8880] n:745204 pts:1523087872 pts_time:190386
> [Parsed_ashowinfo_0 @ 0000023c773c8880] n:745205 pts:1523088896 pts_time:190386
> 
> 
> [Parsed_showinfo_0 @ 0000023c795d07c0] n:   0 pts:8550000000 pts_time:95000
> [Parsed_showinfo_0 @ 0000023c795d07c0] n:   1 pts:8550090000 pts_time:95001
>  ... 
> [Parsed_showinfo_0 @ 0000023c795d07c0] n:95380 pts:17134200000 pts_time:190380
> [Parsed_showinfo_0 @ 0000023c795d07c0] n:95381 pts:17134290000 pts_time:190381
> [Parsed_showinfo_0 @ 0000023c795d07c0] n:95382 pts:17134380000 pts_time:190382
> [Parsed_showinfo_0 @ 0000023c795d07c0] n:95383 pts:17134470000 pts_time:190383
> 
> 
> 
>  fftools/ffmpeg.c | 12 ++++++------
>  fftools/ffmpeg.h |  8 ++++----
>  2 files changed, 10 insertions(+), 10 deletions(-)

I havent looked into this but i have a slightly ungood feeling about this
change

thx

[...]
Gyan Doshi Jan. 11, 2023, 6:59 a.m. UTC | #6 
On 2023-01-11 01:40 am, Michael Niedermayer wrote:
> On Tue, Jan 03, 2023 at 03:52:17PM +0530, Gyan Doshi wrote:
>> At present, the offset for discontinuity adjustment is applied per-file but
>> the check for discontinuity is intra-stream so the same discontinuity when
>> seen in multiple streams with copyts, leads to compounded adjustment of the
>> discontinuity offset. This introduces gaps in streams, leading to loss of sync
>> or even de facto loss of stream.
>>
>> The ts_offset_discont parameter is transferred to InputStream and adjusted
>> based on intra-stream gaps.
>> ---
>> Here's a simulated example to demonstrate the issue:
>> 1) generate input
>> ffmpeg -f lavfi -i nullsrc=s=32x32:r=1,format=gray -f lavfi -i anullsrc=cl=mono:r=8000 -c:v libx264 -preset superfast -bf 0 -crf 50 -c:a aac -b:a 64k -muxpreload 0 -muxdelay 0 -output_ts_offset 95000 -t 100000 src-100000s.ts
>>
>> 2) demo
>> ffmpeg -copyts -i "src-100000.ts" -vf "showinfo=checksum=0" -af "ashowinfo" -f null - -report
>>
>> ==Before patch==
>>
>> timestamp discontinuity for stream #0:1 (id=257, type=audio): -95443717689, new offset= 95443717689
>> timestamp discontinuity for stream #0:0 (id=256, type=video): -95443717689, new offset= 190887435378
>>
>> [Parsed_ashowinfo_0 @ 000002700af98800] n:0 pts:759998976 pts_time:94999.9
>> [Parsed_ashowinfo_0 @ 000002700af98800] n:1 pts:760000000 pts_time:95000
>>   ...
>> [Parsed_ashowinfo_0 @ 000002700af98800] n:745202 pts:1523085824 pts_time:190386
>> [Parsed_ashowinfo_0 @ 000002700af98800] n:745203 pts:1523086848 pts_time:190386
>> [Parsed_ashowinfo_0 @ 000002700af98800] n:745204 pts:2286637614 pts_time:285830
>> [Parsed_ashowinfo_0 @ 000002700af98800] n:745205 pts:2286638638 pts_time:285830
>>
>>
>> [Parsed_showinfo_0 @ 000002700ec34700] n:   0 pts:8550000000 pts_time:95000
>> [Parsed_showinfo_0 @ 000002700ec34700] n:   1 pts:8550090000 pts_time:95001
>>   ...
>> [Parsed_showinfo_0 @ 000002700ec34700] n:95380 pts:17134200000 pts_time:190380
>> [Parsed_showinfo_0 @ 000002700ec34700] n:95381 pts:17134290000 pts_time:190381
>> [Parsed_showinfo_0 @ 000002700ec34700] n:95382 pts:25724314592 pts_time:285826
>> [Parsed_showinfo_0 @ 000002700ec34700] n:95383 pts:25724404592 pts_time:285827
>>
>> ==After patch==
>>
>> timestamp discontinuity for stream #0:1 (id=257, type=audio): -95443717689, new stream offset= 95443717689
>> timestamp discontinuity for stream #0:0 (id=256, type=video): -95443717689, new stream offset= 95443717689
>>
>> [Parsed_ashowinfo_0 @ 0000023c773c8880] n:0 pts:759998976 pts_time:94999.9
>> [Parsed_ashowinfo_0 @ 0000023c773c8880] n:1 pts:760000000 pts_time:95000
>>   ...
>> [Parsed_ashowinfo_0 @ 0000023c773c8880] n:745202 pts:1523085824 pts_time:190386
>> [Parsed_ashowinfo_0 @ 0000023c773c8880] n:745203 pts:1523086848 pts_time:190386
>> [Parsed_ashowinfo_0 @ 0000023c773c8880] n:745204 pts:1523087872 pts_time:190386
>> [Parsed_ashowinfo_0 @ 0000023c773c8880] n:745205 pts:1523088896 pts_time:190386
>>
>>
>> [Parsed_showinfo_0 @ 0000023c795d07c0] n:   0 pts:8550000000 pts_time:95000
>> [Parsed_showinfo_0 @ 0000023c795d07c0] n:   1 pts:8550090000 pts_time:95001
>>   ...
>> [Parsed_showinfo_0 @ 0000023c795d07c0] n:95380 pts:17134200000 pts_time:190380
>> [Parsed_showinfo_0 @ 0000023c795d07c0] n:95381 pts:17134290000 pts_time:190381
>> [Parsed_showinfo_0 @ 0000023c795d07c0] n:95382 pts:17134380000 pts_time:190382
>> [Parsed_showinfo_0 @ 0000023c795d07c0] n:95383 pts:17134470000 pts_time:190383
>>
>>
>>
>>   fftools/ffmpeg.c | 12 ++++++------
>>   fftools/ffmpeg.h |  8 ++++----
>>   2 files changed, 10 insertions(+), 10 deletions(-)
> I havent looked into this but i have a slightly ungood feeling about this
> change
Yes, this is a hack. Ideal is to track and sync discontinuities at both 
file and stream level to avoid compound adjustments.
Let me rework this.

Regards,
Gyan
diff mbox series

Patch

diff --git a/fftools/ffmpeg.c b/fftools/ffmpeg.c
index 881d6f0af2..b967055059 100644
--- a/fftools/ffmpeg.c
+++ b/fftools/ffmpeg.c
@@ -3418,13 +3418,13 @@  static void ts_discontinuity_detect(InputFile *ifile, InputStream *ist,
         if (fmt_is_discont) {
             if (FFABS(delta) > 1LL * dts_delta_threshold * AV_TIME_BASE ||
                 pkt_dts + AV_TIME_BASE/10 < FFMAX(ist->pts, ist->dts)) {
-                ifile->ts_offset_discont -= delta;
+                ist->ts_offset_discont -= delta;
                 av_log(NULL, AV_LOG_DEBUG,
                        "timestamp discontinuity for stream #%d:%d "
-                       "(id=%d, type=%s): %"PRId64", new offset= %"PRId64"\n",
+                       "(id=%d, type=%s): %"PRId64", new stream offset= %"PRId64"\n",
                        ist->file_index, ist->st->index, ist->st->id,
                        av_get_media_type_string(ist->par->codec_type),
-                       delta, ifile->ts_offset_discont);
+                       delta, ist->ts_offset_discont);
                 pkt->dts -= av_rescale_q(delta, AV_TIME_BASE_Q, ist->st->time_base);
                 if (pkt->pts != AV_NOPTS_VALUE)
                     pkt->pts -= av_rescale_q(delta, AV_TIME_BASE_Q, ist->st->time_base);
@@ -3447,10 +3447,10 @@  static void ts_discontinuity_detect(InputFile *ifile, InputStream *ist,
                fmt_is_discont && ifile->last_ts != AV_NOPTS_VALUE) {
         int64_t delta = pkt_dts - ifile->last_ts;
         if (FFABS(delta) > 1LL * dts_delta_threshold * AV_TIME_BASE) {
-            ifile->ts_offset_discont -= delta;
+            ist->ts_offset_discont -= delta;
             av_log(NULL, AV_LOG_DEBUG,
                    "Inter stream timestamp discontinuity %"PRId64", new offset= %"PRId64"\n",
-                   delta, ifile->ts_offset_discont);
+                   delta, ist->ts_offset_discont);
             pkt->dts -= av_rescale_q(delta, AV_TIME_BASE_Q, ist->st->time_base);
             if (pkt->pts != AV_NOPTS_VALUE)
                 pkt->pts -= av_rescale_q(delta, AV_TIME_BASE_Q, ist->st->time_base);
@@ -3463,7 +3463,7 @@  static void ts_discontinuity_detect(InputFile *ifile, InputStream *ist,
 static void ts_discontinuity_process(InputFile *ifile, InputStream *ist,
                                      AVPacket *pkt)
 {
-    int64_t offset = av_rescale_q(ifile->ts_offset_discont, AV_TIME_BASE_Q,
+    int64_t offset = av_rescale_q(ist->ts_offset_discont, AV_TIME_BASE_Q,
                                   ist->st->time_base);
 
     // apply previously-detected timestamp-discontinuity offset
diff --git a/fftools/ffmpeg.h b/fftools/ffmpeg.h
index 5527dbe49b..d7bba8a190 100644
--- a/fftools/ffmpeg.h
+++ b/fftools/ffmpeg.h
@@ -352,6 +352,10 @@  typedef struct InputStream {
     int64_t       next_pts;  ///< synthetic pts for the next decode frame (in AV_TIME_BASE units)
     int64_t       pts;       ///< current pts of the decoded frame  (in AV_TIME_BASE units)
     int           wrap_correction_done;
+    /**
+     * Extra timestamp offset added by discontinuity handling.
+     */
+    int64_t ts_offset_discont;
 
     // the value of AVCodecParserContext.repeat_pict from the AVStream parser
     // for the last packet returned from ifile_get_packet()
@@ -444,10 +448,6 @@  typedef struct InputFile {
      */
     int64_t start_time_effective;
     int64_t ts_offset;
-    /**
-     * Extra timestamp offset added by discontinuity handling.
-     */
-    int64_t ts_offset_discont;
     int64_t last_ts;
     int64_t start_time;   /* user-specified start time in AV_TIME_BASE or AV_NOPTS_VALUE */
     int64_t recording_time;