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

[Pierre d'Herbemont]

On Tue, Feb 2, 2010 at 9:12 AM, Jakob Leben <jakob.leben at gmail.com> wrote:
> On Tue, Feb 2, 2010 at 8:50 AM, Pierre d'Herbemont <pdherbemont at free.fr>
> wrote:
>>
>> Well, input_item_AddSubItem2 is non obvious. Very suspicious.
>>
>> +    input_item_node_t *p_subitems = input_item_node_Create(
>> p_current_input );
>>
>> Why do you need this? Couldn't you emulate this with
>> input_item_AddSubItem()?
>>
>> It seems that this new API is much more complicated than it was
>> before, and there is no reason why. Mostly everything could be done on
>> top of the previous API.
>
>  Well, to be honest, I took the option of a flat playlist as a must. Based
> on this assumption, after thinning the playlist to not store separate tree /
> flat hierarchy, we need to know when an input item _stops_ adding subitems
> to implement the flattening properly.

Ok. This is one call.

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.

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.

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

Thanks for the explanation,

Pierre.