Re: [FFmpeg-devel] [PATCH] doc/ffmpeg: extend -dts_delta_threshold option description

[Stefano Sabatini <stefasab@gmail.com>]

[-- Attachment #1: Type: text/plain, Size: 2065 bytes --]

On date Thursday 2023-02-09 00:41:18 +0100, Michael Niedermayer wrote:
> On Mon, Feb 06, 2023 at 02:25:23AM +0100, Stefano Sabatini wrote:
[...]
> > Subject: [PATCH 2/2] ffmpeg: review -dts_delta_threshold and -dts_delta_error
> >  options
> > 
> > Review handling of -dts_delta_threshold and -dts_delta_error options,
> > specify them as floating point expressed in seconds.
> > 
> > Also, review and simplify logic. Adjust values for tests, since in
> > some cases the new values does not trigger the correction logic.
> > 
> > PR: https://patchwork.ffmpeg.org/project/ffmpeg/list/?series=8252
> > ---
> >  doc/ffmpeg.texi             | 36 ++++++++++++++++---
> >  fftools/ffmpeg.c            | 72 ++++++++++++++++++++-----------------
> >  fftools/ffmpeg.h            |  2 ++
> >  fftools/ffmpeg_demux.c      |  3 ++
> >  tests/fate/filter-audio.mak |  2 +-
> >  tests/fate/mpeg4.mak        |  2 +-
> >  6 files changed, 77 insertions(+), 40 deletions(-)
> 
> This seems to break a case with concat and vsync
> ./ffmpeg -y -i 'concat:///home/michael/videos/angels.mpg|/home/michael/videos/angels.mpg'  -vsync 0 -an file.avi
> 
> ...
> cpb: bitrate max/min/avg: 0/0/200000 buffer size: 0 vbv_delay: N/A
> [mpeg4 @ 0x55e051b8d4c0] Invalid pts (0) <= last (11)00.00 bitrate=N/A speed=   0x    
> [vost#0:0/mpeg4 @ 0x55e051b9d700] Error submitting video frame to the encoder
> Conversion failed!
>
> 
> Ill mail you the angels.mpg, i think its online somewhere but i cant
> find it

Fixed, now the code should be equivalent to the previous
implementation.

What happened in this case (and apparently in the other fate tests
failing), is that some sort of limit correction is applied:

detected dts:-0.041711 < dts_limit:0.358789
ts delta 0.5005 applied => ts_offset_discont:0.5005 dts:0.458789

preventing the invalid pts error.

The limit correction, hardcoded in the ffmpeg.c code, is completely
unrelated to the dts_delta_threshold value, no idea if it would make
sense to make this parametric (but at least now it should be a bit
more clear from the code/logs).

[-- Attachment #2: 0001-ffmpeg-review-dts_delta_threshold-and-dts_delta_erro.patch --]
[-- Type: text/x-diff, Size: 10917 bytes --]

From 8e23638cbcfcf1e47d4b7d582d4eb12a29ad4f57 Mon Sep 17 00:00:00 2001
From: Stefano Sabatini <stefasab@gmail.com>
Date: Fri, 1 Apr 2011 01:13:35 +0200
Subject: [PATCH] ffmpeg: review -dts_delta_threshold and -dts_delta_error
 options

Review handling of -dts_delta_threshold and -dts_delta_error options
in order to simplify a bit the logic and improve logging.

Also extend documentation.

PR: https://patchwork.ffmpeg.org/project/ffmpeg/list/?series=8252
---
 doc/ffmpeg.texi        | 36 +++++++++++++---
 fftools/ffmpeg.c       | 98 ++++++++++++++++++++++++++----------------
 fftools/ffmpeg.h       |  2 +
 fftools/ffmpeg_demux.c |  3 ++
 4 files changed, 96 insertions(+), 43 deletions(-)

diff --git a/doc/ffmpeg.texi b/doc/ffmpeg.texi
index d9d4b75567..63be7951e6 100644
--- a/doc/ffmpeg.texi
+++ b/doc/ffmpeg.texi
@@ -1837,12 +1837,38 @@ results, but increase memory use and latency.
 
 The default value is 10 seconds.
 
-@item -dts_delta_threshold
-Timestamp discontinuity delta threshold.
+@item -dts_delta_threshold @var{threshold}
+Timestamp discontinuity delta threshold, expressed as a floating point
+number of seconds.
+
+The timestamp discontinuity correction enabled by this option is only
+applied to input formats accepting timestamp discontinuity (for which
+the @code{AV_FMT_DISCONT} flag is enabled), e.g. MPEG-TS and HLS, and
+is automatically disabled when employing the @code{-copy_ts} option
+(unless wrapping is detected).
+
+If a timestamp discontinuity is detected whose absolute value is
+greater than @var{threshold}, ffmpeg will remove the discontinuity by
+decreasing/increasing the current DTS and PTS by the corresponding
+delta value.
+
+The default value is 10.
+
 @item -dts_error_threshold @var{seconds}
-Timestamp error delta threshold. This threshold use to discard crazy/damaged
-timestamps and the default is 30 hours which is arbitrarily picked and quite
-conservative.
+Timestamp error delta threshold, expressed as a floating point number
+of seconds.
+
+The timestamp correction enabled by this option is only applied to
+input formats not accepting timestamp discontinuity (for which the
+@code{AV_FMT_DISCONT} flag is not enabled).
+
+If a timestamp discontinuity is detected whose absolute value is
+greater than @var{threshold}, ffmpeg will drop the PTS/DTS timestamp
+value.
+
+The default value is @code{3600*30} (30 hours), which is arbitrarily
+picked and quite conservative.
+
 @item -muxdelay @var{seconds} (@emph{output})
 Set the maximum demux-decode delay.
 @item -muxpreload @var{seconds} (@emph{output})
diff --git a/fftools/ffmpeg.c b/fftools/ffmpeg.c
index 064ec6d4b3..e88a78872a 100644
--- a/fftools/ffmpeg.c
+++ b/fftools/ffmpeg.c
@@ -3671,62 +3671,84 @@ static void decode_flush(InputFile *ifile)
 static void ts_discontinuity_detect(InputFile *ifile, InputStream *ist,
                                     AVPacket *pkt)
 {
-    const int fmt_is_discont = ifile->ctx->iformat->flags & AVFMT_TS_DISCONT;
+    const int fmt_is_discont = !!(ifile->ctx->iformat->flags & AVFMT_TS_DISCONT);
     int disable_discontinuity_correction = copy_ts;
     int64_t pkt_dts = av_rescale_q_rnd(pkt->dts, ist->st->time_base, AV_TIME_BASE_Q,
                                        AV_ROUND_NEAR_INF | AV_ROUND_PASS_MINMAX);
 
+#define BASE_TS(ts_) av_ts2timestr(ts_, &AV_TIME_BASE_Q)
+#define PKT_TS(ts_) av_ts2timestr(ts_, &ist->st->time_base)
+
+    av_log(NULL, AV_LOG_TRACE,
+           "ts discontinuity detection on stream #%d:%d pkt_dts:%s pkt_pts:%s next_dts:%s last_ts:%s fmt_discont:%d copy_ts:%d\n",
+           ist->file_index, ist->st->index,
+           PKT_TS(pkt->dts), PKT_TS(pkt->pts),
+           BASE_TS(ist->next_dts), BASE_TS(ifile->last_ts),
+           fmt_is_discont, copy_ts);
+
     if (copy_ts && ist->next_dts != AV_NOPTS_VALUE &&
         fmt_is_discont && ist->st->pts_wrap_bits < 60) {
         int64_t wrap_dts = av_rescale_q_rnd(pkt->dts + (1LL<<ist->st->pts_wrap_bits),
                                             ist->st->time_base, AV_TIME_BASE_Q,
                                             AV_ROUND_NEAR_INF|AV_ROUND_PASS_MINMAX);
-        if (FFABS(wrap_dts - ist->next_dts) < FFABS(pkt_dts - ist->next_dts)/10)
+        if (FFABS(wrap_dts - ist->next_dts) < FFABS(pkt_dts - ist->next_dts)/10) {
             disable_discontinuity_correction = 0;
+            av_log(NULL, AV_LOG_DEBUG,
+                   "wrapped ts detected in copy_ts, enabled discontinuity correction\n");
+        }
     }
 
-    if (ist->next_dts != AV_NOPTS_VALUE && !disable_discontinuity_correction) {
-        int64_t delta = pkt_dts - ist->next_dts;
+#define CHECK_INVALID_TS(delta_, ts_type_, ts_)                         \
+    if (FFABS(delta_) > ist->dts_error_threshold) {                     \
+        av_log(NULL, AV_LOG_WARNING,                                    \
+               "drop invalid " #ts_type_ ":%s delta:%s > dts_error_threshold:%s\n", \
+               PKT_TS(ts_), BASE_TS(delta), BASE_TS(ist->dts_error_threshold)); \
+        ts_ = AV_NOPTS_VALUE;                                           \
+    }
+
+    if (!disable_discontinuity_correction) {
+        int64_t delta;
+
         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;
-                av_log(NULL, AV_LOG_DEBUG,
-                       "timestamp discontinuity for stream #%d:%d "
-                       "(id=%d, type=%s): %"PRId64", new 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);
-                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);
-            }
-        } else {
-            if (FFABS(delta) > 1LL * dts_error_threshold * AV_TIME_BASE) {
-                av_log(NULL, AV_LOG_WARNING, "DTS %"PRId64", next:%"PRId64" st:%d invalid dropping\n", pkt->dts, ist->next_dts, pkt->stream_index);
-                pkt->dts = AV_NOPTS_VALUE;
+            delta =
+                ist->next_dts != AV_NOPTS_VALUE ? pkt_dts - ist->next_dts :
+                ifile->last_ts != AV_NOPTS_VALUE ? pkt_dts - ifile->last_ts :
+                AV_NOPTS_VALUE;
+
+            if (delta != AV_NOPTS_VALUE) {
+                char fix_delta = 0;
+                int64_t dts_limit = FFMAX(ist->pts, ist->dts) - AV_TIME_BASE/10;
+
+                if (FFABS(delta) > ist->dts_delta_threshold) {
+                    av_log(NULL, AV_LOG_DEBUG,
+                           "detected dts:%s delta:%s > dts_delta_threshold:%s\n",
+                           BASE_TS(pkt_dts), BASE_TS(delta), BASE_TS(ist->dts_delta_threshold));
+                    fix_delta = 1;
+                } else if (ist->next_dts != AV_NOPTS_VALUE && pkt_dts < dts_limit) {
+                    av_log(NULL, AV_LOG_DEBUG, "detected dts:%s < dts_limit:%s\n",
+                           BASE_TS(pkt_dts), BASE_TS(dts_limit));
+                    fix_delta = 1;
+                }
+
+                if (fix_delta) {
+                    ifile->ts_offset_discont -= delta;
+                    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);
+                    av_log(NULL, AV_LOG_DEBUG,
+                           "ts delta %s applied => ts_offset_discont:%s dts:%s\n",
+                           BASE_TS(-delta), BASE_TS(ifile->ts_offset_discont), PKT_TS(pkt->dts));
+                }
             }
-            if (pkt->pts != AV_NOPTS_VALUE){
+        } else if (ist->next_dts != AV_NOPTS_VALUE) {
+            delta = pkt_dts - ist->next_dts;
+            CHECK_INVALID_TS(delta, dts, pkt->dts);
+            if (pkt->pts != AV_NOPTS_VALUE) {
                 int64_t pkt_pts = av_rescale_q(pkt->pts, ist->st->time_base, AV_TIME_BASE_Q);
                 delta = pkt_pts - ist->next_dts;
-                if (FFABS(delta) > 1LL * dts_error_threshold * AV_TIME_BASE) {
-                    av_log(NULL, AV_LOG_WARNING, "PTS %"PRId64", next:%"PRId64" invalid dropping st:%d\n", pkt->pts, ist->next_dts, pkt->stream_index);
-                    pkt->pts = AV_NOPTS_VALUE;
-                }
+                CHECK_INVALID_TS(delta, pts, pkt->pts);
             }
         }
-    } else if (ist->next_dts == AV_NOPTS_VALUE && !copy_ts &&
-               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;
-            av_log(NULL, AV_LOG_DEBUG,
-                   "Inter stream timestamp discontinuity %"PRId64", new offset= %"PRId64"\n",
-                   delta, ifile->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);
-        }
     }
 
     ifile->last_ts = av_rescale_q(pkt->dts, ist->st->time_base, AV_TIME_BASE_Q);
diff --git a/fftools/ffmpeg.h b/fftools/ffmpeg.h
index f1412f6446..686d38c934 100644
--- a/fftools/ffmpeg.h
+++ b/fftools/ffmpeg.h
@@ -368,6 +368,8 @@ 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;
+    int64_t       dts_delta_threshold; ///< dts_delta_threshold value rescaled to AV_TIME_BASE units
+    int64_t       dts_error_threshold; ///< dts_error_threshold value rescaled to AV_TIME_BASE units
 
     // the value of AVCodecParserContext.repeat_pict from the AVStream parser
     // for the last packet returned from ifile_get_packet()
diff --git a/fftools/ffmpeg_demux.c b/fftools/ffmpeg_demux.c
index 98da0678f5..18f8ed8946 100644
--- a/fftools/ffmpeg_demux.c
+++ b/fftools/ffmpeg_demux.c
@@ -736,6 +736,9 @@ static void add_input_streams(const OptionsContext *o, Demuxer *d)
         ist->filter_in_rescale_delta_last = AV_NOPTS_VALUE;
         ist->prev_pkt_pts = AV_NOPTS_VALUE;
 
+        ist->dts_delta_threshold = 1LL * dts_delta_threshold * AV_TIME_BASE;
+        ist->dts_error_threshold = 1LL * dts_error_threshold * AV_TIME_BASE;
+
         ist->dec_ctx = avcodec_alloc_context3(ist->dec);
         if (!ist->dec_ctx)
             report_and_exit(AVERROR(ENOMEM));
-- 
2.25.1


[-- Attachment #3: Type: text/plain, Size: 251 bytes --]

_______________________________________________
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".