[MPlayer-dev-eng] [PATCH] tv config in file (load/save)

[Ötvös Attila]

2007. szeptember 11. 06.20 dátummal Vladimir Voroshilov ezt írta:

Hi Vladimir Voroshilov!

> >
> > tv_global_opts_fields[] : allow in tv config file (global section)
>
> IMHO, global section should allow ALL options which can be passed in
> ~/.mplayer/config.
>
> > tv_global_opts_fields_mix[]: disallow in tv config file (global section)
>
> "scan","scan_threshold","scan_period" is not "-tv" options. it is
> "-tvscan" related options.
>
> Other options are descussable.

mix_tv_param() need all options in tv_param_t. If left out is can't mix...
Please see mix_tv_param().

> "tvconfigfile" can be global option, like "mplayer -tvconfig <myconfig>"
> "tvconfigfilesave" can be tvconfgifile+".new" - no additional option
> needed.

Now, savetvconfig() make backup from tv config file eg: "tv.conf~".

> notvconfig=(tvconfigfile==NULL)
> notvconfigsave: discussable.

I think best idea:
tvconfig=yes - use default: tv.conf
tvconfig=no - don't use tv config file
tvconfig=foo.conf - use specified file: foo.conf (if not exist: load from 
default, save to foo.conf)

> > > >+static void parse_channels_priv(tvi_handle_t *tvh)
> > > >+{
> > >
> > > What is this supposed to do?
> >
> > convert from channel_priv_t to tv_channels_t
>
> I suggest extending tv_channels_t to include all desired info (freq
> offset, norm, etc)
> This will avoid  unnecessary code duplication.

OK. I will modify.

> > > >+ int notvconfig; ///< don't use tv.conf
> > > >+ char *tvconfigfile; ///< config file name
> > > >+ char *tvconfigfilesave; ///< config file name with save
> > > >+ int tvconfigsave; ///< save config file if close
> > > >+ int saveall; ///< save all fields
> > > >+ void *channels_priv;
> > > >} tv_param_t;
> > >
> > > Is notvconfig really needed?
> > > Why not "notvconfig<=>(configfile==NULL)" ?
> >
> > If exist tv.conf (default config file) in mplayer home dir
> > (~/.mplayer/tv.conf) is use this. The notvconfig options disable usage
> > defult file if exist.
>
> if tv.conf exists, mplayer must use it. If user don't want to use
> tv.conf he should either remove it or pass tvconfig without argument
> (discussable).
>
> > > What about putting config file parsing code (or at least for "global"
> > > section) into
> > > stream_open of stream/stream_tv.c (before stream->priv assignement) ?
> > > In this case both default and assigned values for options will be
> > > available and duplicated structure definitions will not be needed..
> >
> > Problem similar -gui options.
> > If order is:
> > 1. parse ~/.mplayer/config
> > 2. parse mplayer arguments - overwrite config
> > 3. parse ~/.mplayer/tv.conf or other if set configfile parameter -
> > overwrite config and mplayer arguments
> >
> > User can't specity arguments in this case because overwrite tv.conf.
>
> User CAN do this:
> 1. parse ~/.mplayer/config
> 2. parse mplayer arguments - overwrite config
> 3. parse ~/.mplayer/tv.conf or other if set configfile parameter -
> overwrite config and mplayer arguments ONLY if them still equals to default
> values (in stream_open() you will
> have access to both default values through stream_tv_defaults variable
> and values got from ~/.mplayer.conf through
> opts parameter. your can compare them and take decision about options
> from tv.c).

I think problem:

tv.conf:
norm=SECAM

if user can't set default: "mplayer tv:// -tv norm=PAL" because nothig 
different empty and default value. 
I think need defults + empty (unsetted) structure.

> In this case we will have these priorities:
> 1. options in command line
> 2. options in ~/.mplayer/config
> 3. options in ~/.mplayer/.tv.conf
> 4. defaults (as last resort)

Yes, this patch make this.
Parse order reversed priorities, but can't parse tv config before parsing 
config or command line. I think need whatever mix.

> To enable options in tv.conf user have to remove them from
> ~/.mplayer/config (Perhaps mplayer should otput apropriate message)

I think problem if user use mplayer without tv config file and don't solve 
priority problem (remain: command line vs tv.conf).

> > If order is:
> > 1. parse ~/.mplayer/config
> > 2. parse ~/.mplayer/tv.conf - overwrite config
> > 3. parse mplayer arguments - overwrite config and tv.conf
> >
> > User can't specify tv config file in this case because loadtvconfig()
> > after parsing mplayer arguments where specify tv config file.
>
> Agree.
>
> > This patch:
> > 1. parse ~/.mplayer/config to stream_tv_defaults[]
> > 2. parse mplayer arguments to stream_tv_defaults[]
> > 3. parse tv config file to tv_param_cfg[]
> > 4. mix stream_tv_default[] + tv_param_cfg[] in mix_tv_param():
> >
> > if stream_tv_defaults.opt == empty_tv_defaults.opt and
> > tv_param_cfg.opt == empty_tv_defaults.opt:
> > no specify config/arguments/tv.conf -> overwrite tv_defaults.opt
> >
> > if stream_tv_defaults.opt == empty_tv_defaults.opt and
> > tv_param_cfg.opt != empty_tv_defaults.opt:
> > specify only tv.conf -> overwrite tv_param_cfg.opt
> >
> > if stream_tv_defaults.opt != empty_tv_defaults.opt:
> > specify config/argumnets -> drop specify in tv.conf
>
> This is the same  behaviour as i desribed above, but without three
> additional structures.
>
> First look shows that you need only one additional structure: for
> options used in
> "channels" section of tv.conf, but this will need more previse look.

Best regard.
Attila