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

[Jussi Kukkonen <jku linux intel com>]

On 03/11/2011 04:21 PM, Martyn Russell wrote:
> 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?

We discussed this a bit on IRC, but recapping here:

tracker:available is already used for the actual media items.

tracker:available currently cannot be used for datasources (in this case
the mediservers) as the domain is nie:DataObject. If tracker:available
was made usable for all rdfs:Resource, I'll use it.


> 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

Yep, Lionel already fixed this. I just forgot to merge.

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

These all sound good.

> 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";

Yes, so it seems. I was (am) not totally clear on how those should be
used... but using both seems to be the common way.

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

Yes, understood.

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

Thank you. I'll try to get back to you this week.

 - Jussi