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

[Rémi Denis-Courmont]

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.

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/