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

Alban Bedel albeu at free.fr
Thu Apr 10 12:23:24 CEST 2003


Hi Andriy N. Gritsenko,

on Thu, 10 Apr 2003 12:39:54 +0300 you wrote:

>     Hi, Alban Bedel!
> 
> Sometime (on Thursday, April 10 at 12:16) I've received something...
> >>    You really ignored my patch? It has (and especially developed for
> >>that) ways to do that. It's sad, since I have no CVS access and you
> >have>monopoly in config parser code then. Sorry.
> >I wrote it, so i think i can decide if a patch for this code should
> >be applied or not.
> 
>     OK. Then reimplement, please, all I've reached with my patch, with
> your own way. I really need that and current config parser doesn't allow
> me to do it. I've sent a patch to have some functionality and to have
> less limitations, you've refused it. Good. Do it yourself but I insist,
> I need it since it doesn't allow me work on some project about MEncoder.
>     I'll wait. If you would like to stop me write anything for MEncoder
> (despite of I already done something, see file AUTHORS) then I'll ask to
> community to allow me improve MEncoder... :)
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).

Hi Andriy N. Gritsenko,

on Sat, 29 Mar 2003 18:07:09 +0200 you wrote:

>     Hi again!
> 
>     Sorry for private mail. Since you asked me don't mail private I've
> sent my patch to maillist. But it seems you have taken offence on me. I
> have to say, I didn't want go against you, I just did it in that time
> when you sent your patch and my patch seems be incompatible with yours a
> little. I'm sorry for that but it cannot be helped. If you want, please,
> let's resolve it. I want to have our Mplayer/Mencoder the best.

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.

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.

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).

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.

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.

For the codecs configuration my plan is to use the same thing as for the vf.
-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.
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.
Then when filters will be ready, we can change the vc, ac, af, etc. So
step by step as it's minimize the chance of introducing too much bugs.

Also you bitch on the syntax wich i can't understand as the -vf syntax is
100% backward compatible with the -vop syntax (beside it's going in the
'right' direction but that's planned since the idea of -vf months
(1 year ?) ago). As for the help, -vf have some auto doc stuff. Just try
-vf help and -vf scale=help. Ok there is no descriptions, but i don't
really like the idea of having that in the code. If the docs ppl agree to
review/maintain these messages, why not ? But i doubt it. Also it have to
be translated, and so on. Imho the 'right' solution would be to automaticly
extract these informations from the docs, but it's a bit harder.
<dream>
Perhaps that a perl expert could write us a little script to translate
the man page in something easy to parse (the best would be asx/xml like
as we alredy have a parser for that).
</dream>

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).
	Albeu



More information about the MPlayer-dev-eng mailing list