[vlc-devel] commit: Qt4: store input_item_t* in plitem and handle metadata in model ( Ilkka Ollakka )

[Jakob Leben]

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;
>        PL_ASSERT_LOCKED;
>
>        /* 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. So then we *will* have the problem I was
writing about.

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.
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). 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 faults.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mailman.videolan.org/pipermail/vlc-devel/attachments/20090814/60294048/attachment.html>