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

[Reimar Döffinger]

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.