[MPlayer-G2-dev] Re: cfg.h (config layer 0 discussion)

[Arpi]

Hi,

> >// position (slider)	off_t*		min	max	-
> >  CONF_TYPE_POSITION,
> >// string (input box)	char**		min	max	- (min/max length)
> >  CONF_TYPE_STRING,
> >// select list (combobox) int*		-	-	config_select_list_t*
> >  CONF_TYPE_SELECT,
> 
> Are you sure if limitation select list to int type only is enough?

Not. Fisrt i also wanted a void* version, but then i dropped, as i couldn't
find any examples of use.

> >// group separator	-		-	-	config_group_info_t*
> >// (this is NOT a real option, but an option group definition, for gui use)
> >  CONF_TYPE_GROUP,
> 
> It was CONF_TYPE_DUMMY in mine, isn't it? But then we have to add into

yes
but dummy is a dummy name for it :)

> config_group_info_t* optional subconfig (config_t*).

why???
forget subconfig

> >//*** this is not a valid type, but the number of types:
> >  CONF_TYPE_MAX
> >} config_type_t;
> 
> Also I'm against of deleting CONF_TYPE_LIST here - it may be very useful
> in many of cases - I've written examples before. It you will stay only
> with CONF_TYPE_STRING_LIST then [G]UI has no ways to get hints which
> choises we have there. If you want to force [G]UI to have __only__ input
> field for string "any,some,next" then I don't think it's useful because
> we just deny [G]UI to have selection list box other than filters. If you
> want extra examples I could find more. And you were agree with me we have
> to have extended list type. :)

i don't really understand your problem with gui here.
guis should implement filter setup dialogs, with filter-dependent configs.

commandline filter list parsing will be handled by layer 3, as i described
in the past. actually it's a string list in the first parsing pass, then
it tries to load each filter in the list and do 2nd pass parsing with its
param list.

>     CONF_NOSAVE were here for purpose of config save - no other hints we
> could have for config parser to don't write it in config if we'll request
> for saving it. Why you removed it?

ah, it was for config files? sorry, i'll readd then.
i thought it's remained from g1's config, where it means no save its value
(in playtree/configtree param saving/restoring mess).

> >/* note: version has to be first in ModuleInfo struct in loadable module
> >   to allow us always do validation/version check of module */
> >typedef struct {
> >  unsigned short type, version; // for loadable plugins
> 
>     I've proposed to have version as unsigned char[] so if we have it as
> unsigned short then we will fill it by some macro, will we?

yes, and it's easier in module implementations than messing with chars
it doens't really matter, i just don't like char as version type.

> >  char *name;
> >  char *desc; // short description for commandline help and gui
> >  char *maintainer;
> >  char *author;
> >  char *comment;
> >  config_t *config;
> >  void *config_defaults;
> >  int  *config_size;
> 
>     I was mistaken here - not int * but size_t (without '*').

yes, i forgot to fix it

> (note: not int since size_t is standard type and is _always_ the same
> size as any pointer - int  may have other size than  void *  on some
> systems).

i don't want the mess with size_t definition, it requires varios
platform-dependent #includes to get it.
since size_t is thye only non-C type in cfg.h, it's better to leave it and
use int. i guess we won't have structs bigger than 2 GB :)

you can still cast to size_t if needed

> >Problem:
> >we should store some reference inside the module_info_t struct to the
> >actual module entry point (maybe the addess of the open() func? or the
> >functions_t struct? do we need to store dlopen() handle there?)
> 
>     I've proposed to have open() here. It may be useful to have even
> control() pointer here also. :)

i've decided (after trying some variants), it will be void* entry, which
may be casted to open() or xx_function_t, depending on module API.


A'rpi / Astral & ESP-team

--
Developer of MPlayer G2, the Movie Framework for all - http://www.MPlayerHQ.hu