[vlc-devel] commit: various modules: adjust to new playlist design (Jakob Leben )

Jakob Leben jakob.leben at gmail.com
Tue Feb 2 20:03:20 CET 2010


On Tue, Feb 2, 2010 at 5:50 PM, Pierre d'Herbemont <pdherbemont at free.fr>wrote:

> On Tue, Feb 2, 2010 at 5:33 PM, Jakob Leben <jakob.leben at gmail.com> wrote:
>
> > Look, how can you avoid the various node manipulation functions, when it
> is
> > the modules that have to construct an input item tree? Only they know how
> to
> > construct it, that is why they are there.
>
> This can be done with input_item_New() and input_item_AddSubitem().
> And a Flush function. Much more robust, lighter, and compatible with
> what we had.


Are you suggesting to add a new Flush function, that will mean the end of
adding subitems?
In the patch I posted previously I had a different API that used
input_item_BeingAddSubItems, then a series of input_item_AddSubitem, and in
the end input_item_EndAddSubItems. The latter is then equivalent to your
Flush function, if I understand you right. Fenrir commented on it, that it
is not very well maintainable, because the Begin or End functions might be
forgotten, when code is refactored. In response to that I made this API that
needs modules to construct an input item tree and then notify about it in
one single event. I think this is indeed more maintainable.

Think of it like that: maybe playlist demuxers could indeed pass over a
vlc_media_list_t or whatever it is called. input_item_node_t has the same
function. And as I said recently, I think using a tree structure already at
source of data is only good.

Moreover, we could use the same API for more versatile services discovery,
and it is a good ground as we can simply extend input_item_node_t if needed.



> > For the moment, for judging the new API, think that input_item_AddSubitem
> > calls are not there, because that will be removed. Or actually
> > input_item_AddSubitem will be removed and the version 2 will come in its
> > place.
>
> I got this. The naming is just wrong. First because of confusion with
> the previous one, then because there is no flush notion what so ever.
> You had to explain the difference to me, that prove that this was not
> rightly named.


First, the confusion was not ment to stay and was perceived by me as a minor
hassle, because the state of having both functions there was for me very
very temporary, only until a little bit more code is adjusted.

Second since input_item_AddSubItemTree has a notion of flushing, then why
doesn't input_item_AddSubItem? the name syntax is the same. And it only
needs a notion of flushing --in relation-- to other input_item_node_XXX
functions used in modules - which it does.
Well, maybe the word "Add" could be changed in --both-- functions to have
even stronger flush notion.




> >>>>>> So here you could help me out. You could convert your
> >>>>>> src/control/media.c to listen to vlc_InputItemSubItemTreeAdded event
> instead
> >>>>>> and process the incoming tree structure. <<<<<
>
> You see how much complexity it adds? I'd rather implement your API on
> top of the previous one. I think what you've done is great, but the
> module interface can be simplified.
>
Well, I have already stated above couple of reasons for preferring my API.

If you think making a small addition to src/control/media.c is too much
work, I can swap input_intem_node_t for media_list (or whatever it is
called) in my API, then there will be even less work to be done in
src/control/media.c


> Thanks, I hope we'll move forward.
>
> Pierre.
>

Well, I am still very open for discussion and argumentation, I do want to
listen and try to understand. I just haven't heard something that would
overrule my arguments yet.

I would also appreciate very much if others joined into discussion, so it is
not just our two heads clashing between each other :)

Cheers!
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mailman.videolan.org/pipermail/vlc-devel/attachments/20100202/82b1b178/attachment.html>


More information about the vlc-devel mailing list