[MPlayer-dev-eng] [PATCH] -noconfig option

[Alban Bedel]

On Sat, 12 Apr 2008 20:17:41 +0400
Andrew Savchenko <Bircoph at list.ru> wrote:

> > > +void preparse_command_line(int argc, char* argv[])
> > > +{
> > > +    int i;
> > > +
> > > +    for(i=1; i<argc; i++) {
> > > +        if(!strcmp(argv[i], "-really-quiet"))
> > > +            verbose= -10;
> > > +        if(!strcmp(argv[i], "-noconfig"))
> > > +            noconfig= 1;
> > > +    }
> > > +}
> >
> > With respect to this patch, this is fine. But it is quiet a hack
> > that will catch stuff which would not normally be treated as
> > command line options. Ideally this should latter be replaced by
> > something robuster.
> 
> I do not understand you. What do you want me to do?
> 
> The problem is that some command line options must be analyzed 
> before configuration files will be. If I'll just move command line 
> parsing before config files one, it will break overriding 
> precedence (sysmem<-user<-CLI).

Sure we still need an extra pass before parsing the config files.
 
> Yes, currently used approach is not the best. Moreover it creates 
> overhead: these options are parsed twice: in preparse and in 
> normal parse routines, though second parsing is absolutely 
> useless, but without it mplayer will complain about unknown 
> options.

That doesn't matter much. There is rarely more than a few dozen
parameters on the command line, and as it happen only at startup
performance is not a problem.

> Do you have some ideas how to fix this? I can see three different 
> posibilities:
> 1) Leave it as is.
> 2) Parse all CLI options before configs, and either :
> 2a) save them in temporary storage and reassign after config files 
> parsing;
> 2b) mark these options somehow and modify config files parser in 
> order to not override marked options.
> 3) Modify parse subsystem to parse some subset of CLI options 
> before config files and forbid their further modification whilst 
> config files are parsed.
> 
> Variant 2b) sounds as the best for me, but even it will create too 
> much overhead. So perhaps our best is to leave things as is?

Again overhead doesn't matter here. What I'm thinking about is a mix of
2b and 3. Basically flag some option as preparsed option, modify the
command line preparsing to use the normal option parsing routines and
the config system to ignore preparsed options afterward.

> Well, lets talk about new patches.
> 
> The first patch moves some common code from mplayer.c and 
> mencoder.c to mpcommon.[ch].
> 
> The second patch fixes a bug or, precisely speaking, missed feature 
> of mencoder. Look at the man page: it says that mencoder read both 
> system-wide and user-specific config files, but currently it reads 
> only user-specific one. Anyone can easy check this by 
> creating /etc/mplayer/mencoder.conf (or whatever is your's 
> MPLAYER_CONFDIR). 
> 
> The reason is easly seen in comparison between parse_cfgfiles() in 
> mplayer.c and mencoder.c. The last one use only get_path() which 
> checks only for home directory. The second patch fixes this, it may 
> be applied with or without the first patch.
> 
> The third patch adds new functionality as described above. It 
> requires both previous patches to be applied.
> 
> The fourth patch is cosmetics required after previous patches, real 
> cosmetics: not newlines, but indentation shifts.
> 
> And finally the fifth patch is the manual page update.

That look alright to me, except perhaps the if(a) if(b) that would
better be replaced by if(a && b). Anyway I'll look at fixing the
preparsing correctly and we get back to this.

	Albeu