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

[Michael Niedermayer]

On Wed, Mar 10, 2021 at 05:49:45PM +0000, Derek Buitenhuis wrote:
> On 10/03/2021 17:24, Michael Niedermayer wrote:
> > what does the muxer exactly do ?
> 
> I provided an explanation of what is happening during the broken muxing 
> in my original email, as well as a sample, and a text dump of the boxes.
> Please see those.

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 ?


> 
> Can you please actually fully read my emails and samples before going on
> long tangents about theortical things or what could be?
> 
> > Let me give an example (totally made up but to explain my point)
> > if each ctts value for example has its file position added to it
> > that can be reverted and the original ctts fully recovered.
> > (also a user app cannot do this, only the demuxer could as the user app
> >  has no knowledge of the atoms file position)
> > 
> > Does it store uninintialized memory then it would not be revertable but
> > it did not sound like thats whats happening.
> 
> This example is neither useful nor relevant to the case at hand. I described
> what is happening, and provided a sample: mid-way through the CTTS box, the entry
> durations start incrementing, i.e. growing, rather than matching the reordering.
> Please see the sample I provided, and also the dump of the boxes as text I provided.
> 
> These theortical and vague replies are not actionable. They're bordering on philosophical
> musings.

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.
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. 


> 
> > about never modifying data, for that a system similar to workaround_bugs
> > we have on the decoder side could be added. But its not really the users
> > job to do this. The demuxer should already return as much recoverable
> > data as possible.
> 
> But it's not. It's returning NO data rather than what is in the file. It
> is completely dropping all timestamps and making them up, based on nothing.
> There is *not* better. It's not fixing or salvaging anything. It's just making
> it broken in a different way. And it is not even doing it consistently. It is
> doing it only when an arbitrary number is reached, 

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


> so as an API user, I can't
> even consistently handle these.

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


> 
> I gave *exact* options to choose from in my original mail, as a well as a sample
> and a description of the problem - I am asking which should be chosen, or to
> suggest a different one in clear, actionable terms.

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.

Thanks

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

Any man who breaks a law that conscience tells him is unjust and willingly 
accepts the penalty by staying in jail in order to arouse the conscience of 
the community on the injustice of the law is at that moment expressing the 
very highest respect for law. - Martin Luther King Jr
-------------- 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/55491952/attachment.sig>