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

D Richard Felker III dalias at aerifal.cx
Mon May 26 16:33:56 CEST 2003


On Mon, May 26, 2003 at 11:44:24AM +0300, Andriy N. Gritsenko wrote:
> >it's always readable by upper layer (so VFCTRL_GET_PARAMS is useless), but
> >we should add VFCTRL_SET_PARAMS (or better name) to notify filter about
> >config changes made by UI. i'm not completely sure about this one yet.
> >(runtime config change is always a mess...)
> 
>     As you said yourself, runtime config change may be a mess and I don't
> like to change parameters in running application - only filter may decide
> if parameter may be applied or not. So I've proposed to get copy of
> config_data_t* and let filter apply it instead of direct changing it,
> changes in a copy are always safe. So application will have something
> like:
> 
> data_copy=malloc(vf->info->config_size);
> vf->control(vf, VFCTRL_GET_PARAMS, data_copy);
> .....change parameters.....
> vf->control(vf, VFCTRL_SET_PARAMS, data_copy);
> free(data_copy);
> 
> Of course, we could
> 
> data_copy=malloc(vf->info->config_size);
> memcpy(data_copy, vf->cfg, vf->info->config_size);
> .....change parameters.....
> vf->control(vf, VFCTRL_SET_PARAMS, data_copy);
> free(data_copy);
> 
> but I don't like to trust developers if they will always use a copy but
> don't change existing data. You know it yourself, many programmers are
> too lazy to know all requirements and that may cause troubles.

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.

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.

Rich





More information about the MPlayer-G2-dev mailing list