[MPlayer-cvslog] r34257 - in trunk: cfg-common.h cfg-mplayer.h m_option.c m_option.h

[Ingo Brückl]

Reimar Döffinger wrote on Fri, 28 Oct 2011 00:24:33 +0200:

> On Wed, Oct 26, 2011 at 10:07:45PM +0200, Ingo Brückl wrote:
>> Reimar Döffinger wrote on Wed, 26 Oct 2011 20:22:53 +0200:
>>
>> > On Wed, Oct 26, 2011 at 07:31:15PM +0200, Ingo Brückl wrote:
>> >> reimar wrote on Tue, 25 Oct 2011 22:18:35 +0200 (CEST):
>> >>
>> >> > Author: reimar
>> >> > Date: Tue Oct 25 22:18:35 2011
>> >> > New Revision: 34257
>> >>
>> >> > Log:
>> >> > Sanitize include behaviour.
>> >>
>> >> > The normal func_param argument type will iterate over all previous
>> >> > values each time a new value is assigned.
>> >> > This leads e.g. to a complete mess and non-working recursion limiting
>> >> > when creating a config file that includes itself.
>> >> > Seem to also fix bug #1994.
>> >>
>> >> > Modified:
>> >> >    trunk/cfg-common.h
>> >> >    trunk/cfg-mplayer.h
>> >> >    trunk/m_option.c
>> >> >    trunk/m_option.h
>> >>
>> >> > Modified: trunk/cfg-mplayer.h
>> >> > ========================================================================= =====
>> >> > +++ trunk/cfg-mplayer.h Tue Oct 25 22:18:35 2011        (r34257)
>> >> > @@ -301,7 +301,7 @@ const m_option_t mplayer_opts[]={
>> >> >      {"noenqueue", &enqueue, CONF_TYPE_FLAG, 0, 1, 0, NULL},
>> >> >      {"guiwid", "-guiwid has been removed, use -gui-wid instead.\n",
>> >> > CONF_TYPE_PRINT, 0, 0, 0, NULL},
>> >> >      {"gui-wid", &guiWinID, CONF_TYPE_INT, 0, 0, 0, NULL},
>> >> > -    {"gui-include", cfg_gui_include, CONF_TYPE_FUNC_PARAM, CONF_NOSAVE, 0, 0, NULL},
>> >> > +    {"gui-include", cfg_gui_include, CONF_TYPE_FUNC_PARAM_IMMEDIATE, CONF_NOSAVE, 0, 0, NULL},
>> >>
>> >> This segfaults the GUI.
>> >>
>> >> If I understand CONF_TYPE_FUNC_PARAM_IMMEDIATE correctly, this isn't the
>> >> desired behaviour either.
>> >>
>> >> Option gui-include is meant to override the gui.conf settings. In order to do
>> >> this, MPlayer must have called cfg_read() where gui.conf is read and
>> >> m_config_t *gui_conf is set. CONF_TYPE_FUNC_PARAM_IMMEDIATE seems to act
>> >> prior to this, thus gui_conf still is NULL.
>> >>
>> >> As the GUI's config files don't allow includes anyway, it seems save to
>> >> stay with CONF_TYPE_FUNC_PARAM.
>>
>> > However you will end up with some really strange behaviour if someone
>> > ever uses multiple gui_include (recursion is just the case where it is
>> > most obvious).
>>
>> I was just doing some tests, but could not figure out any problems. Every
>> gui-include seems to override properly what the previous gui-include may
>> have set. Maybe you could give me a hint what to look or check for?

> Profiles and thus probably also per-protocol or extension options
> probably will not be working properly.
> No idea if they are supposed to anyway though.

At the moment I don't see a practical use in per-any gui configs.

>> > And rather not call it include, when it doesn't really behave like that.
>>
>> I does behave like include, but limited to the gui options which are a subset
>> of the MPlayer options

> No, it doesn't really. As said, for example a second "include" will
> cause all previous includes to be re-evaluated.
> Especially with section this has all kinds of funny and unexpected
> effects.
> You probably should be able to also see it if you do something like
> gui-include=cache.conf # configures cache
> nocache=yes
> gui-include=empty.conf

> should result in cache being enabled at the end.

This actually segfaults for the very same reason (MPlayer didn't have called
cfg_read() before). I suggest the attached patch.

Ingo
-------------- next part --------------
A non-text attachment was scrubbed...
Name: gui-include.patch
Type: text/x-diff
Size: 662 bytes
Desc: not available
URL: <http://lists.mplayerhq.hu/pipermail/mplayer-cvslog/attachments/20111206/b61926c8/attachment.bin>