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

[Stefano Sabatini]

On date Friday 2009-10-16 03:01:11 +0200, Michael Niedermayer encoded:
> 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

Pinging this discussion as it was mentioned in another thread.

Michael's solution with the current layout may sound something like
this:

* move libavcodec/opt.[hc] to lavcore
* keep hardcoded option types, and eventually extend them (e.g. adding
  framesizes, framerates, colors, notes, whatever) without breaking
  API

Note than I'm not volunteering for this, as I predict that I won't
have much time to commit to FFmpeg in the next few months.

Regards.
-- 
FFmpeg = Faithless and Fundamental Most Portable Extreme Governor