[vlc-devel] [PATCH 4/4] libvlc: media: use vlc_atomic_rc for refcount

[Steve Lhomme]

IMO it should be mentioned that it allows more use cases than before. 
I'm fine with the patch BTW.

On 2020-11-23 9:58, Alexandre Janniaux wrote:
> Hi,
> 
> I've seen this but didn't find any issue that could link it
> so didn't mention it (it's pretty obvious from the diff).
> 
> I don't really see why media would not be thread-safe to
> release whereas every other object would be.
> 
> Regards,
> --
> Alexandre Janniaux
> Videolabs
> 
> On Mon, Nov 23, 2020 at 08:03:19AM +0100, Steve Lhomme wrote:
>> This one was not protected by a lock. Is it possible to do the calls from
>> different threads now ? Or was it broken ?
>>
>> On 2020-11-21 13:21, Alexandre Janniaux wrote:
>>> ---
>>>    lib/media.c          | 9 ++++-----
>>>    lib/media_internal.h | 4 +++-
>>>    2 files changed, 7 insertions(+), 6 deletions(-)
>>>
>>> diff --git a/lib/media.c b/lib/media.c
>>> index dd4faf0066..6f7911e0a2 100644
>>> --- a/lib/media.c
>>> +++ b/lib/media.c
>>> @@ -37,6 +37,7 @@
>>>    #include <vlc_meta.h>
>>>    #include <vlc_url.h>
>>>    #include <vlc_thumbnailer.h>
>>> +#include <vlc_atomic.h>
>>>    #include "../src/libvlc.h"
>>> @@ -501,7 +502,7 @@ libvlc_media_t * libvlc_media_new_from_input_item(
>>>        p_md->p_libvlc_instance = p_instance;
>>>        p_md->p_input_item      = p_input_item;
>>> -    p_md->i_refcount        = 1;
>>> +    vlc_atomic_rc_init(&p_md->rc);
>>>        vlc_cond_init(&p_md->parsed_cond);
>>>        vlc_mutex_init(&p_md->parsed_lock);
>>> @@ -643,9 +644,7 @@ void libvlc_media_release( libvlc_media_t *p_md )
>>>        if (!p_md)
>>>            return;
>>> -    p_md->i_refcount--;
>>> -
>>> -    if( p_md->i_refcount > 0 )
>>> +    if( !vlc_atomic_rc_dec(&p_md->rc) )
>>>            return;
>>>        uninstall_input_item_observer( p_md );
>>> @@ -675,7 +674,7 @@ void libvlc_media_release( libvlc_media_t *p_md )
>>>    void libvlc_media_retain( libvlc_media_t *p_md )
>>>    {
>>>        assert (p_md);
>>> -    p_md->i_refcount++;
>>> +    vlc_atomic_rc_inc( &p_md->rc );
>>>    }
>>>    // Duplicate a media descriptor object
>>> diff --git a/lib/media_internal.h b/lib/media_internal.h
>>> index e4d0d03fae..b8646abce2 100644
>>> --- a/lib/media_internal.h
>>> +++ b/lib/media_internal.h
>>> @@ -35,10 +35,12 @@
>>>    struct libvlc_media_t
>>>    {
>>>        libvlc_event_manager_t event_manager;
>>> +
>>>        input_item_t      *p_input_item;
>>> -    int                i_refcount;
>>>        libvlc_instance_t *p_libvlc_instance;
>>>        libvlc_state_t     state;
>>> +    vlc_atomic_rc_t    rc;
>>> +
>>>        VLC_FORWARD_DECLARE_OBJECT(libvlc_media_list_t*) p_subitems; /* A media descriptor can have Sub items. This is the only dependancy we really have on media_list */
>>>        void *p_user_data;
>>> --
>>> 2.29.2
>>>
>>> _______________________________________________
>>> vlc-devel mailing list
>>> To unsubscribe or modify your subscription options:
>>> https://mailman.videolan.org/listinfo/vlc-devel
>>>
>> _______________________________________________
>> vlc-devel mailing list
>> To unsubscribe or modify your subscription options:
>> https://mailman.videolan.org/listinfo/vlc-devel
> _______________________________________________
> vlc-devel mailing list
> To unsubscribe or modify your subscription options:
> https://mailman.videolan.org/listinfo/vlc-devel
>