[FFmpeg-devel] [PATCH] Enhance ffmpeg.c:opt_default()

[Stefano Sabatini]

On date Tuesday 2008-05-13 02:16:11 +0200, Michael Niedermayer encoded:
> On Mon, May 12, 2008 at 11:53:48PM +0200, Stefano Sabatini wrote:
> > Hi all,
> > 
> > actually opt_default() will keep trying to set an option even if it
> > fails to set it in some context.
> > 
> > The correct (or a more correct behaviour) seems to simply give up when
> > the option is found the first time but its value is invalid for that
> > context, in this case the program immediatly exits.
> > 
> > Also the patch make opt_default() distinguishes if an option wasn't
> > found in all the checked contexts or if the option was found in some
> > context but its value was invalid.
> 
> Nice so the patch does nothing except making the code more complex?
> patch rejected

I think that this patch implements the correct behaviour, which is:

patch) exit immediately when you find in some context the option X,
  but its value is invalid, and tell explicitly that the value is
  invalid

as opposed to the current behaviour:

none) if a value is found in a context but it fails to set it, try
  again with the following context, finally if the value hasn't been
  set because it was invalid in one or more contexts or hasn't been
  found in any context then always say the option hasn't been found

To me the actual behaviour looks wrong and confusing for the
user.

I'm attaching the output of a test script I wrote to show more
explicitly the two behaviours.

Regards.
-- 
FFmpeg = Free and Frightening MultiPurpose EntanGlement
-------------- next part --------------
stefano at geppetto ~/s/ffmpeg> avopt-test.sh 

./ffmpeg -flags -mv4+obmc+qpel+foo-trell+none
[...]
Unable to parse option value "foo-trell+none": missing (
Unable to parse option value "foo-trell+none": missing (
Unable to parse option value "foo-trell+none": missing (
Unable to parse option value "foo-trell+none": missing (
./ffmpeg: unrecognized option '-flags'

./ffmpeg -flags -+obmc+bar+qpel+foo-trell+none
[...]
Unable to parse option value "+obmc+bar+qpel+foo-trell+none": missing (
Unable to parse option value "+obmc+bar+qpel+foo-trell+none": missing (
Unable to parse option value "+obmc+bar+qpel+foo-trell+none": missing (
Unable to parse option value "+obmc+bar+qpel+foo-trell+none": missing (
./ffmpeg: unrecognized option '-flags'

./ffmpeg -b foo+1
[...]
Unable to parse option value "foo+1": missing (
Unable to parse option value "foo+1": missing (
Unable to parse option value "foo+1": missing (
Must supply at least one output file

./ffmpeg -b 100*foo-1
[...]
Unable to parse option value "100*foo-1": missing (
Unable to parse option value "100*foo-1": missing (
Unable to parse option value "100*foo-1": missing (
Must supply at least one output file

./ffmpeg -bt -100
[...]
Value -100.000000 for parameter 'bt' out of range.
Value -100.000000 for parameter 'bt' out of range.
Value -100.000000 for parameter 'bt' out of range.
./ffmpeg: unrecognized option '-bt'

./ffmpeg -bt foo
[...]
Unable to parse option value "foo": missing (
Unable to parse option value "foo": missing (
Unable to parse option value "foo": missing (
./ffmpeg: unrecognized option '-bt'

./ffmpeg -bt 1000+foo
[...]
Unable to parse option value "foo": missing (
Unable to parse option value "foo": missing (
Unable to parse option value "foo": missing (
./ffmpeg: unrecognized option '-bt'
-------------- next part --------------
stefano at geppetto ~/s/ffmpeg> avopt-test.sh 

./ffmpeg -flags -mv4+obmc+qpel+foo-trell+none
[...]
Unable to parse option value "foo-trell+none": missing (
Invalid argument for option flags: -mv4+obmc+qpel+foo-trell+none

./ffmpeg -flags -+obmc+bar+qpel+foo-trell+none
[...]
Unable to parse option value "+obmc+bar+qpel+foo-trell+none": missing (
Invalid argument for option flags: -+obmc+bar+qpel+foo-trell+none

./ffmpeg -b foo+1
[...]
Unable to parse option value "foo+1": missing (
Invalid argument for option b: foo+1

./ffmpeg -b 100*foo-1
[...]
Unable to parse option value "100*foo-1": missing (
Invalid argument for option b: 100*foo-1

./ffmpeg -bt -100
[...]
Value -100.000000 for parameter 'bt' out of range.
Invalid argument for option bt: -100

./ffmpeg -bt foo
[...]
Unable to parse option value "foo": missing (
Invalid argument for option bt: foo

./ffmpeg -bt 1000+foo
[...]
Unable to parse option value "foo": missing (
Invalid argument for option bt: 1000+foo
-------------- next part --------------
A non-text attachment was scrubbed...
Name: enhance-opt-default-01.patch
Type: text/x-diff
Size: 2070 bytes
Desc: not available
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20080514/4db80738/attachment.patch>