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

[Jakob Leben]

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

>
> Also, some item may add subitem at some point. I personally think that
> we shouldn't need this grouping function as this is an optimization
> that could be done in the client side.
>
> > The most efficient design was to let playlist demuxers actually transmit
> > over the whole tree of subitems that they create at the same time.
> > input_item_node was a logical step. And _Create, _Append, _Delete are
> there
> > for manipulation of the input item tree, before it is actually sent over
> > with input_item_AddSubItemTree.
>
> I would really prefer a single call such as
> input_item_SubItemShouldBeProcessed(). What do you think? Do you
> really need to expose all the input_item_node thing at the module
> level? What's bothering me is that you need the node thing and the
> input_item_AddSubitem2(), which shows that something is wrong there,
> or at least not complete/mature.
>

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.

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.

Now, I don't need the node stuff AND input_item_AddSubitem2, it is one or
the other. If you checked input_item_AddSubitem2, you'd see that it is just
a wrapper that creates a input_item_node and adds one subnode only and then
emits the same event as input_item_AddSubItemTree. It is just a helper to be
used INSTEAD of input_item_AddSubItemTree, for brevity of code.


>
> My idea behind keeping this simple is that we really want modules API
> to be simple and easy.
>
> > To be handy, I also implemented input_item_AddSubItem2 for demuxers that
> > create only one single subitem - there is couple of them;
>
> You should rename it as input_item_SetForwardItem() then, or something
> that indicate that it will contain only one item. Adding a new call
> that fit those modules seems a nice idea. The name is not so good
> though.
>

It is the singular form of "Item" in the name that says it will we only one
item (as a difference from "Tree" in the other function).



>
> > but as a
> > difference from input_item_AddSubItem, the version 2 emits the same event
> as
> > input_item_AddSubItemTree.
>
> They should be merged then.
>
> > I kept the input_item_AddSubItem everywhere, so everything works also
> where
> > the old event is being observed, for example in vlc_media_xxx - that is,
> > until those places are converted to observe the react to the new input
> item
> > event. Then we can remove the number 2 next to function name...
>
> To me, it looks like something is wrong here. You should merged the
> current event with your architecture.
>
>
The point is, the reaction to this new event, or the old event is different,
they mean different things. Thus, since the input_item_AddSubItem2 emits the
new event while input_item_AddSubItem emits the old one, they also mean a
different thing. Namely, input_item_AddSubItem2 mean that the demuxer has
stopped creating subitems and it created one single subitem only. While
input_item_AddSubItem meant that the demuxer could still continue sending
the same event more and more... That is why to merge the stuff, we have to
first erase all code that listens to the old InputItemSubItemAdded event,
and make it process the new event.

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

Thanks for patience and I hope that my ideas meet understanding,

Best regards
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mailman.videolan.org/pipermail/vlc-devel/attachments/20100202/70552b90/attachment.html>