[vlc-devel] [RFC] Nth playlist rewrite

[Rafaël Carré]

On Sun, 2008-03-16 at 19:32 +0100, Pierre d'Herbemont wrote:
> Selon Rafaël Carré <funman at videolan.org>:
> 
> > > That is a nice way to make sure that there is no circular dependency
> > between
> > > playlist and input, given that from a conceptual point of view, the
> > playlist
> > > should be kept away from the input. The playlist should manipulates input,
> > and
> > > not the other way around.
> >
> > This is exactly the "other way around" which happens with "sub items".
> 
> No, this is done through callbacks/events. The playlist is an input client. So
> we break the circular dependency. (That's why we can deal with two differents
> playlist subsystems at the moment).

Still, callbacks/events or directly modifying the struct is the same.

> > > > > Moreover, the input_item_subitem_added() is only an event handler for
> > the
> > > > > input_AddSubItem call. It can't work well with current playlist (hence
> > the
> > > > nasty
> > > > > hack) but do work well with libvlc-control. So how do you plan to
> > remove
> > > > that
> > > > > input_AddSubItem abstraction?
> > > >
> > > > By statically linking each input_item and playlist_item, i.e. associating
> > > > them for life.
> > >
> > > Don't you feel that playlist_item should have an input_item and not the
> > other
> > > way around?
> >
> > Why would an input move from one playlist_item to another ?
> > An input should have its own playlist_item forever.
> 
> Well, a playlist_item holds a input_item. Not the other way around. Because here
> the playlist_item is an input_item subclass. (A playlist item is a specialized
> input_item, right?). You now just have to manipulates playlist_item.

No not right. At the moment it's clearly separated.

> This will allow you to make sure you separate the playlist from the rest.
> 
> Of course we can just keep an array with the playlist_item<->input_item
> association to do the actual subclassing. This may ease the work. But really
> sounds ugly.

I think you need to make a schema, to show what you think (ASCII is ok).

> > > I am really trying to understand what you want to do. The only way that I
> > could
> > > agree with you would be:
> > > - Have each input_item to have a list of its subitems.
> > > - Not to put any more playlist stuff in the input.
> >
> > Does that mean having both a hierarchy in the playlist, and one in the
> > input ? Freaky.
> 
> Not necessarily. It does mean that you put all the playlist browsing in the
> input_item. You don't have to duplicate.
> 
> Indeed, doing that means exactly the same as having a playlist_item for each
> input_item.
> 
> Because the idea we are facing is that, an input_item is a playlist_item, in the
> sense that it can have children.

Well, I've some trouble here.

Do you want to merge some playlist code in input ? (aka the hierarchy)

> > Well, we could remove the hierarchy from the playlist Model (internal
> > structure) and only use input hierarchy.
> 
> So an idea would be:
> input_item
> -> subitems() returns a struct holding the subitems. A media_list for instance.
> 
> I am more or less fine with it, as long as the media_list struct is light, and
> clearly separated.
> 
> > That means:
> > 	1/ Keep a hierarchy only for the top playlist, i.e.
> > 		PLAYLIST
> > 			- local
> > 			- media library
> > 			- services discovery 1
> > 			- services discovery 2
> 
> To me the PLAYLIST top node should be interface dependant. 

It is already.

> And the name PLAYLIST
> is not really close to the reality. This is not a playlist but an array of
> services that provides media. Media Providers.

We don't care about the name, atm it's p_root.

> > And each of these nodes would have inputs:
> > directory/
> > 	|-file 1 (linked to directory's input)
> > 	\-file 2 (linked to directory's input)
> > audio-cd
> > 	|-track1
> > 	\-track2
> > stream1
> > stream2
> > youtube_http_url
> > 	\-flv
> >
> >
> > In this theory we have 5 playlist_item:
> > directory, audio-cd, stream1, stream2, youtube_http_url
> 
> I am lost. Why aren't those also plain input_items (each playlist_item should
> contains, at least, an empty input_item, if there is a subclass link between the
> two.)?

Me too, as already told, I don't do OO.

> > and 10 input_item, 5 of them are not associated to the playlist in any
> > way.
> 
> ok, I just realized. Let's dump the word playlist again. It's way too vague.
> 
> Each input_item has a media_list or whatever containing its subitems.

A media_list of input_item, right ?
Why do we need more than a simple array ?

> > BUT that means that for the GUI to manage the playlist View, it has to
> > mess with input code.
> 
> The GUI asks for a media_list's media_list_view and you can iterate through it.
> You get input_item in the end. Because that's our lighter unit. You can then
> query its meta an so on. That seems sound to me.

How many types as a gui to know to be able to display the _playlist_ ?

> > So I don't see how we can remove the circular dependancy here.
> 
> Well, currently the circular dependency is gone. Given that we are using
> callback, and that the playlist is an input_item client.

maybe (see above comments)

> But as long as we use a light object that describe the relation there is no
> prob.
> 
> > I don't know OO vocabulary.
> >
> > Do you mean :
> >
> > struct
> > {
> > 	input_item_t 	*p_item;
> > 	int		i_subitems;
> > 	input_item_t	**pp_subitems;
> > } media_descriptor;
> >
> 
> I meant:
> media_descriptor contains
>    media_list_t * subitems()
> and has probably a private input_item.

I still didn't read the code.

> > The next step is to propose a design (a drawing/picture) of what
> > playlist should do, how it communicates with other parts of the code, to
> > clear out any misunderstanding.
> 
> Basically libvlc API work that way:
> 
> media_descriptor
> - subitems() [returns a media_list] [Note, We don't really need that and it
> creates a circular very light circular dependancy, but it is was convenient back
> in the time when there was no media_list_view]

Then remove it.

> media_list
> - get_flat_view() [returns a media_list_view]
> - get_hierarchical_view() [returns a media_list_view]
> 
> [setters]
> - add/insert_item( media_descriptor )
> 
> [getters, do we really need them?]
> - item_at_index() [returns a media_descriptor]

Well we can get it directly from the array, we do C don't we ? ;)

> media_list_view
> - item_at_index()
> - children_at_index()

what is the diff between item & children ?

> media_list_player
> - set_media_list()
> - play()
> - pause()
> - etc...
> 
> I really think that this way we really separate the essential part of the
> playlist.
> 
> Pierre.

-- 
Rafaël Carré <funman at videolan.org>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 189 bytes
Desc: This is a digitally signed message part
URL: <http://mailman.videolan.org/pipermail/vlc-devel/attachments/20080316/6636103d/attachment.sig>