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

[Michael Niedermayer]

On Thu, Mar 11, 2021 at 11:50:03AM +0000, Derek Buitenhuis wrote:
> On 11/03/2021 08:36, Michael Niedermayer wrote:
> > These are not enough to unambigously reverse engeneer the bug in the muxer
> > is it true for every output of the muxer, does it always happen at the
> > same position ?
> > is the runaway delta always 8 ?
> > does it always coincide with the 2nd entry of stts ?
> > is it a coincidence that the first and 2nd stts entries differ by 8 too ?
> 
> I think you and I have very different opinions on how this shuld be fixed. The files
> are broken.
> 
> Nothing is consisent between hese files, and it is not a good use of anyones time
> to try and reverse engineer an unknown muxer we don't have access to based on ses
> of files, so that we can remove or change a a hack from years ago. What you are
> asking for is ridiculous.
> 
> Even if some files showd a pattern, coding that into a demuxer is *wrong* and fragile
> at best. It just leads to more confusion and more hacks down the road when some files
> are broken in slightly different ways. It's adding fragile hacks on top of fragile hacks,
> to work around a bug that an unknown muxer had once.
> 
> Frankly, I regret asking for input if this is the result.
> 
> 
> > I was trying to describe how I would go fixing this if i was working on it.
> > And thus how someone else could fix it as well.
> 
> This is not useful - I can do that till the cows come home,
> 
> > What you seem to want is the result to a snippet of your "homework". Yes that would be
> > easier but if i knew what exactly to do then i would already have had to test
> > it and so would already have an implementation of it and would need to have
> > all samples and much more time. 
> 
> This is personally insulting for a few reasons:
> 
> 1. You are implying the digging I've already done is not good enough or that I'm
>    lazy for not followng your extremely vague 'well you could do this' email
>    based on nto even checking anything. I did the digging that is possible with
>    what is available to me, and to what I think is a good thechnical solution,
>    and I've recieved no feedback on those except 'maybe do some more digging and
>    add some hacks'. This is demotivating and insulting, and this crap is part of
>    the reason I've distanced myself from this community.
> 2. 'all samples' do not exist. It is infuriating to imply I should reverse engineer
>    some unknown muxer, with a limited set of samples, and add a hack for them, in
>    order to justify a previous entirely arbitrary hack added for a single file.
> 
> To state out this outright: The commit this revers is *wrong* and *terrible* and
> should never have been pushed in the first place. I was trying to be ammicable here
> engage in discussion on how best to remove or change it, but all I got was the
> 'Bring Me A Rock, Bring Me A Different Rock' scenario. Your barrier to add the hack
> was 0, my barrier to remove it is high.
> 
> > yes but that code is unrelated, with or without it the ctts are trash for this file
> > this needs code prior to that test that fixes the ctts and then this would not
> > trigger anymore as the ctts would be corrected
> 
> It does not *fix* anything. Even on the old file. It removes the ctts, but it does not *fix*
> it. Making up timestamps where PTS==DTS for all timestas is not *fixing* anything.
> 
> What you actually mean to say is 'it made ffmpeg.c decode linearly', which is something enirely
> differnet. It can't even seek. It only works linearly because of the parser. The DTS are entirely
> broken in your 'fix'.
> > the API user should receive valid timestamps and not need to handle that.
> > it cannot be expected that every API user carries around workarounds for random
> > muxer bugs. That would be really alot of duplicated code
> 
> This is a HARD disagree from me. A demuxe should not be 'fixing' timstamps. This is
> an applciation level problem.
> 
> But even ignoring that, a demuxer should provide *consistent* timestamps for simila files,
> not different based on some arbitrary number hack added in for a single file.
> 
> > I cant give an exact solution as it requires testing that solution against as many
> > samples as possible (requires both the samples and the time to do the work)
> > Maybe something like subtracting i*8 from entry 2+ works maybe the stts entry 1-2
> > time should be used instead of 8, maybe we need to take the first - last ctts to
> > get the slope to correct it.
> > then we could compare the normally read CTTS vs. the "corrected" CTTS and check which
> > is "flatter" as in maybe sum of abs diff from stts. with a strong bias toward the
> > normal. 
> > This needs to be implemented to know if it even works, its not unlikely that this
> > would require adjustments to work well, i cannot in 15min just from a text dump
> > tell how to best detect and correct this abberation. Maybe theres also some
> > metadata in the file that could be used to limit to which files to attempt to
> > apply this ... (i didnt see any but again i didnt look very hard)
> > The first goal must of course be not to break any correct files which such
> > compensation.
> 
> This is insane. Totally insane. I have no other words for this. 
> 
> Given that others on the list (Anton, James, etc.) are probably avoiding responding to this
> crazy thread, I am asking for someone to come in an moderate this formally, be it by choice
> in responding or by he recently approved technical committee.
> 
> We cannot continue with this ridiculousness.

You explicitly asked me to comment, and also stated that all options you listed suck:

"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 just wanted to help and so
I described a solution that will likely allow demuxing these files without
errors. Its more work and you would workaround the muxer bug in the demuxer.
Which is, lets call it unpopular.
You are opposed to that ? then just pretend i never replied.
What i wrote was there to help not to restrict or object to anything.

The fix for Ticket 385 is not nice and if that would be done better iam
all for it. But i dont think reverting that will magically fix these other
broken files. (Thats not an objection!) It just leaves it to the user
application to deal with the muxer bugs.


> 
> I have CC'd j-b.

Iam not sure jb is interrested in this but ill leave him in CC

Thanks

[...]
-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

Those who would give up essential Liberty, to purchase a little
temporary Safety, deserve neither Liberty nor Safety -- Benjamin Franklin
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 195 bytes
Desc: not available
URL: <https://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20210311/94e18bfc/attachment.sig>