On Mon, Mar 08, 2021 at 03:55:19PM +0000, Derek Buitenhuis wrote: > This reverts commit 4093220029a4d77f272c491e9299680480a08c00. > > Signed-off-by: Derek Buitenhuis <derek.buitenh...@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.
I would try to detect the specific abberant muxer based on the input. Then have custom code in the demuxer which is based on reverse engnenering the issue to do a best effort to recover as much as is possible. And also print a big warning about the broken input / muxer so the user doesnt stay unaware. You can see this also in 2 ways 1. It sucks its not our job to workaround someone elses bugs 2. It cool, it gives FFmpegs demuxer a unique advantage over competing demuxers Implementing this will be more enjoyable to whoever does, if (s)he sees it as in "2." Thanks [...] -- Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB Rewriting code that is poorly written but fully understood is good. Rewriting code that one doesnt understand is a sign that one is less smart then the original author, trying to rewrite it will not make it better.
signature.asc
Description: PGP signature
_______________________________________________ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
