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

[Andriy N. Gritsenko]

    Hi, Alban Bedel!

Sometime (on Wednesday, May 14 at 22:15) I've received something...
>>     BTW, did you wrote new config_t and module_info_t structures (I mean
>> API layer 0) yet? If you didn't then there is a draft here (from yours
>> API description, of course - you really fine generalized all). :)  Also
>> I typed here some #define because they have values of fields of
>> config_t. I hope I did all correctly now. :)  BTW, I'm not sure if
>> filename must be in module_info_t - plugin subsystem can have it in
>> internal arrays. That subsystem anyway has to keep bad filenames list to
>> avoid trying its (in cache also, I think) further. Bad filenames - if
>> plugins are in plugin's subdirectory but are not plugins or have
>> incompatible version.

>I like the cache idea but imho the it should use more than the filename
>to identify a file. Smthg like filename + date (a hash would be better
>but that's probaly overkill) so we could detect if the file changed
>since last run.

    I wrote already (in one of previous letters) - in cache for identify
each cache entry must have a path, modification time and size of file. :)
But that is implementation issue. What I said above is that I wonder if
we really need that info in module_info_t (as it was proposed by Arpi).

>> ------------------------- begin of cfg.h
>> #ifndef __CFG_H
>> #define __CFG_H 1
>> 
>> typedef struct {
>>   char *name;
>>   void *p; /* variable pointer, type specific */
>>   int type;
>>   unsigned int flags;
>>   float min, max; /* range */
>>   void *priv; /* additional pointer, type specific */
>>   char *desc; /* short description for help and gui */
>> } config_t;

>I liked the idea you had of putting some hint for GUI widget type.
>It would avoid having different options type wich are in fact the
>same C type and it would also make things nicer for GUI writers.
>Also type specific flags would be nice. Ok there is priv but
>if a type only need a few flags priv is overkill. We could either
>add another flag field or use the lower bit for common flags and
>the higher for type specific ones.

I think second way is better.

>> // string as list - must be obsoleted to CONF_TYPE_LIST
>>   CONF_TYPE_STRING_LIST,
>Obsolated stuff in a draft ??????

I've added that only because there are still options of that type. But I
think you are right and we must remove it as soon we apply new API. :)

>> // function		cfg_func*	-	-	revert-cfg_func*
>>   CONF_TYPE_FUNC,
>>   CONF_TYPE_FUNC_PARAM,
>>   CONF_TYPE_FUNC_FULL,
>I think we should avoid callback based stuff. These were nice in the
>old days for very specific stuff. But now the only place in G1 where
>they are really needed is for the -aa* options of aalib. But as in G2
>each module instance have it's own context it doesn't really make
>sense as options parsing append before instanciation.
>For options global to a layer that would make some sense but then the
>callback also need to get the struct/context on wich it should
>operate. So this context should be static otherwise you can't put
>it in the config_t->priv field. But we don't want static stuff so
>for G2 lib that's anyway completly unusefull.
>We may let that for app level stuff but i doubt it's really usefull.

    I've added these for the same reason as CONF_TYPE_STRING_LIST. And
application level stuff of that type still exist and I wonder if we can
avoid that - see "include" or "help" options.

>> // text (none)		char**		-	-	-
>>   CONF_TYPE_PRINT,
>That's also only usefull at app level.

Of course, it is for application level.

>> /*
>> 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".

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

CONF_TYPE_DUMMY is just subconfig type but it's name will be ignored so
config parser will just parse it's subconfig as continuation of current
config. It may be useful for: 1) adding new config_t without reallocation
current config_t (someone already wrote about something alike here); 2)
for Gui - it will indicates submenu or if option is empty then just menu
separator. What I mean submenu - for example, you may have root config
consists of options "File", "Video", "Audio", "About" but inside of it
are real options. Also it's possible to have second, etc. level of menu,
just look on any GUI application. It's also application level option, of
course. :)

>> /* these macros are very useful for parsers or UI */
>> #define CONF_TYPE_HAS_CHILD(x)		(x>=CONF_TYPE_SUBCONFIG)
>> #define CONF_TYPE_ALLOW_WILDCARD(x)
>> #(x==CONF_TYPE_LIST||x==CONF_TYPE_CHAIN||x==CONF_TYPE_FUNC_FULL)
>I don't like this way for wildcard. Wildcard support depend on the
>string 2 C parser so imho it should be queried from it.

You propose to move it into that parser? Anyway I placed it as helper
macro so I don't mind to remove it.

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

>> // It used for lists to reverse order of arguments in char***
>> #define CONF_REVERSE		(1<<7)
>Is that really useful ??? I mean you need code to support that, on
>the other hand you can only just reorder your list instead of
>adding this flag.

It really is here only for "-vop" option. If we'll get rid of it then it
may be don't need. :)

>> /* macro to get relative address of x.y in struct x */
>> #define STRUCT_OFF(x,y)		((void *)(&x.y-x))
>I doubt that this macro really work. And even if it work it need an
>instance to apply it to. Why don't you just take the one in m_struct.h
>(wich is from glib.h) wich just need the declaration of the struct.

Hmm. I think you are right. Sorry. It seems I tired and I must to have a
rest. Thank you. This is right?

#define STRUCT_OFF(x,y)  ((void *)((x *)0)->y)

>>     With best wishes.
>My best wishes too.

    Thank you. :)
    Best wishes again and more. :)
    Andriy.