ffmpeg.c's discontinuity handling breaks when one of the A/V streams creeps

[jeeb]

Summary of the bug:

The intra-stream discontinuity checker checking in ffmpeg.c seems to be breaking and causing more breakage than it fixes if the timestamps themselves are correct, but if there's a jump in one of the streams that is more than dts_delta_threshold while the other one "creeps" along with much smaller intervals between packets.

Sample: ​https://kuroko.fushizen.eu/samples/2018-09-14-10s_AV_desync.ts

What happens:

1. Audio does fine, goes forward in smaller steps.
2. Video is lost and then comes back.
3. Timestamps received from libavformat are OK. After the video timestamps jump, the DTS delta checking code within ffmpeg.c starts adjusting the timestamps.
4. ffmpeg.c intra stream discontinuity logic keeps adjusting the timestamps keeping the A/V desync all the way to the end.

How to reproduce:

1. Patch the discontinuity code to be more visible without requiring all of the debug logging.

   diff --git a/fftools/ffmpeg.c b/fftools/ffmpeg.c
   index 934dc71..a2a612d 100644
   --- a/fftools/ffmpeg.c
   +++ b/fftools/ffmpeg.c
   @@ -4465,8 +4465,11 @@ static int process_input(int file_index)
                    delta >  1LL*dts_delta_threshold*AV_TIME_BASE ||
                    pkt_dts + AV_TIME_BASE/10 < FFMAX(ist->pts, ist->dts)) {
                    ifile->ts_offset -= delta;
   -                av_log(NULL, AV_LOG_DEBUG,
   -                       "timestamp discontinuity %"PRId64", new offset= %"PRId64"\n",
   +                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,
   +                       av_get_media_type_string(ist->dec_ctx->codec_type),
                           delta, ifile->ts_offset);
                    pkt.dts -= av_rescale_q(delta, AV_TIME_BASE_Q, ist->st->time_base);
                    if (pkt.pts != AV_NOPTS_VALUE)
   
   

2. Run ffmpeg to decode the input

   % ffmpeg -v verbose -i 2018-09-14-10s_AV_desync.ts -f null -
   ...
   timestamp discontinuity for stream #0:1 (id=100, type=video): 10970111, new offset= 26067745
   timestamp discontinuity for stream #0:0 (id=200, type=audio): -10970111, new offset= 37037856
   timestamp discontinuity for stream #0:1 (id=100, type=video): 10970111, new offset= 26067745
   timestamp discontinuity for stream #0:0 (id=200, type=audio): -10970111, new offset= 37037856
   timestamp discontinuity for stream #0:1 (id=100, type=video): 10970111, new offset= 26067745
   timestamp discontinuity for stream #0:0 (id=200, type=audio): -10970111, new offset= 37037856
   ...
   

3. If you encode, you will receive an A/V desynch of about 10 seconds, matching the number that's being thrown around, I think? Tested with mp4 container at least.

How I worked around this:

I added another inter stream discontinuity check which checks audio streams in case of video, and vice versa - and only applied the intra-stream discontinuity handling if that was the case. I made this ticket because I am not sure if this is the correct way forward, as I did not have the time to check if the discontinuity fixing code is just broken.

I will send the patch to the mailing list if it is felt that that is the right way forward with this discontinuity handling part.

[jeeb]

And yes, one could mention that by making dts_delta_threshold larger you could work around this as well.

But that - unfortunately - really doesn't seem to do anything about the base issue of the discontinuity logic ending up in an eternal loop of going either 10 seconds up or 10 seconds down, while there only should be one discontinuity (or at most two, if both have to be adjusted once).

[Carl Eugen Hoyos]

For the given sample, this is a regression since 2d74dea84f3d2dea04c77a9c02b01efab4d03a3b

[jeeb]

Thank you for the bisect.

I think that might only end up being the "breaker" as that was the point at which libavformat got discontinuity handling by itself, and thus ffmpeg.c started receiving monotonically rising timestamps. The sample does also contain an MPEG-TS timestamp discontinuity at around that point as well, which would before that point be handled by ffmpeg.c instead of "what came before it".

In any case, this leads me to think how much of the ffmpeg.c discontinuity handling could we clean out? Definitely not all of it since the timestamps-start-at-zero logic is also under it I think, but some of it seems to attempt to do things which lavf is doing without possibly having all of the context (like the demuxer-specific discontinuity bits etc, which IIRC are private to lavf?).

I will try to see from any of the cases where I've used ffmpeg.c which parts of discontinuity handling were actually useful. And if those could actually have been fixed in lavf itself instead.