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

[Besnard Jean-Baptiste]

Niles,

On mar., 2009-09-08 at 10:45 -0500, Niles Bindel wrote:
> 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).

Maybe media list is not the best place to put this metadata "getter" but
it was intended to "scan" a media list content in order to generate a
playlist with metadata. The main advantage of putting it here is to
avoid code repetition in the plugins. It can be used without a media
list player this is why I did not put it here.


> 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.
Yes those index are hard to handle at API level but the idea of 
index is interesting from the scripting language side because 
it is easy to "crawl" the media list with item count. Replacing it with an 
iterator would offer the same level of simplicity and allow the use of
other data structures in the media list.

> 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), 

It may be interesting to reference a media_t pointer for the currently
playing item this would allow the retrieval of it's true index by linear
search even in case of insertion//deletion. For the rest of the items
this would make things more complicated. Direct item search is not so
used at least by me.


> 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.

Yes there is the "gap" problem. In my opinion we have to maintain a
continuous playlist on the javascript side because it is very easy to
use. In JS indexes are not unique identifiers, and a deletion//insertion
will change the following items' index for me it is acceptable if the
currently playing item index is held by an unique Identifier.

> 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.

It is always good to discuss and enhance things.

> 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.

Thank you for pointing this out, this is a mistake, I removed it
from the attached patch. I promiss to stop watching TF1 on TV
while programming ;)

Patching the plugin may not be a good idea if you plan to
rework the playlist management scheme, for me this patch won't be
too hard to adapt. I used playlist_get_current_index and
libvlc_media_list_item_at_index  so if there are changes they will be
easily adapted to this patch unless you drop or update the media_list
thing completely. 

> I'll grant that the indexes currently allow a fast way for finding the
> items in the list, but as we discussed previously in another post,
> this should probably be turned into a tree implementation with an
> iterator anyway.  As such, a movement away from using indexes to
> locate items in the list would be an appropriate first step in that
> direction.


Switching to a tree in media_list may be a good idea particularly for
subitems but you'll still have to "flatten" the tree to get a media list
view with something like a DFS and you will lose the direct indexing for
an iterator. Media list player will have to be modified to take this
into account maybe by removing the media list path. The media list
player will be in charge of populating the currently playing node with
the subitems as child. 

Regards,

Jean-Baptiste BESNARD.

-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-Adding-a-metadata-getter-to-libvlc-media-list.patch
Type: text/x-patch
Size: 5750 bytes
Desc: not available
URL: <http://mailman.videolan.org/pipermail/vlc-devel/attachments/20090909/a1c8978e/attachment.bin>