[vlc-devel] commit: Implement access_GetParentInput and demux_GetParentInput and use. ( Pierre d'Herbemont )

[Laurent Aimar]

On Wed, Aug 26, 2009, Pierre d'Herbemont wrote:
>
> On Aug 26, 2009, at 8:01 PM, Rémi Denis-Courmont wrote:
>
>>> Are we not supposed to do that? Fine, but don't let it compile  
>>> without
>>> warning.
>>
>> How do you know that the caller is right or wrong?
>
> Just don't allow vlc_object_hold() on it. Then you are safe. Or make  
 Nope, on correctly written object, you can call var_Set* on a reference,
but you cannot arbitrary call any accessors. For that you need to look
at the docs when present, and if not documented, assume you cannot.

> sure that you allow p_demux->psz_path to be NULL, and don't set a random 
> value to it.
 Without lock, it would be pointless.

> What I am trying to point out, is that our API doesn't reflect the  
> reality at all. You can vlc_object_hold() an object, but that won't  
> actually let you live with a valid reference of it, with apparently no  
> way to know it.
 It kind of does, it's more a documentation problem.
 - var_Set is legal (otherwise I would consider the module/object buggy)
 - For accessors, it is more complex: you have 3 categories:
   1. for module usage only.
   2. for object owner only.
   3. for the whole world.

 For demux_t/access_t/stream_t, there are only accessors of type 1 and 2 and as
there are non threaded object (internal module thread does not count), 1 and 2
can in fact be used by both module and owner (being carefull with recursion).

> Either choose, we fix demux_Delete() or we make sure vlc_object_hold()  
> doesn't work on that object.
 It is wrong as explain.

>>>  So, if they don't need to be refcounted and doesn't support
>>> refcounting, vlc_object_hold() should be disabled on those objects  
>>> then.
>>
>> The private data does not support reference counting. It is completely
>> impossible. We need to attach an object *before* we run module_need() 
>> on it,
>> so that variables can be propagated in pf_open. At the same time, as  
>> soon as
>> an object is attached, it becomes visible from vlc_object_find(),
>> vlc_object_find_name() and vlc_list_children().
>
> Well, I thought we were going away from this. ie, we simply let owner  
> decide to expose the object when it is ready, and try to remove those  
> accessors.
 Even if an object is ready, it may not be exposable. A lot of module are
in this situation with sometime the exception of var_Set/Get.
>
>> So evidently, anything that pf_open initializes or modifies is outside 
>> the
>> scope of reference counting. From access_t, p_module, p_sys,  
>> pf_control,
>> pf_read, p_block, psz_access, psz_path and info are all in that case  
>> for
>> instance. Besides, we really don't want to make all object callbacks  
>> re-
>> entrant. It would be too complicated, error-prone and inefficient.
>
> Err. Creating something outside refcounting is just fine, provided that 
> access to the data is thread safe and that you don't let a random  
> pointer value when you free it. This is the problem I am stating above.
 No, a random value is fine if you are forbidden to use it in any case...

>> I stay by my statement that pf_destructor is almost completely  
>> useless. It's
>> only useful for those few object types with type-specific thread-safe 
>> data.
>
> It's useful if you want a set of data to be freed when it is not needed. 
> ie -  when you expect to do refcounting on some memory object.
 And it is actually needed in only a really few cases, that is when 
accessors of type 3 are presents.

-- 
fenrir