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

[Michael Niedermayer]

On Fri, Oct 16, 2009 at 01:06:31AM +0200, Stefano Sabatini wrote:
> On date Thursday 2009-10-15 16:36:54 +0200, Michael Niedermayer encoded:
> > On Tue, Oct 13, 2009 at 11:38:37PM +0200, Stefano Sabatini wrote:
> [...]
> > Ive already in the past rejected this hybrid system. You try again with the
> > very same system ...
> 
> +hybryd-rewrite -> rejected
> -hybrid+rewrite -> rejected
> 
> You rejected both approaches, now I don't know if there is another
> available -hybrid-rewrite approach, which allows all the features I
> would like.
> 
> > Its poor design, first the whole need of having user supplied parsing
> > functions is somewhat unclear to me. I mean the whole is for patches i
> > rejected and IIRC that was not the only nor primary reason for their
> > rejection but i might misremember ...
> > 
> > AVOptions have a variable called type, it specifies which type the
> > underlaying field is (float/string/color/flags/...)
> > this variable selects which parser / set_string code is used
> > Currently all these parsers are inside opt.c, they could be moved
> > into opt_parser.c or we could even allow an user app or a libav*
> > to register its own parsers for some types. 
> 
> One of the things I object to the current system is that the types are
> statically defined, so I wonder how would be possible to register new
> types at run-time.

I would prefer if all types where listed in opt.h


> 
> > But completely bypassing the existing framework, and to make that
> > not blow up rewrite it all using a new AVOption2 filled with
> > callbacks, is simply just a mess.
> > 
> > > 
> > > Show me how to do it without to extend opt.c and I'll be happy, just
> > > telling that such a feature isn't worth the trouble is not an option
> > > (pun intended).
> > 
> > see above but i still have my doubts about it being a good idea
> > The only thing that becomes possible with it is putting the code in a
> > different libraray or into the user app, i dont see the point of that
> > currently
> 
> Again consider my example, the parsing code for a color is currently
> implemented in lavfi, in order to implement an OPT_TYPE_COLOR you
> would have to:
> 
> 1) re-implement in lavc/lavu/wherever-the-opt-system-is the parsing
> code
> 
> 2) move the color parsing code to wherever-the-opt-system-is.
> 
> which are both rather poor choices.

please read what i wrote
ill re-explain it just in case
what you suggest is to rewrite the code (all your suggestions included
a rewrite of the struct -> AVOption2 or whatever, there was no hybrid without
rewrite) and the only thing you add is function pointers bloating the tables
up and creating a huge mess bypassing the existing architecture (yeah thats
why you cannot do it without a rewrite) and it also always breaks ABI&API

What i suggest is that you register the set_string() for your type like
a codec or demuxer is registered, its still not needed but it does everything
you claim your system can and the existing can not
you could have your code scattered in whichever lib and register it from there,
that said, i dont see for what we would want to use this. We dont scatter
codecs or pixel formats in 3 libs either.
changing default_val to char* makes sense, what is with that now btw???
are you going to work on that or not? this has no relation to the rest


> 
> Also an application (e.g. ffmpeg) may want to use the option system
> already implemented for e.g. parsing the CLI options, rather than
> implement a new one, for example ffmpeg may have an FFmpegContext
> where to set all the parsed options.

ffmpeg could do that already if we wanted


> 
> And in general you need for each library a different set of options,
> which are only useful in that library, I did the example of
> colors/notes which would be useful only in lavfi, for example a
> size/framerate option may be useful in lavf/lavc.

colors are usefull for lets say subtitle fonts
size/framerate is usefull for libavcodec and filters

do you have another use case of this set_string() scattering?
Iam not against it (with type in opt.h and clean registering) assuming
there is a actual case where we want to use it

[...]

-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

The greatest way to live with honor in this world is to be what we pretend
to be. -- Socrates
-------------- 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/20091016/3e1bc3a5/attachment.pgp>