[FFmpeg-devel] [PATCH] ffmpeg: make timestamp discontinuity logging a warning

[Jan Ekström]

On Sat, Mar 25, 2023 at 6:11 PM Jan Ekström <jeebjp at gmail.com> wrote:
>
> On Sat, Mar 25, 2023 at 5:46 PM James Almer <jamrial at gmail.com> wrote:
> >
> > On 3/25/2023 12:43 PM, Stefano Sabatini wrote:
> > > On date Friday 2023-03-24 16:19:27 +0200, Jan Ekström wrote:
> > >> From: Nongji Chen <nchen at aminocom.com>
> > >>
> > >> In most cases this should only occur once or so per stream in an
> > >> input, and in case the logic ends up in an eternal loop, it should
> > >> be visible to the end user instead of being completely invisible.
> > >>
> > >> Signed-off-by: Jan Ekström <jan.ekstrom at 24i.com>
> > >> ---
> > >>   fftools/ffmpeg.c | 2 +-
> > >>   1 file changed, 1 insertion(+), 1 deletion(-)
> > >>
> > >> diff --git a/fftools/ffmpeg.c b/fftools/ffmpeg.c
> > >> index 438bee8fef..1428988f9d 100644
> > >> --- a/fftools/ffmpeg.c
> > >> +++ b/fftools/ffmpeg.c
> > >> @@ -3696,7 +3696,7 @@ static void ts_discontinuity_detect(InputFile *ifile, InputStream *ist,
> > >>               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,
> > >> +                av_log(NULL, AV_LOG_WARNING,
> > >>                          "timestamp discontinuity for stream #%d:%d "
> > >>                          "(id=%d, type=%s): %"PRId64", new offset= %"PRId64"\n",
> > >>                          ist->file_index, ist->st->index, ist->st->id,
> > >
> > > LGTM.
> >
> > Wont this be a bit noisy? Maybe it could be VERBOSE instead?
> > If it was DEBUG until now, then it was not deemed that important to
> > become a log message shown by default when it was added.
>
> In my experience this will log for each stream having a different
> offset until all streams have gone past the discontinuity point.
>
> If it spams, it means that something has gone badly wrong (usually one
> stream gone to either past or future). And in my opinion that should
> be shown instead of hiding it by default.

Ping. Are you OK with this explanation/reasoning, or are you still
blocking this?

Jan