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

[Niles Bindel]

Jean-Baptiste,

I've recently spent some time working on the plug-ins and the media
list player and I feel that there are a few things that should be
resolved before there are too many patches like this.  I believe the
real problem here is that the media_t class is not exposed to the
Javascript API.  Exposing this object to the API would allow for a
better way to expose the meta data and help resolve a number of issues
in the current API, including the following:

1.  Direct access the meta data and any other data (like the
sub-items) related to the media_t without routing the call through the
media list (which from a software design perspective should solely be
a container and not know anything about the contents of the media_t).

2.  Proper identifiers to be used by Javascript for a media_t object
instead of the arbitrarily and potentially error prone array index
that is currently being used.

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 and
cannot add/remove them from the list.  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.  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.

I realize this is probably more than you expected to hear, but I feel
as though this broader change would be better for the API and browser
plug-ins in the long run and I thought this would be a good
opportunity to discuss this further.

========================================================================================

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.

My 2 cents,

Niles Bindel