[MPlayer-dev-eng] Re: [TEST PATCH] How to get rid of all sscanf() or new face of config :)

Andriy N. Gritsenko andrej at lucky.net
Thu Mar 27 19:17:15 CET 2003


    Hi, Alban Bedel!

Sometime (on Thursday, March 27 at 15:12) I've received something...
>I don't like this patch. For me it's partially a step back, or at best i
>don't see any improvement. First of all, for the filters you just use a
>static struct for each filter type. Each filter must get his own parameters
>you can't use a common var for that.

Ok. You want set all parameters explicitly from command line? I don't
like it, I think I could set filters parameters in config file or just
use defaults if available. Also I think about GUI - why I have to set
each time each parameter? Any user want set it once and forget until it
will need to change.

About static struct - that static struct used only for configuration!
When filter inited it have own parameters! Just see the patch, please!

>Also the 'defaults' are there for something. If the user deosn't set a
>value for an option from where should you know the default value ?

Many filters have these defaults already, if there isn't then you will
set it and save in config file (if you don't set M_OPT_NOSAVE for that
option) and it will be default later. BTW, I plan to make a function
m_option_print(m_config_t*,FILE*) to save current config. :)

>For the help messages, imho it's not a so good idea. Dunno what other ppl
>think but i don't think that it's worth it considering the inpact on the
>cvs logs. If we manage to have a converter to convert the man page to a
>simplfied xml it is then very easy to add help messages (the asx parser can
>be used for that).

Ok, ok, ok... Are you sure if docs changed when each change in filters
comes? I'm not. I'm not sure if docs will be "up-to-date" forever.

Anyway, to be honest, that help improvement is side effect. My biggest
goal was to get rid of static structs and to simplify menu structure. As
I said earlier, I thought about video editor so I want to have all menu
from current configuration options, for each codec/filter/parameter. I've
started to think about that menu almost month ago and now I can get it,
with my patch. With your patch I cannot get it, sorry. (you said mine
have not advantages at all, eh?)

>As you know i allrdy implemented something similar wich is imho much
>simpler, so i can't see any reason to apply such thing.

Sorry, it's iyho. IMHO, mine is much simpler since it take a little lines
in code at all and new config scheme could be applied to filters with
less changes than by your way. May be I'm too lazy to change the code but
I don't think all authors prefer biggest changes. ;)  Also, I already
done big part of my patch when you sent yours. I said it before if you
remember it. And also, with your patch you have already 16 types of
options (mine has 14, with the same functionality), yours doesn't support
filters current syntax, doesn't allow get options tree to menu...

    Please, be less pathetic and more constuctive. I don't mind if you
correct me, point me where I could do it better, even if you will insist
on part of your changes which doesn't conflict with my patch. But if you
just say "I don't like it so it doesn't worth to try" then it's bad. :(

    With best wishes.
    Andriy.



More information about the MPlayer-dev-eng mailing list