[vlc-devel] [vlc-commits] Revert "lib: media: Use an atomic refcounter"

[Hugo Beauzée-Luyssen]

On Wed, Nov 18, 2020, at 9:57 AM, Steve Lhomme wrote:
> On 2020-11-18 9:53, Hugo Beauzée-Luyssen wrote:
> > On Mon, Oct 28, 2019, at 11:34 AM, Hugo Beauzée-Luyssen wrote:
> >> vlc | branch: master | Hugo Beauzée-Luyssen <hugo at beauzee.fr> | Thu Oct
> >> 24 10:28:35 2019 +0200| [cf14bc8f63bc19092f19354d93f99c289d00afb4] |
> >> committer: Hugo Beauzée-Luyssen
> >>
> >> Revert "lib: media: Use an atomic refcounter"
> >>
> >> This reverts commit 1b07eaa8612eb77a28178a8739a1a23d4dc16cef.
> >>
> >>> http://git.videolan.org/gitweb.cgi/vlc.git/?a=commit;h=cf14bc8f63bc19092f19354d93f99c289d00afb4
> >> ---
> >>
> >>   lib/media.c          | 8 +++++---
> >>   lib/media_internal.h | 3 +--
> >>   2 files changed, 6 insertions(+), 5 deletions(-)
> >>
> >> diff --git a/lib/media.c b/lib/media.c
> >> index 36dee3b9e5..d4e3ba7a62 100644
> >> --- a/lib/media.c
> >> +++ b/lib/media.c
> >> @@ -394,7 +394,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;
> >> -    vlc_atomic_rc_init(&p_md->refcount);
> >> +    p_md->i_refcount        = 1;
> >>   
> >>       vlc_cond_init(&p_md->parsed_cond);
> >>       vlc_mutex_init(&p_md->parsed_lock);
> >> @@ -547,7 +547,9 @@ void libvlc_media_release( libvlc_media_t *p_md )
> >>       if (!p_md)
> >>           return;
> >>   
> >> -    if(!vlc_atomic_rc_dec( &p_md->refcount ))
> >> +    p_md->i_refcount--;
> >> +
> >> +    if( p_md->i_refcount > 0 )
> >>           return;
> >>   
> >>       uninstall_input_item_observer( p_md );
> >> @@ -583,7 +585,7 @@ void libvlc_media_release( libvlc_media_t *p_md )
> >>   void libvlc_media_retain( libvlc_media_t *p_md )
> >>   {
> >>       assert (p_md);
> >> -    vlc_atomic_rc_inc(&p_md->refcount);
> >> +    p_md->i_refcount++;
> >>   }
> >>   
> >>   /**************************************************************************
> >> diff --git a/lib/media_internal.h b/lib/media_internal.h
> >> index cfeb056178..aefc731c68 100644
> >> --- a/lib/media_internal.h
> >> +++ b/lib/media_internal.h
> >> @@ -29,13 +29,12 @@
> >>   
> >>   #include <vlc_common.h>
> >>   #include <vlc_input.h>
> >> -#include <vlc_atomic.h>
> >>   
> >>   struct libvlc_media_t
> >>   {
> >>       libvlc_event_manager_t event_manager;
> >>       input_item_t      *p_input_item;
> >> -    vlc_atomic_rc_t   refcount;
> >> +    int                i_refcount;
> >>       libvlc_instance_t *p_libvlc_instance;
> >>       libvlc_state_t     state;
> >>       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 */
> >>
> > 
> > I can't find back the rational for reverting this commit, but with a708c988091d6bc770212f5deeaba45e2395f4b2 it seems to be required, since the refcount will be accessed from the preparser thread & the libvlc_media_parse_with_options thread, as can be seen here: https://code.videolan.org/videolan/medialibrary/-/jobs/487725
> 
> There's this thread
> https://mailman.videolan.org/pipermail/vlc-devel/2019-October/128362.html

Yes, following this thread the commit was applied, but I can't find the reason for it being reverted.

-- 
  Hugo Beauzée-Luyssen
  hugo at beauzee.fr