Re: Re: cfg.h (config layer 0 discussion)

Arpi <[email protected]>
Newsgroups gmane.comp.video.mplayer.g2.devel
Message-ID <[email protected]>
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
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.