Max Kellermann writes: > On 2013/10/11 16:26, j...@dockes.org wrote: > > > > Hi, > > > > I have fleshed out the code inside ProxyDatabasePlugin.cpp so that it > > now works. > > Your patch is hard to read because it's all in one big undocumented > chunk. Please split the patch into smaller pieces and document the > API changes.
Hi,
Attaching the two patch parts (I use splitdiff for this).
The patch changes two files in all:
The first part changes the SongFilter class inside SongFilter.hxx to make
its items list accessible:
- Make the SongFilter::Item class definition public.
- Add two methods to the SongFilter::Item class to retrieve the value and
the fold_case qualifier.
- Add a call to retrieve a const ref to the SongFilter::items list.
While I understand that these details would be best kept private, the
changes are quite useful to allow for an efficient search of the remote mpd
database. I'll explain further, after finishing with the patch.
This is the only part of the patch which could be called an API change.
The second part of the patch changes the file db/ProxyDatabasePlugin.cxx.
This file was a skeleton, containing mostly empty methods. What the patch
does is just add implementation where there was none, there are very few
modifications of existing code (a few additional parameters to calls
internal to the file). Because the original file was almost empty, I think
that it would be simpler to look at the patched result than to discuss the
patch itself.
The existing ProxyDatabase plugin is not configured in the default mpd, and
not usable anyway. I am not claiming that the patched version is ready for
production, but it does supply a viable implementation for testing.
To come back to the more general issue, I have now experience in
implementing two Database plugins:
- The one in the patch, which uses the database from a remote mpd.
- A second one, which uses an UPnP MediaServer as the remote. This one is
currently working for browsing the Content Directory and playing songs
(accessed through the curl input plugin), but the search parts are not
implemented, mostly because of issues in the plugin interface. However
if someone wanted to give it a try, I was getting ready to supply the
current version. It would be interesting to try it with other
MediaServers, because I have only tested MediaTomb and minidlna at this
point.
The Database plugin interface currently assumes that a recursive walk of
the database can generally be done, and that it can be used for
searching. I did not modify this, because this would be an internal API
change, which obviously needs prior discussion, and which would probably
have some repercussions on the current local database code.
However, in the case of a remote mpd, it is highly inefficient to walk the
tree for performing a search, something like 100 times slower than a local
search.
For the "VisitUniqueTags" method, I was able to delegate the search to
the remote mpd, and this is what necessitated the opening of the
SongFilter class described above: I have to get at the search criteria
so that I can build a search for mpd_search_db_tags() to remotely
perform.
In the case of a song search, I appear to be bound, because the Visit()
API method seems to require a recursive tree walk and the songs are
filtered during the walk.
In the case of a remote UPnP MediaServer, a tree walk is mostly impossible:
the tree that the UPnP ContentDirectory presents is virtual, generally
arranged by tag hierarchy. There may be a subtree which represents the
physical filesystem, but as far as I know, this is not mandatory, the name
is not fixed (it's named "PC directory" in MediaTomb, something else in
minidlna), and there might be wormholes to the virtual tree lurking around,
for all I know. There may also be a directory which flatly contains all the
tracks.
I am not by far an UPnP standard expert. I found nothing about the general
tree structure during my cursory study of the Content Directory part, and
it's probably all up to the implementer; the "tree" might contain cycles,
and there is certainly huge duplication in general.
The standard does not seem to describe a minimal approach to walking the
tree, except if I missed it: if someone knows better, please make me right.
To make it short: the database proxy API should be changed to remove any
idea of a recursive tree walk, and to completely delegate search operations
to the plugin.
If more detail or explanations are needed, just ask, this was already quite
long...
Cheers,
jf
a_src_SongFilter.hxx.patch
Description: Binary data
a_src_db_ProxyDatabasePlugin.cxx.patch
Description: Binary data
------------------------------------------------------------------------------ October Webinars: Code for Performance Free Intel webinars can help you accelerate application performance. Explore tips for MPI, OpenMP, advanced profiling, and more. Get the most from the latest Intel processors and coprocessors. See abstracts and register > http://pubads.g.doubleclick.net/gampad/clk?id=60135031&iu=/4140/ostg.clktrk
_______________________________________________ Musicpd-dev-team mailing list Musicpd-dev-team@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/musicpd-dev-team
