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

[Andriy N. Gritsenko]

    Hi, D Richard Felker III!

Sometime (on Monday, May 26 at 17:26) I've received something...
>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.

    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!
    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
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.

    With best wishes.
    Andriy.