[MPlayer-G2-dev] Re: g2 config - restart...

[Andriy N. Gritsenko]

    Hi, Alban Bedel!

Sometime (on Thursday, May 15 at 11:52) I've received something...
>As discussed in other mails we should keep 1 of those. But for the
>example you point out it's not really needed: 'help' can be a simple
>print type and 'include' could be internaly implemented by the
>config parser.

    I think 1 type is sufficient anyway, so "help" will just ignore extra
parameter (anyway it will exit(1) after execution and "include" will work
with standard parameter handle so CONF_TYPE_FUNC_FULL may be only type
for all FUNC*. :)

>> >> /*
>> >> all below this have
>> >>   ->priv as config_t*
>> >> */
>> >> // subconfig (dialog)	-		-	-	config_t*
>> >>   CONF_TYPE_SUBCONFIG,
>> >> // choose list		char**		-	length	config_t*
>> >>   CONF_TYPE_CHOOSE,
>> >> // select list		char***		-	-	config_t*
>> >>   CONF_TYPE_LIST,
>> >> // config group (sep.)	-		-	-	config_t*
>> >>   CONF_TYPE_DUMMY
>> >> };
>> >Could pls explain these a bit more.

>> CONF_TYPE_CHOOSE is string that could have only few fixed choises (in
>> Gui it will be combobox). For example - oac:lame:mode may have values
>> "1","2", "3", "4" or "stereo", "joint", "dual", "mono".
>Ok i think i get it. But then i don't get why you define the possible
>values with a config_t array.

    Because parameters of that may have different types. See example
below. Anyway if we have only string parameter but choises are just
flags then module have to parse that string by itself and set these
flags. In case of Gui it's even worse - Gui must allocate that string
then pass it to module so we have two conversions - from Gui internal
into string then from string into flag by module. Remember that Gui will
not use option->name for own (because that field is for commandline or
config parser) but use option->desc instead.

>I thought to something more general but as useal with generalized
>stuff it's overkill for the simple case. It would be some kind
>of list of anything with name attached to all possible values.
>->p would point to the target array to fill. And ->priv would be
>smthg like this :

>struct {
> int type; // Type of the elements in the array
> struct {
>  char* name; // Name of this value like "stereo"
>  union { // The value
>   int i;
>   float f;
>   size_t s;
>   void* p;
>  } val; // There i don't know wich is better union or simple void*
> } *values; // contain all possible value and their names
>};

You are going to add one more struct definition to layer 0? I think that
extra struct definition is unnecessary. Sorry but it may be done by all
we have already (I mean config_t) and that struct is overkill.

>As i said it's overkill for the simple case like string list. For
>string case we could reduce that to:
>->p : char**
>->priv : char**
>where priv contain all possible values.

    We cannot, unfortunately. For example, we have an option '-fstype'.
It's string list. It may have only few choises - "none","layer","above",
"below","fullscreen", and "stays_on_top". All except "layer" are flag
type options but layer must have integer parameter. Why we have parse it
twice - once by commandline parser then after that have another parser in
x11 module to do work that parser can do? We was about to avoid parsers
in the modules, eh? So why that must be exception? And remember that any
UI may have internal representation of parameters another that just
string (strings are good only for config and commandline) so why we will
force pass it unparsed? Anyway in that case we unable to do call open()
with prefilled structure (as we all prefer) but pass parameters as string
and force all modules do parsing itself. AFAIK, it's even impossible with
our new concept.

>> CONF_TYPE_LIST is string list but you will have subconfig for all
>> available choises - it will help you to have subconfigs for these and
>> let config parser check it itself. For example - "vo" will have list of
>> currently available video drivers and you can get help for it and write
>> in the commandline "-vo jpeg:quality=95" and get rid of option "-jpeg".
>Ok, but why is the target type char*** ?

    The same as for CONF_TYPE_STRING_LIST. But may be we have to create

typedef struct {
  char *item;
  char *param;
} config_list_item_t;

where if an item will have no params (after ':' or '=') then ->param
there will be NULL. Would you like it? Then target type of CONF_TYPE_LIST
will be **config_list_item_t. BTW, it will be a special prototype for
CONF_TYPE_LIST since CONF_TYPE_FUNC_FULL will have a special prototype
int (*cfg_func_t)(config_t *, char *, char *);

>Anyway for the example you point out such type is imho not the good way.
>For an option wich handle a list of modules instance the option should
>have in priv the array of info struct for the corrensponding module type.
>Then possible modules names and their options are retrived from the
>info struct array.

I think you are right and subconfig of that list has to have choises with
special meaning. But we have to distinguish they are modules so add
CONF_TYPE_MODULE type then parser will not parse suboptions for choises
but leave its for application, and application will retrieve info array
and call some parse_subconfig_params() function of parser (level 2)
before calling open().

>> >> // This subconfig has unnamed parameters list (i.e. 0.3:-1:-1 for
>> >> example)// It looks like a regular subconfig but different in parsing
>> >a> bit
>> >> #define CONF_SEQUENT		(1<<6)
>> >Dunno if that's really usefull as it's possible to have a parser
>> >how handle both mode (ie named and unamed suboptions) and even
>> >a mix of the 2.

>> What do you mean, sorry? I meant if we have that type of subconfig then
>> suboptions of that option don't have to have names - it will be checked
>> by parser and don't produce an error. Regular subconfigs have no rights
>> to have unnamed suboptions (for example, "-info Me:NewFilm" is nonsence
>> at least).

>These unamed list come from G1 vf where each filter had to parse his
>options himself. So Arpi used this simple way in the firsts filters
>and it stuck. These unamed list just imply that you know the order of
>the suboptions. -info Me:NewFilm make sense if i know that the first
>field is author and the second one title.

    You thinks about users too good. :)  Unfortunately, many of users are
too lazy to read documentation (at least) so they will step into that
trap very frequently. So I like to check that small flag and avoid that.
Anyway, it only means if we deny unnamed options then we don't set that
CONF_SEQUENT flag.

>See -vf in G1, you can write '-vf filter=100:width=500:45' where 100
>is the value for the first field, as width is the 12 field i name it
>to avoid entering the defaults of the first 11 fields and 45 is
>the value for the second field for wich i don't give the name bcs
>i'm lazy ;)
>This is what i mean with mixing named and unamed suboptions it's
>really possible and nice for lazy power users.

    So you still can do this anyway - flag CONF_SEQUENT (may be, rename
it to CONF_UNNAMED?) don't deny you have named parameters but contrary,
allow you have unnamed ones. :)  I hope, you understood me now. I don't
like to be forced to type names for all filter options but I don't like
to allow stupid user type "-info Me:NewFilm" also.

    With best wishes.
    Andriy.