[FFmpeg-devel] [PATCH] Avoid suppressing parse errors in matroska

[Reimar Döffinger]

On Thu, Feb 24, 2011 at 03:04:26PM -0800, Ami Fischman wrote:
> > Your patch seems likely to make the demuxer handle broken
> > files worse.
> 
> Can you say more about this?  What scenario do you see becoming worse?

You stop parsing all elements of a cluster as soon as there is an error
somewhere. It might well be possible that the rest can be parsed just fine.

> Also, can you tell me how parse errors are meant to be handled before my patch?

FFmpeg error handling sucks, there's only the option of failing completely
(which often enough is really not reasonable) or given no indication of
error at all, at most a error message.
The primary guiding rule in either way is though: get as much possibly valid data
out of the file as possible.
Fail if there is nothing useful you can do with the data you get out of the file.
Print error/warning messages if something is wrong (and this really should be
complemented with extra error flags - though even more for decoders so that applications
can decide when and at what kind of curruption they do or do not want to display a frame.

> > Also I cannot see anything that would cause an endless loop
> > (though admittedly I think it might progress through a file
> > very slowly).
> 
> The loop behavior just showed the ffplay time counter increasing
> (quickly), but it didn't stop even though the file in question is
> quite small.  What behavior do you see with "ffplay arith.webm" at
> head?

That is not an endless loop, and in particular ffmpeg handles the file fine.
It might be that there's a bug in ffplay or just that a timestamp is ridiculously
high or something else, but any of these are an application's job to decide
what to do with, not for libavformat to discard the data just to protect applications
"from themselves". I am sure that libavformat can be more helpful to these applications,
but just failing is not a soution to playback issues.