[MPlayer-dev-eng] Re: mplayer - runtime win32 codecs dir selection

[Diego Biurrun]

On Wed, Nov 16, 2005 at 05:24:54PM -0600, Joey Parrish wrote:
> On Wed, Nov 16, 2005 at 10:58:46AM -0500, Rich Felker wrote:
> > On Wed, Nov 16, 2005 at 10:16:45AM +0100, Diego Biurrun wrote:
> > > On Tue, Nov 15, 2005 at 01:22:50PM +0200, Oded Shimon wrote:
> > > > On Tue, Nov 15, 2005 at 10:31:36AM +0100, Attila Kinali wrote:
> > > > > 
> > > > > Could you do me a favor and write a small patch for
> > > > > mplayer/mencoder that allows to select the directories
> > > > > where the codecs are stored at runtime ?
> > > > 
> > > > +#ifdef WIN32_LOADER
> > > > +	{"codecs-dir", &def_path, CONF_TYPE_STRING, 0, 0, 0, NULL},
> > > 
> > > Why #ifdef WIN32_LOADER?  The Real and XAnim libs don't depend on that.
> > 
> > IMO MPlayer has waaaay too many #ifdefs like this. If the code in
> > question doesn't actually _depend_ on the optional feature (like
> > depend on its include files, library code, etc.) then it should be
> > unconditionally compiled. Otherwise it just hurts readability and we
> > get into cases like the DVD/MKV problem where MKV demuxer didn't have
> > the chapter variables because they were under #ifdef HAVE_DVD or
> > whatever.
> 
> It _DOES_ depend on def_path, which is defined in the loader code.
> Removing the ifdef will break compilation without the loader.
> So the proper solution is to:
>   1) move def_path elsewhere.
>   2) make another name for that variable, and change the loader to
>      reference that instead.
> 
> I vote for #2, because the variable def_path is too cryptic out of
> context.

Well, you could rename it while moving it somewhere else...

Diego