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



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



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