Review Request: Port PlaydarCollection to QJson instead of JsonQt

Stefan Derkits stefan at derkits.at
Tue Feb 22 17:08:20 CET 2011 


> On Feb. 8, 2011, 2:36 a.m., Andy Coder wrote:
> > This seems reasonable enough to me. Especially since JsonQt was used because it was already hanging around in the source tree, and availability of friendly JSON libraries in package repos wasn't what it is today.  However, if the PlaydarCollection didn't start up automatically and you had Playdar running on your system, then something's broken.
> 
> Stefan Derkits wrote:
>     No Playdar was not running on my System but I believe I didn't introduce Bugs. Could you try it please?
> 
> Andy Coder wrote:
>     I'll try again when I get time, but I couldn't apply your patch, (no idea why, I'm probably doing it wrong).
>     
>     In the meantime, why don't we go ahead and make Qjson an optional dependency, and build the PlaydarCollection conditionally.  That way, we don't add another real dependency. Realistically, Playdar's current status means that, while useful and fun for those who make use of it, the PlaydarCollection isn't likely to see enough use to require all users to install Qjson and build it.
>     
>     And for the record, the way to activate the PlaydarCollection is simply to have a Playdar service running on your machine, (whether or not Amarok's already started).
> 
> Stefan Derkits wrote:
>     Making PlaydarCollection optional (depending on QJSon) sounds good to me ... can do it quickly tonight or after Thursday when I have time again.
>     
>     On the Playdarsite I didn't find how to install it on Linux, is there no Linux Version? Haven't tried this Patch under Windows ... probably there it makes Problems cause not many People have QJson installed ;)
> 
> Leo Franchi wrote:
>     Playdar is pretty quiet/dead atm, but it has a successor project on github that's pre-alpha atm.
>     
>     You can get playdar here: https://github.com/RJ/playdar-core
> 
> Stefan Derkits wrote:
>     I saw here (http://mail.kde.org/pipermail/amarok-devel/2010-December/008362.html) that Playdar Collection was disabled. So if someone wants to test this Patch you have to re-enable it.
>     
>     Generally I was thinking about the "longer startup time" and thought ... maybe (if Playdar should be usefull one day again) create a small Playdar Service, which doesn't do much except enabling the Collection. And this Service can be enabled by Users who want to use Playdar, and the rest doesn't have to suffer from long Startup Times
>     
>     But this is somewhere in the Future, atm this is only about the Port to QJson and that is finished (if Andy Coder gives his ok)
> 
> Andy Coder wrote:
>     Well, I've got an unrelated crash, (in the MySqlEmbeddedStorage constructor), so I haven't been able to test anything at runtime, but the patch applied and built fine, and looks fine.  You should do some more testing, however:
>     
>     In order to add tracks to the PlaydarCollection in Amarok, you can either search using the artist:/album:/title: fields in the filter bar or play music from another collection while the similar artist applet, (or any other applet that generates queries). Any affirmative responses to these queries should be added to the collection automatically, (if Playdar finds them). The idea is that Amarok+Playdar is most useful when Playdar aggregates other sources than the local collection to fill holes in searches/suggestions/playlists, (the playlist thing, admittedly requires the url for a missing track to have been produced by a Playdar::Controller, so that could use improved someday).
>     
>     Now that you know the Collection starts for you, Amarok and Playdar are communicating, so if you can get a few tracks to appear and play, that's all the testing that should be needed, (i.e. we should ship it).
> 
> Stefan Derkits wrote:
>     Tried it; but can't get the PlaydarCore to return any Query with Results (also not with Playdar in the Main Branch) ... if I search for Air the Query that goes to PlaydarCore seems to be: {"artist":"Air","album":"Air","track":"Air"}

Works good for me now that I found out how it works ;) Tested it (also finds songs that are not yet in my library) and just waiting for a ship it ;)


- Stefan


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


On Feb. 20, 2011, 10:51 p.m., Stefan Derkits wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/100607/
> -----------------------------------------------------------
> 
> (Updated Feb. 20, 2011, 10:51 p.m.)
> 
> 
> Review request for Amarok.
> 
> 
> Summary
> -------
> 
> What I did:
> -) added cmake/modules/FindQJson.cmake
> -) deleted external/JsonQt & adapted external/CmakeLists.txt
> -) use QJson in collections/playdarcollection/support/
> -) only compile Playdarcollection if QJson was found
> 
> Why:
> QJson is available via Package on pretty much every System now
> Why QJson instead JsonQt? libmygpo-qt uses it, libechonest uses it and both Libraries will (sooner or later) be used in Amarok
> 
> 
> Diffs
> -----
> 
>   CMakeLists.txt 11d4c9d5a296560f1c779e1dee4405c9827dac91 
>   README d8c142968c2da3888a703cc2b2e38d854f5c868b 
>   cmake/modules/FindQJSON.cmake PRE-CREATION 
>   external/CMakeLists.txt 0e161c701e0d1a9d7b769a1dda742df9e8f303ae 
>   external/JsonQt/CMakeLists.txt 9978895bb72fa4993a4326397fa7284c74fd00ee 
>   external/JsonQt/COMPATIBILITY 1a5b6195ca32395657535bd426a91a8a8ee5c22f 
>   external/JsonQt/COPYING.HEADER bf9c30eb90c5aa3c04a87a010736bb1c64326885 
>   external/JsonQt/Doxyfile.cmake 4d2a4bf7035976f1cd06dcd43a55cb8ab660e47d 
>   external/JsonQt/README.txt 4ebcf735ccbff8556914ed687b27f065dfb4a229 
>   external/JsonQt/add-copyright-header.sh 8d21c92fbfa305e0f24d57e9a921235f624aecba 
>   external/JsonQt/lib/CMakeLists.txt 3d394572d1af6b062ff6fb10881e462b5b4f7f9e 
>   external/JsonQt/lib/JsonQtExport.h 30d5a7569710d4c31d9deffe3798566008fe89bc 
>   external/JsonQt/lib/JsonRpc.h 8f0024c03c0c89c021f271421821e57b3d4b9367 
>   external/JsonQt/lib/JsonRpc.cpp cbcd16156ac44a55b7c34aae3d2b47b2b776f856 
>   external/JsonQt/lib/JsonRpcAdaptor.h 83836c810358b7b5db44d151c6840027ec85586a 
>   external/JsonQt/lib/JsonRpcAdaptor.cpp 18a1504435040a1cb8e6a785ae8093ade7d232ef 
>   external/JsonQt/lib/JsonRpcAdaptorPrivate.h 03c990ce187ff90ccb591052f25de547e5651982 
>   external/JsonQt/lib/JsonRpcAdaptorPrivate.cpp c8fddb012b45c6117e5ac74b7d36cfa6c02048b1 
>   external/JsonQt/lib/JsonToProperties.h f04bd586e5d1aae9727d8bc4e218171954d86ec5 
>   external/JsonQt/lib/JsonToProperties.cpp 4859d49b643bcd2e9cfb54c0c64581c94654b3f4 
>   external/JsonQt/lib/JsonToVariant.h 82b62c0446b03df25aabcbd508fd24e290a9ac98 
>   external/JsonQt/lib/JsonToVariant.cpp 323e31adfe0558d9b4dd671b7a9409cb9bb202fa 
>   external/JsonQt/lib/ParseException.h 2f6566099bc13ab84eba28054330bcb0812e9785 
>   external/JsonQt/lib/ParseException.cpp c663179388f193175febe0eaaca6a2ce8241c215 
>   external/JsonQt/lib/VariantToJson.h 2a1a645e4743af267ce21b55d7c164ea2805940b 
>   external/JsonQt/lib/VariantToJson.cpp baa7cddb548f10e6ea25e1b194683de7863055fc 
>   external/JsonQt/tests/CMakeLists.txt 4954fe78d726afc534b541cd97c79551f40f742a 
>   external/JsonQt/tests/JsonRpc.cpp 1ce4c9c59f5739426adbb3ed3ddaf148a3081a50 
>   external/JsonQt/tests/JsonRpcAdaptor.cpp 6e995993c5683f64a8313e97ba4ec687032fc98a 
>   external/JsonQt/tests/JsonToProperties.cpp e1a0707737dd8051e601283cc0b0069894a095bd 
>   external/JsonQt/tests/JsonToVariant.cpp 414d1c9c0e8da5f1a9a651ddd6a2e2da04395f4e 
>   external/JsonQt/tests/VariantToJson.cpp dfad043d13b25508785bace2277f3ac24d8dd2d5 
>   src/core-impl/collections/CMakeLists.txt 43217903cadf2f56c1ee8610dacbd4e1b34ebd57 
>   src/core-impl/collections/playdarcollection/CMakeLists.txt 589ab02a8a1a09acdd2289c028fbca7b521b0e6a 
>   src/core-impl/collections/playdarcollection/support/Controller.cpp f8e78055115f8ff38673c5eba0d493b53a000fa2 
>   src/core-impl/collections/playdarcollection/support/Query.cpp 4815d362a50344c7d1936fe0a98536577393a372 
> 
> Diff: http://git.reviewboard.kde.org/r/100607/diff
> 
> 
> Testing
> -------
> 
> compiled & run Amarok; didn't specifically try out Playdarcollection, didn't know how to "activate" it
> 
> 
> Thanks,
> 
> Stefan
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://mail.kde.org/pipermail/amarok-devel/attachments/20110222/5d2037e1/attachment-0001.htm

More information about the Amarok-devel mailing list