[vlc-devel] [PATCH 00/10] Item browsing system

[Rémi Denis-Courmont]

Le 2014-05-26 17:41, Julien 'Lta' BALLET a écrit :
> What do i mean by 'browsing system' ? There's two things:
>
>   - A new type of module: 'browse' module, whose job is to take an
>     input_item_t (and optionnaly a stream) as an input and to fill an
>     input_item_node_t as its output. Basically it's meant to read
>     folders, playlists, discovery protocol and APIs.

Unfortunately, that seems to assume that the directory object can be 
serialized. Typically, it is not, and often cannot be, e.g. a directory 
file handle coming from the filesystem access, or a node object from the 
UPnP library.

Note that input_item_t is purely (serializable) data.

>   - A (very) lightweigt clone of the input_thread_t to handle the
>     browse modules: browser_thread_t. The browser thread is 
> integrated
>     to the playlist loop, to have a consistent behavior with what
>     happens with input.

I don't see why that needs a thread in the core. It sounds like 
reproducing the design bug of the input/demux loop, which often causes 
the input thread to lock up, e.g. on Stop.

> How it works ?
>
>   - Browse modules have a very simple API: They expose only a
>     pf_browse function. They receive an input_item_t item to browse,
>     optionnaly a stream_t and a input_item_node_t to fill.

See above.

>   - There are two kind of browse module. access_browser and browser.
>     - access_browser are for browsing folders at a low
>       (system/protocol) level (local, ftp, smb folders).

Same problem.

>     - browser requires a stream_t and handle files/streams (API,
>       playlist files).
>
>   - Browsing thread looks like input_thread_t. While it carries most
>     of its looks and behavior (events, state, API), it is much much
>     simpler. He runs on it's own thread but will soon gain a
>     synchronous and simpler API for usage in UIs.
>
>   - Item gets a few flags (browsable/alread browsed) to handle node
>     going back and forth between input and browsing.

That seems naive. In reality, the content can change asynchronously. I 
don't think a boolean is sufficient state.

>   - When the playlist tries to play an item with the browsable flag
>     it starts a browser thread instead of an input one.

Typically, it is not not known whether a URI points to a directory or a 
file at the time the playlist starts, so that cannot work. Besides, 
there is a corner case where an URI changes from directory to file or 
vice versa.

And no, we cannot restart; that would be a massive performance 
regression (for filesystem or regular playlist files) and introduce 
functional problems.

>   - input modules (especially access) have the opportunity to mark
>     item as browsable, making the playlist retry them in the browser

See above.

>   - A rewrite of access/directory.c is included to demonstrate this.
>     A browser version of wpl playlist module is working, but needs 
> some
>     cleaning before submission.

I don't think it can work properly that way; see above.

-- 
Rémi Denis-Courmont