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

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

[Jan Ekström]

From: Nongji Chen <nchen@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@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,
-- 
2.39.2

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

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

[Stefano Sabatini]

On date Friday 2023-03-24 16:19:27 +0200, Jan Ekström wrote:
> From: Nongji Chen <nchen@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@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.
_______________________________________________
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".

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

[James Almer]

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

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

[Jan Ekström]

On Sat, Mar 25, 2023 at 5:46 PM James Almer <jamrial@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@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@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.

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

Re: [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@gmail.com> wrote:
>
> On Sat, Mar 25, 2023 at 5:46 PM James Almer <jamrial@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@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@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
_______________________________________________
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".

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

[Anton Khirnov]

Quoting Jan Ekström (2023-04-03 11:05:24)
> 
> Ping. Are you OK with this explanation/reasoning, or are you still
> blocking this?

Just push IMO

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

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

[Jan Ekström]

On Wed, Apr 12, 2023 at 11:15 AM Anton Khirnov <anton@khirnov.net> wrote:
>
> Quoting Jan Ekström (2023-04-03 11:05:24)
> >
> > Ping. Are you OK with this explanation/reasoning, or are you still
> > blocking this?
>
> Just push IMO
>

Alright, applied as eb96cfbf57822f7ce0af508d4a9ecd2224cbd565 .

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