[FFmpeg-devel] [PATCH] fix possible crash if vpre before vcodec

[Robert Swain]

2009/2/27 Michael Niedermayer <michaelni at gmx.at>:
> On Fri, Feb 27, 2009 at 11:25:26AM +0100, Reimar D?ffinger wrote:
>> I recently tried the following command
>> ffmpeg -i baldursgate-logo.mve -vpre hq -vcodec libx264 -b 500k -an out.mp4
>> and wondered why it would not work.
>> strace showed the following:
>> open("/home/reimar/.ffmpeg/hq.ffpreset", O_RDONLY) = -1 ENOENT (No such file or directory)
>> open("/home/reimar/.ffmpeg/(null)-hq.ffpreset", O_RDONLY) = -1 ENOENT (No such file or directory)
>> open("/usr/local/share/ffmpeg/hq.ffpreset", O_RDONLY) = -1 ENOENT (No such file or directory)
>> open("/usr/local/share/ffmpeg/(null)-hq.ffpreset", O_RDONLY) = -1 ENOENT (No such file or directory)
>> obviously ffmpeg tries to open the file already during option parsing (this could probably be considered
>> the real bug). When -vcodec is set only afterwards it passes NULL to
>> snprintf %s argument which may (and will) crash with some
>> implementations.
>> I propose a patch that at least avoids the crash, but e.g. this:
>> -vcodec blub -vpre hq -vcodec libx264
>> will still have the counter-intuitive behaviour of reading
>> blub-hq.ffpreset but encoding with libx264.
>
> would it not make more sense to warn the user that the codec is not set
> before -*pre instead of silently skiping searching for the file?

I agree. I prefer command lines to error out on options that don't
work as intended than to continue on. But a warning at least is
absolutely necessary.

Also, it should be documented that the preset options should come
after the codec name.

Regards,
Rob