[vlc-devel] [PATCH] Adding metadata support to web-plugin

[Pierre d'Herbemont]

On Sep 8, 2009, at 5:45 PM, Niles Bindel <zaggal69 at gmail.com> wrote:


>
> 3.  The deprecation and correction of the following functions:
>
>                 libvlc_media_list_remove_index
>                 libvlc_media_list_item_at_index
>                 libvlc_media_list_index_of_item
>
> Currently, these functions are not aware of potential sub-items

This is a feature. The naming clearly indicates that this is only  
running on the current media list *indexes*.

I don't see why you would remove those.

> cannot add/remove them from the list.

This is not true.

>  The replacement functions could
> reference the media_t pointer directly instead of an index which would
> ensure that the referenced media_t is the actual item desired and not
> just the one located at that index.

-1

While this can be done easily in a client application using the  
existing API, what you want to add does not allow the other way  
around. This is a loss of functionality.

> Additionally, there is also the
> problem of having a list of items and storing the references to their
> indexes in Javascript (which is currently artificially returned by the
> AddItem functions), then deleting an item in the middle of the list
> which invalidates all of the indexes beyond that item.  This requires
> the Javascript user to have to manually re-index those references.
> IMO, pushing all of this index managing responsibility onto the
> Javascript user is not only unnecessary, but is also confusing and
> error-prone.

Just expose an iterator at the Javascript level, or accessor around  
media lists. Just like arrays.

> As for the patch itself, I noticed that you create a media player
> instance (meta_retriever) and seem to only use it to ensure a media
> player can be created from the media.  I assume this serves some
> purpose, but it does seem rather odd to me that you would have to
> create a media player instance in order to retrieve the meta data from
> the media item.  I'm guessing whatever purpose this check actually
> serves could potentially be done in a more straightforward way.

Right, meta data preparsing is available from media_t and you don't  
need a media_player for that.

Pierre.