[FFmpeg-devel] [PATCH] ffmdec: sanitize codec parameters

Carl Eugen Hoyos ceffmpeg at gmail.com
Tue Nov 22 02:35:48 EET 2016

2016-11-22 0:06 GMT+01:00 Andreas Cadhalpun <andreas.cadhalpun at googlemail.com>:
> On 20.11.2016 21:02, Carl Eugen Hoyos wrote:
>> 2016-11-20 20:40 GMT+01:00 Andreas Cadhalpun <andreas.cadhalpun at googlemail.com>:
>>> Currently many demuxers silently accept wrong (i.e. negative) values
>>> for channels, bit_rate, block_align and so on. I'd like to fix that,
>>> so the question is now, how?
>>> There are a few possibilities:
>>> a) error out for negative values and also for zero
>> Only if -fstrict strict was set.
>>> b) error out for negative values, silently accept zero
>> Only if -fstrict strict was set.
> So you have no preference between accepting zero or not?

By default, FFmpeg should not error out because of an
invalid value in some field, neither if it is zero although
it should be positive nor when it is negative.

> I tend to silently accept zero, because I'd like a
> consistent solution and in some cases rejecting zero
> causes FATE failures.

If there were a bug, you should of course fix fate, fate
alone is generally no argument in favour or against a patch.

> If a zero causes SIGFPE crashes, it obviously needs to
> be rejected with an error.

The crash has to be fixed.

If a simple fix is possible that allows to decode such samples,
it should be preferred over a fix that doesn't allow it.

> I also like the idea to only error out if strict is set
> and otherwise only warn.
>>> c) warn for negative values and also for zero
>>> d) warn for negative values, silently accept zero
>> I obviously cannot stop you if you feel this should
>> be done, but note that users will report regressions
>> "warnings are shown for playable files".
> Isn't that a good thing?

I tried to explain above why it is not necessarily a good thing.

> Because either our demuxer has a bug and should be fixed,
> or some other tool created broken files, and if ffmpeg
> informs it's users about that, they can try to get that
> tool fixed.

(Träum weiter)
Seriously: We have an uncountable amount of files that
do not respect some "specification", many of them apparently
by intention, many written by applications that do not exist
since a long time, and some written by applications that have
fixed the issue meanwhile: Of course it can be a (very) good
thing to print warnings but in general, we should try to fix a
few of the thousands of known bugs in FFmpeg and not
spend infinite time on possible issues in other applications.

Sorry if I just misunderstand you.

Carl Eugen

More information about the ffmpeg-devel mailing list