[FFmpeg-devel] [PATCH][RFC - DO NOT MERGE] Revert "mov: Discard invalid CTTS."

[Derek Buitenhuis]

This reverts commit 4093220029a4d77f272c491e9299680480a08c00.

Signed-off-by: Derek Buitenhuis <derek.buitenhuis at gmail.com>
---
I have CC'd Michael here, as he is the original author here and the "soltuion" is nor clear.

To explain this RFC:

We (Vimeo) have started seeing an uptick in broken MP4 files (what creates them is under
investigation; we have reached out to some users in hopes that we can identify the broken
muxer and contact its authors). They are broken in a very specific way: The CTTS box starts
off normal, but then at some point, the duration field of the CTTS entries starts to increment
on each entry, and this continues until the end of the file, resulting in incorrect and insane
PTS/DTS differences (like PTS/DTS differing by 15 minutes / 10,000 frames). I have linked both
a text dump of the boxes and a trimmed moov box as an example in [1] and [2]. I can only provide
a full file (~12 GiB) privately.

Thes files *happen* to decode linearly just fine, of course, because FFmpeg doesn't care about the
CTTS info in that case, but if you seek, everything will explode, as you would expect.

So, anyway, in the meanwhile, I was writing a simple detection filter for these sorts of files,
and I noticed it only detected some of them, and this lead me to 1fb9efbda0149c3491529ea5fa92cfdcb8cebfaa.
What happens is that if the file is long enough, the incrementing CTTS entry durations eventually
trigger this, as far as I can tell, totally arbitrary check for 1<<28, and that results in the
entire CTTS being thrown away, and libavformat setting PTS==DTS for all packets, which both
makes detection impossible, and breaks seeking in a completely different way.

So the options are:
    * Revert 1fb9efbda0149c3491529ea5fa92cfdcb8cebfaa and detect them as I was already.
    * Change the check to something else or some other value, which would still be totally
      arbitrary - but maybe something like >16 frames. This would still leave seeking totally
      broken, of course, since it will still set PTS==DTS for all packets. This is still maybe
      detectable for API users by checking b_frames and has_b_frames of video_delay, but
      PTS==DTS? Seems kind gross, but... eh.
    * (Possibly in addition to other changes) Make the check an error, or make seeking return an
      error when this sort of file is detected.
    * Something I haven't listed here.

So, basically, all options suck, and I thought some people may have opinions on the least bad
option, here - specifically Michael, as the author of the original patch.

People: Please comment if you feel strongly about one solution or another.

[1] http://chromashift.org/ctts_sadness/broken_cut.mp4.xz
[2] http://chromashift.org/ctts_sadness/broken.boxes.txt.xz

---
 libavformat/mov.c | 7 -------
 1 file changed, 7 deletions(-)

diff --git a/libavformat/mov.c b/libavformat/mov.c
index 1c07cff6b5..256e7d376f 100644
--- a/libavformat/mov.c
+++ b/libavformat/mov.c
@@ -3079,13 +3079,6 @@ static int mov_read_ctts(MOVContext *c, AVIOContext *pb, MOVAtom atom)
         av_log(c->fc, AV_LOG_TRACE, "count=%d, duration=%d\n",
                 count, duration);
 
-        if (FFNABS(duration) < -(1<<28) && i+2<entries) {
-            av_log(c->fc, AV_LOG_WARNING, "CTTS invalid\n");
-            av_freep(&sc->ctts_data);
-            sc->ctts_count = 0;
-            return 0;
-        }
-
         if (i+2<entries)
             mov_update_dts_shift(sc, duration, c->fc);
     }
-- 
2.30.0