[FFmpeg-devel,2/2] ffmpeg: bump the intra stream discontinuity message to warning

As libavformat should at this point be handling general input
timestamp discontinuities for us - such as with MPEG-TS - the
amount of messages from this case should be small, and if it
does start spamming messages, that would be a sign that either
the input, or the discontinuity handling code itself is broken.

In other words, printing this on the warning level makes more
sense than staying silent on most verbosity levels.
---
 fftools/ffmpeg.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
  

On Wed, Oct 10, 2018 at 02:10:26AM +0300, Jan Ekström wrote:
> As libavformat should at this point be handling general input
> timestamp discontinuities for us - such as with MPEG-TS - the
> amount of messages from this case should be small, and if it
> does start spamming messages, that would be a sign that either
> the input, or the discontinuity handling code itself is broken.

libavformat handles the case where a timestamp wraps around
that is goes beyond the maximum value. Thats not the only
type of discontinuity. Is there some code i am missing or that
i forgot about ?


[...]
  

On Thu, 11 Oct 2018, Michael Niedermayer wrote:

> On Wed, Oct 10, 2018 at 02:10:26AM +0300, Jan Ekström wrote:
>> As libavformat should at this point be handling general input
>> timestamp discontinuities for us - such as with MPEG-TS - the
>> amount of messages from this case should be small, and if it
>> does start spamming messages, that would be a sign that either
>> the input, or the discontinuity handling code itself is broken.
>
> libavformat handles the case where a timestamp wraps around
> that is goes beyond the maximum value. Thats not the only
> type of discontinuity. Is there some code i am missing or that
> i forgot about ?

FWIW libavformat only handles wraparound _once_.

Regards,
Marton
  

On Thu, Oct 11, 2018 at 10:50 PM Marton Balint <cus@passwd.hu> wrote:
> On Thu, 11 Oct 2018, Michael Niedermayer wrote:
>
> > On Wed, Oct 10, 2018 at 02:10:26AM +0300, Jan Ekström wrote:
> >> As libavformat should at this point be handling general input
> >> timestamp discontinuities for us - such as with MPEG-TS - the
> >> amount of messages from this case should be small, and if it
> >> does start spamming messages, that would be a sign that either
> >> the input, or the discontinuity handling code itself is broken.
> >
> > libavformat handles the case where a timestamp wraps around
> > that is goes beyond the maximum value. Thats not the only
> > type of discontinuity. Is there some code i am missing or that
> > i forgot about ?
>
> FWIW libavformat only handles wraparound _once_.
>
> Regards,
> Marton

Looking at update_wrap_reference and where it gets called from et al,
it would seem like it should take care of multiple as it keeps
updating the reference and applying wrap-around as required?

Generally the one in ffmpeg.c seemed to only get hit when there was
some sort of jump in the timestamps in addition to the wrap-arounds -
or if the lavf wrap-around logic decided to go follow a teletext track
with utterly broken timestamps as the reference.

As for the comment, yes - lavf's wrap-around logic doesn't try to poke
all sorts of discontinuities, but in my opinion the fact that "casual"
discontinuities should no longer happen at this stage definitely makes
logging that a discontinuity happened and that we tried to apply a new
offset to all streams worth mentioning on a more normal verbosity
level than "debug". Currently you will never, ever hear anything about
this logic activating unless your verbosity level is ridiculously
high. And as a general case, if the logic is working correctly this
code should only fire once per discontinuity, not in an eternal loop.

Best regards,
Jan
  

On Thu, 11 Oct 2018, Jan Ekström wrote:

> On Thu, Oct 11, 2018 at 10:50 PM Marton Balint <cus@passwd.hu> wrote:
>> On Thu, 11 Oct 2018, Michael Niedermayer wrote:
>>
>> > On Wed, Oct 10, 2018 at 02:10:26AM +0300, Jan Ekström wrote:
>> >> As libavformat should at this point be handling general input
>> >> timestamp discontinuities for us - such as with MPEG-TS - the
>> >> amount of messages from this case should be small, and if it
>> >> does start spamming messages, that would be a sign that either
>> >> the input, or the discontinuity handling code itself is broken.
>> >
>> > libavformat handles the case where a timestamp wraps around
>> > that is goes beyond the maximum value. Thats not the only
>> > type of discontinuity. Is there some code i am missing or that
>> > i forgot about ?
>>
>> FWIW libavformat only handles wraparound _once_.
>>
>> Regards,
>> Marton
>
> Looking at update_wrap_reference and where it gets called from et al,
> it would seem like it should take care of multiple as it keeps
> updating the reference and applying wrap-around as required?

Timestamps are updated in wrap_timestamp and that only adds or substracts 
the wrap amount once, so it can't be right.

>
> Generally the one in ffmpeg.c seemed to only get hit when there was
> some sort of jump in the timestamps in addition to the wrap-arounds -
> or if the lavf wrap-around logic decided to go follow a teletext track
> with utterly broken timestamps as the reference.
>
> As for the comment, yes - lavf's wrap-around logic doesn't try to poke
> all sorts of discontinuities, but in my opinion the fact that "casual"
> discontinuities should no longer happen at this stage definitely makes
> logging that a discontinuity happened and that we tried to apply a new
> offset to all streams worth mentioning on a more normal verbosity
> level than "debug". Currently you will never, ever hear anything about
> this logic activating unless your verbosity level is ridiculously
> high. And as a general case, if the logic is working correctly this
> code should only fire once per discontinuity, not in an eternal loop.

Agreed. In fact there is another similar message with DEBUG level, that 
should be bumped as well at least to INFO.

Regards,
Marton
  

On Thu, Oct 11, 2018 at 09:50:22PM +0200, Marton Balint wrote:
> 
> 
> On Thu, 11 Oct 2018, Michael Niedermayer wrote:
> 
> >On Wed, Oct 10, 2018 at 02:10:26AM +0300, Jan Ekström wrote:
> >>As libavformat should at this point be handling general input
> >>timestamp discontinuities for us - such as with MPEG-TS - the
> >>amount of messages from this case should be small, and if it
> >>does start spamming messages, that would be a sign that either
> >>the input, or the discontinuity handling code itself is broken.
> >
> >libavformat handles the case where a timestamp wraps around
> >that is goes beyond the maximum value. Thats not the only
> >type of discontinuity. Is there some code i am missing or that
> >i forgot about ?
> 
> FWIW libavformat only handles wraparound _once_.

I think its more restrictive than this. If for example ths duration
of a video is more then the full timestamp range (with just 1 wraparound)
this will likely not work completely


[...]