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

[Julien 'Lta' BALLET]

Hi Remi,

Thank you so much for such a quick reply ! It kicks ass


On Mon, May 26, 2014 at 12:09 PM, Rémi Denis-Courmont <remi at remlab.net>wrote:
>
>
>  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.


I was thinking in the beginning of basing my work on a serialization format
for input_item_t, but i though it would be too much maintainance.

I do not assume directories are serializable. The browse module receive a
stream_t when you are trying to access an API over http, but not when you a
trying to browse a folder. In that case, the module has to assume the
access role (access_browser, much like access_demux). The access_browse is
likely to be part of the same .so that the regular access


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


I haven't been able to observe a problem on Stop, but it doesn't mean the
problem doesn't exists. I'd really like to have examples of when it happen !
The browsing is done in a thread in case you are browsing a slow media or
remote service (like saturated nfs). Also, i've tried to think the
input/demux stuff was here for a good reason, assuming it was in good shape
and working, so that's why it borrows most of its behavior.

My very personal opinion, is that the core playlist should be flat and that
the tree structure and all browsing/discovery stuff should be done
separately. But i was wanting to reproduce the existing behavior of 'when
you play a folder, it plays what's inside the folder' and also wasn't
wanting to break everything :)

Commit log says you're one of the guys who knows the core best, so it would
really be helpfull to have a lead to a design that would be better in your
opinion and knowledge of the vlc internals while retaining the same / close
behavior for the users.



>
>
>  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 totally agree with that. But let's take it as a first step :)
On the other hand, i don't think you and other people on this mailing list
would have enjoyed receiving a patchset rewriting half of the world.

Before starting to work on this, i've been thinking about inotify
integration. New items can be appended to folders at every moment, old one
can disappeared, etc. Handling all those cases would push the item event
managers usage onto a new level. But i kinda felt it was over the scope of
a first patchset on this topic



> I don't think a boolean is sufficient state.


My initial work introduced more change to input_item_t, but i thought it
would be better to introduce minimal changes to such a core data structure.
Having at least the last time the item was browsed, when it was
created/refreshed would be a step toward handling thoses cases. If you
don't mind adding new fields inside of input_item, i'd be glad to add this
to the patchset and UI might beneficiate of it.


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

Obviously agreed


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


I'm intrigued. I'll bring some numbers on the table whenever i've the time
to do benchmark of it.

A middle solution would be to keep fs/playlist the way they are, and allow
for new modules to use browsing (upnp, bonjour, smb, etc.)


> introduce functional problems.


I guess you're pretty busy, but if you have time at some point to expand on
this, it'll be greatly appreciated and would help me submit something you
might find more suited


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

I'd like to thank you again for you quick answer and for the kind attention.

Looking forward to read from you,
Lta.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mailman.videolan.org/pipermail/vlc-devel/attachments/20140526/f25ee9d2/attachment.html>