[vlc-devel] [PATCHv3 4/7] lib: media_discoverer: compare with the original item

[Thomas Guillem]

On Tue, Sep 22, 2020, at 17:57, Rémi Denis-Courmont wrote:
> Le tiistaina 22. syyskuuta 2020, 15.09.55 EEST Thomas Guillem a écrit :
> > Since a libvlc_media_t won't hold the original item anymore, but a copy,
> > we need to store the original pointer for the
> > services_discovery_item_removed comparison.
> > ---
> >  lib/media_discoverer.c | 4 +++-
> >  lib/media_internal.h   | 3 +++
> >  2 files changed, 6 insertions(+), 1 deletion(-)
> > 
> > diff --git a/lib/media_discoverer.c b/lib/media_discoverer.c
> > index 6fcab48ecad..6dd0e7c718d 100644
> > --- a/lib/media_discoverer.c
> > +++ b/lib/media_discoverer.c
> > @@ -67,6 +67,7 @@ static void services_discovery_item_added(
> > services_discovery_t *sd,
> > 
> >      p_md = libvlc_media_new_from_input_item( p_mdis->p_libvlc_instance,
> >                                               p_item );
> > +    p_md->p_weekref_item = p_item;
> 
> That does not seem right, unless you yield the item.

I thought I didn't have to hold the item since SD modules were already holding them, but some SD modules are not.
So, indeed, I have to hold it.

By the way, this patch means dual allocations of SD items. I think it's worth it.

> 
> Also we_a_k.
> 
> > 
> >      if( parent != NULL )
> >      {
> > @@ -123,7 +124,8 @@ static void services_discovery_item_removed(
> > services_discovery_t *sd, {
> >          p_md = libvlc_media_list_item_at_index( p_mdis->p_mlist, i );
> >          assert(p_md != NULL);
> > -        if( p_md->p_input_item == p_item )
> > +        assert(p_md->p_weekref_item != NULL);
> > +        if( p_md->p_weekref_item == p_item )
> 
> This only works if you keep a reference. Otherwise, it could as well be a non-
> concurrent item that just happens to be allocated at the same address on the 
> heap.
> 
> >          {
> >              libvlc_media_list_internal_remove_index( p_mdis->p_mlist, i );
> >              libvlc_media_release( p_md );
> > diff --git a/lib/media_internal.h b/lib/media_internal.h
> > index e4d0d03faec..cf6301ac8e3 100644
> > --- a/lib/media_internal.h
> > +++ b/lib/media_internal.h
> > @@ -42,6 +42,9 @@ struct libvlc_media_t
> >      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;
> > 
> > +    /* Week reference of the original item, used for pointer comparison */
> > +    const input_item_t *p_weekref_item;
> > +
> >      vlc_cond_t parsed_cond;
> >      vlc_mutex_t parsed_lock;
> >      vlc_mutex_t subitems_lock;
> 
> 
> -- 
> レミ・デニ-クールモン
> http://www.remlab.net/
> 
> 
> 
> _______________________________________________
> vlc-devel mailing list
> To unsubscribe or modify your subscription options:
> https://mailman.videolan.org/listinfo/vlc-devel