[MPlayer-dev-eng] Re: stupid option handling.. again???

[Andriy N. Gritsenko]

    Hi, Alban Bedel!

    Sorry if my last letter offended you. I really want to get rid of all
misunderstaindigs and reach of consent. Let's talk about it and find good
solution that will be good for both sides, ok?

Sometime (on Thursday, April 10 at 13:24) I've received something...
>I really don't want to stop you from doing anything. Here is a copy of the
>mail you never received (I'm really sorry for forgeting to retry to send it).

[.......]
>I'm sorry if you take that against you but that's rather against your
>patch. I wrote the first version of the config system (so most of the code
>in cfgparser.[ch]). In the first system i wanted to let each part of
>mplayer register his options in order to minimize the number of global
>vars. In your patch you also use registering quiet a lot.

    I did that to let each filter register itself so it can be visible as
soon as it developed. No changes need at filters at all, exempt config
structure (but you did it too, isn't it?). Changes for new config scheme
are even less than yours.
    Ok, we can let filters be undefined and parsed on run. But then we
cannot reach and configure them from Gui. One way to do it is to describe
it in Gui explicitly. Another point of registering filters is to allow
set its parameters in config file. Now you can (I didn't chacked if even
that is possible, may be not) only describe full -vf (or -vop) line
there. It's bad for me, I don't like that way. You can solve both these
problems (and don't mentioned yet a problem with modularized filters that
may come in the near future) by registering from vf.c. What's wrong with
it?
    BTW, I wonder where is that registering stuff you mentioned above? I
couldn't find it, point me, please. May be it was in too old version of
mplayer/mencoder so i din't see it?

>Later it turned out to be a bad idea as by registering you introduce an
>uneeded dependency. It's just the same (and is better) to simply have a
>config variable wich is used by the main part of mplayer.

Ok. I think just as developer. My reasons:
1) when I add new filter/etc. then why I want to know every point where
  that filter is mentioned? I want just add the file and include it in
  main list. It's a little problem and that is why some filters are
  missed in documentation yet. Sorry, but it's inconvenient.
2) let's think if I want to modularize that stuff. In current project I
  cannot do it at all - I have all my code linked. If I do anything like
  a loader then I cannot get any configuration for any modules since it
  needs static links to main config.

>Another thing that i don't like is all the changes you did in m_config.c.
>Do be honest i don't really understand why you need these changes. My idea
>with the new system was to maximize the use of specialized options and
>minimize the change on the config code (wich is really the sensitive part).

    I've even simplified the config parser code, which is really the
sensitive part. Now it's shorter:

-rw-r--r--    1 andrej   staff        9174 Mar 15 20:00 m_config.c
-rw-r--r--    1 andrej   staff       14682 Apr  9 19:30 m_config.c.new
-rw-r--r--    1 andrej   staff       32663 Mar 15 22:49 m_option.c
-rw-r--r--    1 andrej   staff       24965 Apr 10 14:27 m_option.c.new

And it's even with a new function m_config_print_config() in m_config.c
to exclude any parsing m_config_t* outside of parser code! :)  Correct me
if I don't get right what you meant.

>Finnaly concerning the way you handle the vf options, i'm sorry to say that
>but you totaly missed the point. Each filter should get his OWN options,
>using statics vars wich are set just before the open call is no more than a
>hack. It work bcs mplayer is not a multithread program but it's not an
>acceptable solution imho.

    Thank you, I really missed something in it, now (for week now) I'm
working on it. I'll discuss it with you when it ready, ok? :)
    Anyway if you use multithreaded program something may be wrong if
someone will change config of running filter, isn't it? I think if we get
MPlayer/MEncoder multithreaded then we have to rethink it all. :)

>Now, i must say that your patch add some intersting things but imho not in
>the 'right way'. Concerning the 'possibility of integration of config into
>GUI' i don't think that it's so easy. I think we need some new kind of
>options type like key-value pair where the internal value can be of any
>type (so we can attach 'nice names' to any kind of options wich have
>'unfriendly' arguments). Otherwise we can also add some options flags as
>hints for the widget type to use (as the option kind alone won't be enouth
>for all iho). Later if it's really envolve on the gui side we migth needed
>more changes like a 'gui widget' field in the option to hold a real widget
>desc.

    As I said before, new extra "desc" field is for "nice names" for Gui,
also it give you a nice commandline help. About 'widget hints' above: Gui
already know which widget you have to use - see my previous letter how
field type has to be interpreted in Gui. :)  The main goal of my patch
was to give Gui all need hints.

>For the codecs configuration my plan is to use the same thing as for the vf.

    Agree.

>-vc, -ac are alredy lists so switching to vf like syntax will do no arm. The
>currents existings options will stay (but as global option) and set only
>the defaults. But first i'd like to finish the vf. Imho the vf should
>get there 'config struct' via the arg param, so my plan is to first
>make all filters ready (as there authors don't seems to be very likely
>to do it) and then change the argument type on the open call.

    Heh. I don't like to push all authors - since my way doesn't touch a
syntax (what I don't like at most in your current way, sorry, and I think
most of users and developers also hate changes of syntax), it's not need
to wait, all filters will use the same syntax now as before, even if you
will move parsing from sscanf() to config parser. :)

>If you think that's needed we can also add some options to setup the
>defaults values of the filters options (so you can put them in the
>config file). But it's perhaps better to add sufix support to the
>'object list' type. By this a mean supporting -vf-add f1,f2 and
>-vf-del 2 (index is needed as the same filter name may appear several
>times). It this way one can have a default filter chain in his config
>file and modify it when needed.

    If you don't like a war too then just wait a little, I'll send you
all my proposals as a new m_* files by private mail, ok? I'd like to get
an agreement at that all.
    My way is to let filters have the same syntax, let them save it in
config file and let config to be browsed by Gui. It's all without any
further complication of syntax, of course.

[.......]
    About -vf - I missed something in it too but it's resolved now, I'm
still working on it.

>I understand that it probaly take you a lot of time for this patch but i
>can't accept it. Also the parts in libvo, libao, etc where you register
>the options won't either be accepted by the maintainers of these parts
>of mplayer (bcs of the unneeded dependency it introduce).

    Let's ask the maintainers of these parts? :)  BTW, what is dependency
that you said about? You meant m_config* stuff? But you already changed
many in libmpcodecs, I don't think it's a problem with any other stuff.

    With best wishes.
    Andriy.