[vlc-devel] [PATCHv3 6/7] lib: media: copy the input item

[Thomas Guillem]

On Wed, Sep 30, 2020, at 18:52, Rémi Denis-Courmont wrote:
> Le keskiviikkona 30. syyskuuta 2020, 11.05.46 EEST Thomas Guillem a écrit :
> > On Tue, Sep 29, 2020, at 17:27, Rémi Denis-Courmont wrote:
> > > Le tiistaina 29. syyskuuta 2020, 11.10.04 EEST Thomas Guillem a écrit :
> > > > > > -    return libvlc_media_attach_input_item( p_md, p_input_item );
> > > > > > +    input_item_t *p_item_copy = input_item_Copy( p_input_item, NULL
> > > > > > );
> > > > > > +    if( p_item_copy != NULL )
> > > > > > +        return libvlc_media_attach_input_item( p_md, p_item_copy );
> > > > > > +    return NULL;
> > > > > 
> > > > > Won't that "unshare" item updates from parsing and from playing?
> > > > > If so, is that expected by apps?
> > > > 
> > > > Yes it will, but only for copy of medias.
> > > > 
> > > > If you create a media, play or preparse it, you will get updates.
> > > 
> > > That sounds like a regression? I'd expect updates to be shared between
> > > preparsers, media players and, if that's where the item comes from, SD. At
> > > least that's how it always worked. While it has its downsides, I suspect
> > > that the opposite approach will turn out worse.
> > 
> > It's still the case, except for SD (but we don't have any modules that are
> > updating items).
> 
> AFAIK, it is intended that SD can update their items. I don't think the fact 
> that you can't find one case in-tree right now is a reason to discard the 
> possibility.
> 
> In fact, I would hardly be surprised if some SD plugins turned out to 
> incorrectly deleting and recreating items already now, when they really should 
> be updating them.
> 
> > User1 create one media1, play it, then duplicate it into media2 and pass it
> > to User2. If User2 play media2, you don't want the User1 to receive updates
> > from it. This patch set is also fixing this problem (but it could also be
> > fixed independently of this patch set).
> 
> If there is an API to copy media that does not copy media, that API is buggy, 
> regardless of this patchset.
> 
> > > I don't really understand why you copy items.
> > 
> > In order to have an immutable owner.
> 
> And I don't understand why you have an immutable owner private data.
> 
> The owner ought to know to set its private data pointer before its callbacks 
> can be triggered, so there's no race (as I pointed out in the review of the 
> first version). And in any case, it has to know when the callbacks can no 
> longer be triggered to clean up the private data.

But this is what I did in my first version, no ?
As a fix for the first version, I could set the owner data in the same function that setup callbacks of input_items (from libVLC).
Is that OK with you ?

> 
> Immutability is sufficient to achieve the first point but I don't see why it's 
> necessary or why you chose it.
> 
> -- 
> Rémi Denis-Courmont
> http://www.remlab.net/
> 
> 
> 
> _______________________________________________
> vlc-devel mailing list
> To unsubscribe or modify your subscription options:
> https://mailman.videolan.org/listinfo/vlc-devel