[MPlayer-G2-dev] Re: About final cfg.h (config layer 0 discussion)

[Arpi]

Hi,

> On Mon, May 26, 2003 at 05:48:53PM +0300, Andriy N. Gritsenko wrote:
> > >This is total nonsense since some of vf->cfg may be pointers to other
> > >dynamically allocated stuff. With all this confusion about who will
> > >allocate, free, and update what, you're bound to create memory leaks.
> > 
> >     Where did you find memory leak in lines above? Caller (upper level of
> > application) allocate memory, use it as temporal and then free it. No
> > other level (core or other application layer) doesn't know what is that
> > memory. No leaks at all!
> 
> vf->cfg may be full of string pointers.

yes

btw if you wnat to copy vf->cfg, there is a func in layer-0 for it:
  new_cfg=mpconfig_get_cfg(vf->info,vf->cfg);

but it's not the way we want to go.

> >     On other hand, if you will change vf->cfg directly in multithreaded
> > application, you will get a memory leak - you don't know if some of
> > parameters there are static or allocated and don't know if it will be
> > reallocated or freed while you changing it. So don't touch that vf->cfg
> > directly if you don't want memory leaks! :)
> >     About dinamically allocated data as string parameters - when you'll
> > call VFCTRL_GET_PARAMS control you'll get only copy of parameters then
> > only module itself may allocate and free own copies. Just let's have all
> > documented well.
> > 
> > >IMO you should just let the program modify vf->cfg directly. If the
> > >module needs an untouchable copy of the 'current value' of some param
> > >to use when deciding whether a runtime change it allowed, it should
> > >make its own copy of the data in vf->priv. Otherwise it should expect
> > >the program to call VFCTRL_SET_PARAMS immediately after changing
> > >vf->cfg.
> > 
> >     How will you know if filter allowed set some parameter only on filter
> > opening? If your application will change some parameter (for example,
> > size of frame) which is critical for the filter then would you guarantee
> > that _any_ filter will work correctly (think, please, about multitasking
> > or multithreading application) until you say VFCTRL_SET_PARAMS for it? I
> 
> No, it's nonsense to allow setting filter options in a separate
> thread. MPlayer will not be polluted with such thread/locking/etc.

i want to make g2 libs thread-safe as far as possible
(but where it means big mess/hacks/ugly code then it is ignored :))

> crap. It's even a problem with your design unless the filters do some
> sort of locking during VFCTRL_SET_PARAMS, and filters MUST NOT have to
> be thread-aware!! If the calling program wants to do such idiocy, it
> should have to make its thread exclusive before setting vf->cfg and
> calling VFCTRL_SET_PARAMS.

actually filters should keep their local copies of the runtime changeable
parameters if they want to read it after config() is done.
(most filter params are used only to set up tables or internal data, it's not
used any more after config()).

they should read/write those vars only in open(), config() and VFCTRL_SET_PARAMS.

> > would to say you may have a big mess if you will change some parameter
> > directly. BTW, on running application you may have that vf->cfg already
> > dinamically allocated and changed right when you trying to modify it. I'm
> > against it very much and I've against have that vf->cfg visible for
> > higher level of application at all - it's _private_ area of variables.
> 
> No. vf->priv is private. vf->cfg is totally public. This is clean C

yes

> code, not some object-oriented encapsulated bullshit with data-hiding.

also note that cfg is actually filled by the GUI code directly, before
passing to the filter open. so it MUST access/see it.
(in case of CLI, there is strings=>cfg parser in layer 1)


A'rpi / Astral & ESP-team

--
Developer of MPlayer G2, the Movie Framework for all - http://www.MPlayerHQ.hu