[vlc-devel] commit: Qt4: store input_item_t* in plitem and handle metadata in model ( Ilkka Ollakka )
pdherbemont at gmail.com
Fri Aug 14 12:43:56 CEST 2009
On Aug 14, 2009, at 12:19 PM, Jakob Leben wrote:
> On Fri, Aug 14, 2009 at 11:11 AM, Pierre d'Herbemont <pdherbemont at gmail.com
> > wrote:
> But imagine that in the time when we don't hold the lock the other
> thread adds a child to the playlist item, so changes it's
> pp_children field, but then unreferences both the item and its new
> child, before we could add a reference to the child! The former item
> will still exist as we had a reference to it, but with wrong
> reference to a child that got already deleted, as we didn't add a
> reference for it.
> That's one of the weakness of the playlist. However, this won't
> happen as playlist_item are only freed when the playlist is freed.
> See src/playlist/item.c
> int playlist_ItemRelease( playlist_item_t *p_item )
> /* For the assert */
> playlist_t *p_playlist = p_item->p_playlist;
> /* Surprise, we can't actually do more because we
> * don't do refcounting, or eauivalent.
> * Because item are not only accessed by their id
> * using playlist_item outside the PL_LOCK isn't safe.
> * modules do that.
> * Who wants to add proper memory management? */
> uninstall_input_item_observer( p_item );
> ARRAY_APPEND( pl_priv(p_playlist)->items_to_delete, p_item);
> return VLC_SUCCESS;
> As the comment in the above code suggests, the point of adding
> reference counting to playlist items is exactly that they can be
> freed sooner then when the playlist is freed. And I guess it is
> desirable so, instead of just hanging on to the current behavior.
It is the point of my mail, right.
> So then we *will* have the problem I was writing about.
For sure, there will be a synchronization issue between the playlist
data and the view.
> However, now I think of a possible solution: even if both parent and
> it's children are being removed, they shouldn't be just freed, but
> their internal pointers (parent -> children, child -> parent) should
> still be adjusted.
What is to be freed? The playlist_item? If there is someone holding a
reference to it, it won't happen with refcounting. With current
implementation those pointers will also always be valid
> This way when control in a qt interface returns from a view to our
> implementation of a model's function and it uses a QModelIndex with
> a pointer to the parent item, both that pointer and it's internal
> pointer to children are valid (only the pointer to children is NULL).
If you use refcounting they will *alway* be valid. Again, with current
implementation those pointer will also always be valid.
> This way we can still serve the view's request for data about the
> parent, even if it changed since it obtained the QModelIndex. I
> guess qt views should be (cosmetically) protected against this
> change of data, since the documentation states that QModelIndex is
> only temporary. The main concern is just to prevent segmentation
This is addressed by refcounting, and proper ownership. You are not
trying to address segmentation fault but synchronization, unless I am
-------------- next part --------------
An HTML attachment was scrubbed...
More information about the vlc-devel