Hello,
I've meant to post these notes for a long time, but got lost on the
way. I've read all the code in nesciens' coll2 branch, although I
haven't reviewed the main commit in detail yet (query generation,
token, sources & so on).
Notes are semi-random, so bear with me. It probably won't make much
sense, unless you're nesciens.
- xmmsv_coll_add_order_operators could apply apply xmmsv_t list type
restriction. re:FIXME on how to throw clientlib errors, I suppose it
should print the error to stderr, since it's a bug in the client
that passed wrong arguments to the clientlib (stderr > silent error).
- xmms_collection_query_medialist uses a _string_ query specs (JSON),
that's really nasty (but predates xmmsv_t, so that explains
it). Still, it feels like xmmsv_t should be used instead for this
kind of stuff, possibly with helper constructor functions.
- in xmmsc_coll_query_ids, why query coll -> mediaset -> order,
i.e. why apply the mediaset operator? to remove duplicates?
- don't ask the client to strtol all numeric properties from strings!
we should maybe use some heuristic server-side to determine what
type to wrap in the xmmsv_t. could use the content and/or hardcoded
or even dynamic mapping of property->type for that.
- what's the deal with plugin sourceranks?
- collparser: autofilter
* COMPARE/MATCH: applied on what properties? "all"? doesn't seem to
be just artist/album/title?
- collparser: TOKEN is not used when doing a compare with a given
property (binary operator)?
- collparser: # and /
* / always references active playlist, cannot ref another?
originally, the idea was that PLAYLISTNAME/SEQ selects positions
identified by SEQ in PLAYLISTNAME (defaults to active playlist).
* # always uses id matching instead of IDLIST, makes sense for large
or unbound ranges, but does it for single matches ("#1")?
* # orders each range, rather than the whole result?
- how are multiple ordering applied (e.g. album, artist, etc)?
Main Coll2.0 commit:
- why have both xmmsc_playlist_add_medialist and
xmmsc_playlist_add_idlist?
- INTERSECTION_ORDER vs INTERSECTION - is that an extra operator we
agreed on doing implicitely?
- why the operand type ID? couldn't COMPARE be used for that?
- operation == "=", "<", etc: could use a nicer or more verbose value?
- global idlist register:
* not very nice, though probably not formally a problem..
* is it ever pruned?
* what do idlist ids (index in reg) correspond to?
- query_ids was removed (replaced by _medialist_ids?), but QUERY_IDS
still in the code?
- would it be possible to split:
* tokens
* source prefs/rank
* collquery
* idlist vtable
into subcommits? not worth it if you need to recreate entire stubs
if they are interdependent, but this main commit is just scary.
- database:
* why Media vs MediaBackend (aliases?) ?
* need 4 indices for so many combinations? as we noticed, indices
take it shitloads of space, so carefully picking those that are
needed would be better.
* performance of triggers on insertion/update/delete?
* avoid duplicated queries in schema and upgrade (reuse var)
* token table: never pruned on value changes? (could be expensive)
- static void udata_set_idlist (tree_node_t *node, xmmsv_coll_t *coll);
declared twice.
- xmms_collection_is_medialist uses validate without locking the DAG,
is that okay?
Side-notes:
- we should fix the IPC so it sends colls as a DAG, not a tree.
- use xmmsv_t for coll attributes?
(e.g. support list of values or fields to compare?)
That's all for tonight!
nesciens, could you also tell us what's your roadmap for working on
getting coll2 ready for merge, whether you have time to do it, etc?
Awesome work in any case, I can't wait to get low-level details
finalised so that we can refine the API and get things ready for
merge!
nesciens++
--
Sébastien Cevey / inso.cc
" There ought to be limits to freedom. "
George W. Bush
--
_______________________________________________
Xmms2-devel mailing list
Xmms2-devel@lists.xmms.se
http://lists.xmms.se/cgi-bin/mailman/listinfo/xmms2-devel
