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

[Robert Swain]

2009/2/27 Reimar D?ffinger <Reimar.Doeffinger at stud.uni-karlsruhe.de>:
> On Fri, Feb 27, 2009 at 12:36:27PM +0000, Robert Swain wrote:
>> 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.
>
> Well, the warning still would not work right, as the example
> -vcodec blub -vpre hq -vcodec libx264
> above shows, that's why I didn't consider it particularly useful.

Maybe the non-explicit codec idea wasn't so good after all. Maybe we
should suggest people use -vpre libx264-hq?

Regards,
Rob