Re: [Tracker] RFC: UPnP mediaserver (ContentDirectory) miner



On 10/02/11 12:37, Jussi Kukkonen wrote:
Hi all,

Hi,

I'm writing a UPnP ContentDirectory (mediaserver) miner for tracker. See
code at
    git://gitorious.org/tracker-upnp/tracker-upnp.git
    http://gitorious.org/tracker-upnp/tracker-upnp
I'm currently working on it as a independently installable component (as
I need it for tracker 0.10 in MeeGo and assumed I'm way too late to get
it in the release). It would still be nice to get this into tracker git
(better sooner than later), which is why I'm writing this email. Let me
know if you have comments on the idea / implementation.

Thanks for writing this miner :) my comments:

1. Why use upnp:available when tracker:available exists and a lot of queries already use that?


2. The miner object needs to be GInitiable, see how we do this with the flickr miner if you need some hints:

http://git.gnome.org/browse/tracker/tree/src/miners/flickr/tracker-miner-flickr.vala


3. Coding style, for vala files, we keep the initial function bracket on the same line as the function parameters. We also don't have parameters on separate lines as a general rule (unless we breach 80 columns). There are cases where there is no space before/after {} for some property get/sets. There are also situations where the if () { } else if () { } else { } brackets are not all in place. For details see:

  http://live.gnome.org/Tracker/Documentation/CodingStyle#Vala


4. We should handle Ctrl+C gracefully (which the Flickr miner seems to do too).


5. The state changes should be debug messages I think so identify what's going on when running it more clearly.


6. There is an indentation issue here, but also I think this should be an nmm:MusicPiece AND nfo:Audio...

                        else if (item.upnp_class.has_prefix ("object.item.audioItem"))
                        nmm_type = "nmm:MusicPiece";


7. There are some TODOs which would need fixing, like:

/* TODO add a nmm:Artist */
/* TODO add nmm:MusicAlbum and link to that */

We don't allow extractors to insert with links to missing resources generally.

--

If you can amend these issues, I will do a final test and sanity check the SPARQL used before migrating it into the tracker code base.

Thanks for all the hard work put in here guys,

--
Regards,
Martyn



[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]