Review Request: GSoC : Nepomuk plugin for Amarok

Wed Sep 5 08:58:51 UTC 2012

> On Sept. 4, 2012, 6:11 p.m., Matěj Laitl wrote:
> > src/core-impl/collections/nepomukcollection/NepomukCollection.cpp, line 124
> > <http://git.reviewboard.kde.org/r/106042/diff/7/?file=83644#file83644line124>
> >
> >     This works for now, but cannot one configure which folders Nepomuk indexes? You should use these for this call. (not a merge-blocker)
> 
> Edward Hades Toroshchin wrote:
>     Phalgun, add a // TODO comment here, so that this can be investigated in the future.
> 
> Bart Cerneels wrote:
>     You shouldn't try to change nepomuk's indexed folders. Instead create an opt-in or opt-out list for the content that it does need to include in the collection. i.e. add an additional query join (or equivalent, I'm not well versed in SPARQL) including/excluding based on file location.
> 
> Phalgun Guduthur wrote:
>     I have been meaning to ask this for a while. 
>     What does isDirInCollection actually do? Is this the function that checks if the track lies in the folder that user has selected for Amarok to index(or track) for songs? 
>     If it is so, shouldn't the Nepomuk collection also follow those directory limits to query tracks from? 
>     
>     Right now, tracks from all of the Nepomuk index is being shown to the user. 
>     
>     For example, in my box, I have set Amarok to show tracks only from ~/Music. So the SQL collection shows about 300 tracks. 
>     But the Nepomuk Collection shows about 2000 tracks, all those 2000 tracks are from the whole of the Nepomuk index.
> 
> Edward Hades Toroshchin wrote:
>     Guys, guys, hold on a second, I have a question.
>     
>     Whut?
>     
>     isDirInCollection is a method that tells if a dir is in the collection. It does not change anything for the collection, it just tells, if a specific directory is covered with it. There is no semantics of "modifying the collection paths" in Amarok. There is only setting of collection paths for "Local collection", which is basically Amarok's own scan mechanism.
>     
>     > Is this the function that checks if the track lies in the folder that user has selected for Amarok to index(or track) for songs?
>     
>     No, of course not, it wouldn't have this signature in this case.
>     
>     > Right now, tracks from all of the Nepomuk index is being shown to the user.
>     
>     That is the correct behavior.

But isn't the Collection not supposed to show tracks from all over the system? It should adhere to the directories that the user has selected in Settings -> Configure Amarok -> Configure Local Collection. 

Maybe I could have a separate settings dialog for Nepomuk plugin ( like the one where you enter your credentials for last.fm ) where the user can select the folders to fetch the tracks from. 

- Phalgun

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/106042/#review18515
-----------------------------------------------------------

On Sept. 4, 2012, 4:51 p.m., Phalgun Guduthur wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/106042/
> -----------------------------------------------------------
> 
> (Updated Sept. 4, 2012, 4:51 p.m.)
> 
> 
> Review request for Amarok, Edward Hades Toroshchin, Vishesh Handa, and Matěj Laitl.
> 
> 
> Description
> -------
> 
> Nepomuk plugin for Amarok.
> 
> Almost all of the code changes can be found in src/core-impl/collections/nepomukcollection/*
> And a minor change in src/core-impl/collections/support/MemoryMeta.cpp
> 
> Code builds and after Nepomuk plugin is activated in the "Settings" dialog, Nepomuk Plugin comes into play and queries all the tracks in your machine. The query is not 'that' fast and might take several seconds depending on the number of tracks in your box. 
> IMPORTANT : Make sure Nepomuk is enabled if you want to give the plugin a spin. 
> 
> ===========================================================================
> Commit messages : 
> 
> 
> Code formatting changes
> 
> 
> Removed unnecessary documentation
> 
> 
> Set right Logger
> 
> The logger shows the progres of the collection updation correctly now.
> The number of tracks were calculated using a small sparql query
> 
> removed local variable
> 
> 
> Removed secondary Nepomuk meta hashmaps.
> 
> Left all the duplicate object checking to MapChanger.
> The CMakeLists logs the presence of Nepomuk
> Initialized the labellists in NepomukTrack constructor
> 
> Code formatting changes
> 
> 
> fixing strohels nitpicks
> 
> 
> Code formatting changes
> 
> 
> vHanda nitpicks in review board
> 
> 1. Moved soprano model instanciation after query string construction
> 2. QString -> QString::fromLatin1() for sparql query string
> 3. Nested album gains into album sub query
> 4. removed extraneous code that cropped up. (nao::has)
> 5. revamped tags query as I was only querying uri and not names of tags
> 
> If nepomuk is not found, the user gets a logger message informing the same
> 
> 
> Code formatting changes to fit 90chars per line
> 
> 
> The progress bar says which track is being updated
> 
> 
> The Includes order for NepomukCollection was fixed
> 
> 
> Make Nepomuk dependency optional
> 
> 
> Modified debug string to be concise and meaningful
> 
> 
> Formatting changes and documentation - part 2
> 
> Also deleted the .autosave file that creeped along the other commits
> 
> Documentation + Formatting - Part 1 ( meta classes )
> 
> 
> Added year implementation
> 
> The sparql metadata query is complete with the major metadata.
> Lyrics and Album art is yet to be done.
> 
> Code builds.
> 
> Code still in Nepomuk 1
> 
> Implemented label metadata ( tags ) in sparql enumeration
> 
> 
> Code formatting changes
> 
> 
> SPARQL query implemented instead of Nepomuk::Resource API
> 
> Hefty changes in the core of the background job of the query of tracks
> and subsequent enumeration.
> 
> Initially a manual sparql query which queries for all properties of the
> track and other meta data like artist, album etc.
> 
> The meta HashMaps are changed to incorporate Resource URIs and Labels
> instead of resources themselves, as keys.
> 
> Year and labels ( tags in nepomuk ) are yet to be implemented
> 
> Added a resource data member to NepomukTrack
> 
> The resource was needed to store and extract ratings and score. It gets
> constructed in the constructor using the QUrl
> 
> Removed a unnecessary debug line
> 
> 
> Modified NepomukTrack constructor
> 
> The constructor now takes in the uri of the track resource as an argument
> instead of the Nepomuk::Resource itself.
> 
> Manual sparql query : NepomukTrack changes
> 
> To support the usage of manual sparql query, a lot of support functions are
> added to NepomukTrack. A lot of these are setter functions which were
> previously set from the resource of the track.
> 
> This was removed as a optimization technique as any use of Nepomuk::Resource
> ::getProperty() consumes more cycles.
> 
> Year gets populated now
> 
> The query uses nmm:releaseDate and if it finds a date, populates
> the track using a NepomukYear object.
> 
> added NepomukTrack::setYear()
> 
> 
> Revamped NepomukYear like other meta objects
> 
> The constructor takes in a string argument
> A tracks() method is implemented which returns all tracks of that year
> 
> Add label details to track during query and enumeration
> 
> Like other meta parameters, labels are now added to the NepomukTrack object
> 
> Removed unused functions in NepomukLabel
> 
> tracks() was not a function that was defined in Meta::Label. It is mostly
> not needed by any class/GUI. Wasn't being used, so what the hell.
> 
> Added and implemented label specific functions in NepomukTrack
> 
> 
> Removed debug library include
> 
> And removed other extraneous library includes in NepomukYear
> 
> Added NepomukLabel in CMakeLists.txt
> 
> 
> Added NepomukLabel
> 
> Amarok labels == Nepomuk tags
> 
> Removed unnecessary libraries that were included
> 
> 
> Added Replay Gain to NepomukTrack
> 
> 
> Remove extraneous argument to NepomukYear constructor
> 
> 
> Const correctness
> 
> Arguments wherever applicable are prepended with const modifier
> to comply with coding standards.
> 
> Commenting and documenting for NepomukConstrucMetaJob
> 
> 
> Duplicate meta objects are no longer created
> 
> Using HashMaps for each meta type, duplicate objects that might have been
> created or else have eliminated. Now, all of artist, album, genre, composer
> and track will have only one corresponding Nepomuk{meta} objects.
> 
> NepomukTrack::prettyName() returns right name
> 
> NepomukTrack::prettyName() now returns nie:title
> 
> nie:title is used for title of track
> 
> But this doesn't solve the problem completely.
> A few tracks now have titles that start with
> file://yadayadayada..
> 
> if condition to CMakeLists.txt
> 
> Nepomuk plugin is built only if Nepomuk found.
> Haven't tested this yet
> 
> Merge branch 'gsoc' of git://hades.name/amarok into gsoc
> 
> 
> Implemented collection() in NepomukTrack
> 
> 
> nepomuk: use null album ptr for unknown albums
> 
> 
> memory collection: fix crash due to empty album
> 
> 
> Resolved graying out of tracks
> 
> Still empty NepomukAlbum are constructed when a track with no album is
> encountered. This might be the reason for the numerous 'Unknown Album'.
> The reason for not fixing this is, there might be a bug in
> MemoryMeta::MemoryChanger::addExistingTrack() which doesnt handle
> empty albums right. It is breaking the code.
> 
> Plugin builds collection only if Nepomuk is enabled
> 
> In the constructor of NepomukCollection, buildCollection() is called only
> if Nepomuk is found.
> Removed commented code.
> 
> tracks query and enumeration is a background job
> 
> The background job is complete and the collection updates dynamically.
> Code builds.
> Removed a TODO in Collection Factory as it was done.
> 
> Backqround job works
> 
> But in a crude way. The constructor is stopped from exiting
> using a while(1) loop. This looks and is wrong.
> 
> Background job of NepomukCollection added
> 
> The query and enumeration runs as a background job in
> NepomukCollection. But the UI doesn't show the tracks
> properly. I suspect this is because the UI gets updated
> even before the tracks are queried and enumerated. There
> should be a mechanism to push the queried results in batches.
> This should solve this issue
> 
> Remove unwanted NepomukTrack constructors
> 
> 
> Warning messages if nepomuk is not enabled
> 
> Removed freak error that was incorporated in destructor of NepomukCollection
> 
> Added documentation
> 
> Removed a redundant function in NepomukCollection
> m_collectionReady changes value more meaningfully
> 
> Nepomuk Collection works with albums, but artists are clubbed
> 
> While local collection lists all the different artists on level 1,
> nepomukcollection clubs all the artists into one 'Various Artists'. Albums
> are listed properly. With unknown albums listed as unknown.
> 
> Incorporated the use of MapChanger
> 
> Code compiles and builds. But on running, Amarok crashes.
> I suspect it is because the Track::album() returns something that is not
> being handled properly by the caller
> 
> Tried to check for meta objects before inserting into MetaMaps
> 
> But Amarok crashes on startup with Dr.Konqi error. Added a new constructor for NepomukAlbum that takes a album name and ArtistPtr as arguments
> 
> Deleted notifyObservers()
> 
> Deleted commented code. And reverted CMakeLists.txt since the nepomukcollection wasn't compiling or else. Will look into this at a later stage
> 
> NepomukTrack::score() now returns 0
> 
> 
> Deleted commented lines, ones of NQM,NQMHelper etc
> 
> 
> Removed extraneous functions
> 
> Deleted setup{meta}map() and renamed setupTrackMap to setupMetaMap.
> 
> Now nepomuk collection loads only if Nepomuk and Soprano are found
> 
> 
> Fixed 0 tracks bug in GUI
> 
> Removed commented code
> 
> addTrack() added to NepomukAlbum
> 
> And corresponding changes were made in NepomukCollection
> to populate AlbumMap during construction of TrackMap
> 
> Nepomuk Collection playsgit status
> 
> The tracks in each meta object are populated
> during construction of TrackMap
> 
> Revamped Nepomuk{Meta}::tracks()
> 
> Added a new function called addTrack() which is used
> to add a track to a meta object as soon as the tracks
> are queried in the construction of TrackMap in NepomukCollection
> 
> Tracks and genres are now added to the respective MetaMaps
> 
> The tracks don't list in the GUI yet.
> Formatting changes in other files
> 
> Modified argument placing in constructor of NepomukTrack
> 
> The arguments are ordered according to other
> meta classes.
> 
> Merge branch 'gsoc' of git://hades.name/amarok into gsoc
> 
> 
> New constructor for NepomukTrack
> 
> A construtor which takes MetaPtrs as arguments
> 
> nepomukcollection: remove old files
> 
> 
> Nepomuk Collection shows a count of the tracks!
> 
> Have to implement the other meta maps. Hopefully the
> drop down works then.
> 
> Was returning reference of a local variable. Fixed it
> 
> 
> Setting my repo up, guided by dr_lepper
> 
> Added collections/CMakeLists.txt on amend
> Changed commit msg on amend too
> 
> 
> Diffs
> -----
> 
>   src/core-impl/collections/nepomukcollection/meta/NepomukLabel.h PRE-CREATION 
>   src/core-impl/collections/nepomukcollection/meta/NepomukLabel.cpp PRE-CREATION 
>   src/core-impl/collections/nepomukcollection/meta/NepomukTrack.h PRE-CREATION 
>   src/core-impl/collections/nepomukcollection/meta/NepomukTrack.cpp PRE-CREATION 
>   src/core-impl/collections/nepomukcollection/meta/NepomukYear.h PRE-CREATION 
>   src/core-impl/collections/nepomukcollection/meta/NepomukYear.cpp PRE-CREATION 
>   src/core-impl/collections/support/MemoryMeta.cpp 37ba510f61605af7ebd803aee3529bde18ad84c5 
>   src/core-impl/collections/nepomukcollection/meta/NepomukGenre.h PRE-CREATION 
>   src/core-impl/collections/nepomukcollection/meta/NepomukGenre.cpp PRE-CREATION 
>   src/core-impl/collections/nepomukcollection/meta/NepomukComposer.cpp PRE-CREATION 
>   src/core-impl/collections/nepomukcollection/meta/NepomukComposer.h PRE-CREATION 
>   src/core-impl/collections/nepomukcollection/meta/NepomukArtist.cpp PRE-CREATION 
>   src/core-impl/collections/nepomukcollection/meta/NepomukArtist.h PRE-CREATION 
>   src/core-impl/collections/nepomukcollection/meta/NepomukAlbum.cpp PRE-CREATION 
>   src/core-impl/collections/nepomukcollection/meta/NepomukAlbum.h PRE-CREATION 
>   src/core-impl/collections/nepomukcollection/amarok_collection-nepomukcollection.desktop 815e69e492e819740aba620cc399a8ee79eace74 
>   src/core-impl/collections/nepomukcollection/NepomukYear.h 504cbe2b146ae9a53291de9e82fa384467eb14e1 
>   src/core-impl/collections/nepomukcollection/NepomukYear.cpp 1f13de0bd24e56b1b64b8c45f1d22720dd487a3c 
>   src/core-impl/collections/nepomukcollection/NepomukTrack.cpp 7db01cf34b3765f18f8b8b3cf6efbdf07af6e564 
>   src/core-impl/collections/nepomukcollection/NepomukTrack.h 77dd8c70c8b0727655dfe1db89c7bd19208e77e5 
>   src/core-impl/collections/nepomukcollection/NepomukRegistry.cpp 8afa199f73035eb7d95a8913eb1cbe9fea8b2ebd 
>   src/core-impl/collections/nepomukcollection/NepomukRegistry.h a21347eca2ab519a3c8b5b1f14650878fd7b4333 
>   src/core-impl/collections/nepomukcollection/NepomukQueryMaker.cpp 33163eaa0b279dedcf92de01346312930f10d944 
>   src/core-impl/collections/nepomukcollection/NepomukGenre.cpp 945074c4737ac2856469d5041ca2ea888d609bad 
>   src/core-impl/collections/nepomukcollection/NepomukQueryMaker.h 50067decec72f34a845e1da50e74cdf19e9c0f83 
>   src/core-impl/collections/nepomukcollection/NepomukGenre.h ce0e3b71515d88e57dd3d01beba85e3cdfd8ede6 
>   src/core-impl/collections/nepomukcollection/NepomukConstructMetaJob.h PRE-CREATION 
>   src/core-impl/collections/nepomukcollection/NepomukConstructMetaJob.cpp PRE-CREATION 
>   src/core-impl/collections/nepomukcollection/NepomukCollectionFactory.h PRE-CREATION 
>   src/core-impl/collections/nepomukcollection/NepomukCollectionFactory.cpp PRE-CREATION 
>   src/core-impl/collections/nepomukcollection/NepomukComposer.h 1b11325ec488f202a7b13b10d36c8216b487ae89 
>   src/core-impl/collections/nepomukcollection/NepomukComposer.cpp f21251eab6798bb499d01900151b2c9a1783deae 
>   src/core-impl/collections/nepomukcollection/NepomukCollection.cpp cb185e818de2e00091f9cb03f4b19ccface14635 
>   src/core-impl/collections/nepomukcollection/NepomukArtist.h 6fcedf3ac3724083b6992deb71fb659d9b2dc5d0 
>   src/core-impl/collections/nepomukcollection/NepomukArtist.cpp 13ddf0142796d90af265d28a06d60110da64f138 
>   src/core-impl/collections/nepomukcollection/NepomukCollection.h 928b1458782f0145a012c81468f22edfafc0f547 
>   src/core-impl/collections/nepomukcollection/NepomukAlbum.cpp 6a09a1bbb4ea9bdfc08280326d29a351c666ab25 
>   src/core-impl/collections/CMakeLists.txt c78b9202ece71b51189c4e47d85acfa4a74ef8d6 
>   src/core-impl/collections/nepomukcollection/CMakeLists.txt 7cfd4b056000cf5de18c87d1d014b6670703e796 
>   src/core-impl/collections/nepomukcollection/NepomukAlbum.h 185c25a0fe5b19248a3ab40c1d9d84fd66e6d2fe 
> 
> Diff: http://git.reviewboard.kde.org/r/106042/diff/
> 
> 
> Testing
> -------
> 
> Minimal. Plan to spend the remaining time on testing the plugin. 
> 
> 
> Thanks,
> 
> Phalgun Guduthur
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/amarok-devel/attachments/20120905/4dfe9023/attachment-0001.html>