[FFmpeg-devel] [PATCH] Implement libavcodec/opt.[ch] generic options handling

[Michael Niedermayer]

On Thu, Feb 05, 2009 at 08:39:30PM +0100, Stefano Sabatini wrote:
> On date Tuesday 2009-02-03 01:58:49 +0100, Michael Niedermayer encoded:
> > On Thu, Jan 08, 2009 at 01:51:11AM +0100, Stefano Sabatini wrote:
> > > On date Monday 2009-01-05 19:52:58 +0100, Michael Niedermayer encoded:
> > > > On Mon, Jan 05, 2009 at 02:04:11PM +0100, Stefano Sabatini wrote:
> > > > > Hi all,
> > > > > 
> > > > > this is my first attempt to move the option handling code from the
> > > > > applications to the library, it also should make possible to remove
> > > > > the deprecated lavf AVFormatParameters.
> > > > > 
> > > > > Please comment.
> > > > > 
> > > > > Regards.
> > > > > -- 
> > > > > FFmpeg = Furious Faboulous Majestic Picky Encoding/decoding Gangster
> > > > 
> > > > > Index: ffmpeg/libavcodec/opt.c
> > > > > ===================================================================
> > > > > --- ffmpeg.orig/libavcodec/opt.c	2009-01-05 13:49:31.000000000 +0100
> > > > > +++ ffmpeg/libavcodec/opt.c	2009-01-05 13:51:55.000000000 +0100
> > > > > @@ -114,7 +114,7 @@
> > > > >          *o_out = o;
> > > > >      if(!o)
> > > > >          return AVERROR(ENOENT);
> > > > > -    if(!val || o->offset<=0)
> > > > > +    if(!val || (o->type != FF_OPT_TYPE_HANDLER && o->offset<=0))
> > > > >          return AVERROR(EINVAL);
> > > > >  
> > > > >      if(o->type == FF_OPT_TYPE_BINARY){
> > > > 
> > > > this patch needs a seperate thread and some justification why it would be
> > > > a good idea
> > > > [...]
> > > 
> > > Well, why would this be useful?
> > > 
> > > This would make possible for example to use the frame size/rate
> > > abbreviation using av_set_string3(), when it is currently possible
> > > only in the ff* tools while, having this facility in the library will
> > > avoid to duplicate the code.
> > > 
> > > It also may be used for moving other code (e.g.: opt_preset() and
> > > opt_target()) from applications to the library, reducing overall code
> > > duplication and providing an higher level API.
> > > 
> > > > and moving the AVOption related code from libav*/utils to libav*/options is
> > > > fine if that is really just moving and done with svn cp
> > > 
> > > Done in a separate thread.
> > 
> > what is the advantage of OPT_TYPE_HANDLER over adding 
> > OPT_TYPE_BLAH FOO and BAR ?
> > 
> > similarly what is the advantage of not handling all by OPT_TYPE_HANDLER
> > and not have any other types ?
> > 
> > supporting such a 2 layer system means more complexity than a single
> > layer whichever of the 2 that would be
> 
> The idea was to provide predefined "handlers" for the most used
> options, while providing to the applications the possibility to extend
> the option system when they need some very ad-hoc handler, that is an
> handler for some option which needs to be managed in a very peculiar
> way.
> 
> Pixel formats, frame rate/size abbreviations seem to belong to the
> latter category.
> 

> The alternative is to define a new handler every time we need to deal
> with a new kind of option, but I don't think this is a good idea since
> we would end-up with a very huge/messy opt.c.

why?
its the same code, just in functions instead of if/switch
if its more messy than what we have now then you are doing something
wrong. And in the same way your handlers likely are then sub optimal


> 
> The other alternative (all options handled by a
> already-defined/user-defined handler), looks more appealable but I
> still didn't considered how much hard it would be not to break
> backward compatibility implementing it, 

> so I ended up with this hybrid
> system.

anyway, iam against your hybrid hack

[...]
-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

Awnsering whenever a program halts or runs forever is
On a turing machine, in general impossible (turings halting problem).
On any real computer, always possible as a real computer has a finite number
of states N, and will either halt in less than N cycles or never halt.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 189 bytes
Desc: Digital signature
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20090205/55209b14/attachment.pgp>