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

[Pierre d'Herbemont]

On Aug 27, 2009, at 8:50 AM, Rémi Denis-Courmont wrote:

>
> On Wed, 26 Aug 2009 22:27:10 +0200, Pierre d'Herbemont
> <pdherbemont at gmail.com> wrote:
>> My point is we don't do proper refcounting on some object that
>> announce it.
>
> You still failed to give any concrete example of that.

*All* object, it is a matter of naming. input_thread_t, demux_t,  
access_t are objects, They all have vlc_object_hold() and some of them  
may crash on public accessor.

By object I generally refer to an entity that you create and destroy.  
This is simply a generic term that is being misused by vlc_object_t.

I say that demux_t is an object, and using vlc_object_hold() on it is  
misleading because it's only retaining some part of the structure.  
Okay, that you say it's only retaining at least the COMMON_MEMBERS,  
but then, please change the name. If you have use some serious object  
oriented API this is completely misleading.

>> Your point is that we've never been annoucing more than only
>> refcounting internals so that variables can be used.
>
> No. The point is that we have not been exposing at least:
> - object name,
> - object type (name),
> - VLC variables,
> - children list,
> - "alive" status and ability to kill.

ie - internals. We agree on that. (I assume that you don't want the  
"not", else this is non sense ;) ).

>> So obviously there is something missing: The documentation on that,  
>> or
>> the name changes in the API, to be less misleading.
>
> It is not misleading. It holds an object, not an access/demux/ 
> whatever.
> You are misleading.

Come on. access is an object, demux is an object, whatever is an  
object. Don't use generic misleading term to describe something else.  
This is misleading.

>> We don't do proper refcounting for (all?) vlc_object_t, even if we
>> pretend we do by having this vlc_object_hold. (This used to be even
>> more true back in the time when we used to wait until object
>> destruction).
>
> You still failed to give any such object. I am not aware of any  
> error in
> our handling of access and demux objects, other than the way we  
> "kill" them

I don't think there should be any error because the only vlc_object_t  
that are being used outside are
aout_instance, input_thread_t, and playlist_t.

Those are safe to use if you hold a reference that comes from  
vlc_object_hold().

The point is, this is not the case with other object (demux, access),  
and I would like to see a distinction between those. Laurent suggested  
doc change, which would be nice.

I would like probably to change the API to reflect those different  
type of reference owning, toward something less generic but more  
explicit.

> (which is _not_ a reference counting problem anyhow).

Completely agreed.

> vlc_object_hold() is legal if:
> - the caller already has a reference to the object, and/or

Yes, but then the reference you keep is restrained compared to the  
reference that was passed. I would like to make that distinction at  
API level.

Pierre.