Re: plasma-framework, kactivities and kactivities-stats: please consider proper de-KF-ication now
Hi, +1 to what Carl said. Right now my Elisa barely needs any time to read my music library, presumably because Baloo already has it indexed. It'll be quite a shame if apps did their own indexing, wasting time and power. The effects (of removing Baloo support) are even more pronounced if we consider that a majority of Elisa users are running Plasma as well. (Citation needed, I know, but IMO it's not such a risky assumption.) Regards, Shantanu On Sun, Nov 5, 2023, 11:53 PM Carl Schwan wrote: > On Sunday, November 5, 2023 6:01:38 PM CET Nate Graham wrote: > > On 11/5/23 07:42, Kevin Ottens wrote: > > > I was clumsily advocating for this Akademy 2021 or 2022 (can't remember > > > which). > > > > > > This way it's clearer to application authors when they tie themselves > to a > > > given workspace or not. > > > > > > Also, isn't Elisa able to work without Baloo? It even seems to do the > > > right > > > thing if I trust its CMakeLists.txt. It has Baloo as a recommended but > > > optional dependency, and disable it altogether for Win32 builds. > > > > Yes, Elisa also includes an internal indexer, for use on Windows and > > Android, or on *Nix when Baloo isn't installed or is disabled. > > > > I think the original idea for the app was to delegate all the indexing > > heavy lifting to Baloo to avoid internal complication, but clearly this > > has not worked out in practice, since to be truly cross-platform, it > > can't assume that Baloo is present and active and does need its own > > indexer. So maybe the best course of action is actually to remove Baloo > > support entirely and always use the internal indexer, so that we don't > > have two different code paths. > > > > Nate > > I do not belive this is a good idea. Mostly because it means we would then > have an indexer in Baloo and one indexer in Elisa running at the same time > and > doing something similar which is wasteful. It becomes even more of an > issue > when other apps like Peruse or Arianna start doing the same and use their > own > indexer instead of Baloo on Plasma. > > This defeat the point of having a general purpose indexer in Plasma. > > Cheers, > Carl > > > >
Re: Review Request 118697: Treat albums with different artists as different albums
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/118697/ --- (Updated Sept. 22, 2016, 8:40 p.m.) Status -- This change has been discarded. Review request for Plasma. Repository: plasma-mediacenter Description --- This one has been pending for some time now. Our Album data model assumed that a album name is enough to uniquely identify an album. This creates problems because multiple artists may have albums with the same name. This patch makes Media Library and ItemCache aware of this fact. Diffs - libs/mediacenter/album.h 4926e48 libs/mediacenter/album.cpp 881f65a libs/mediacenter/itemcache.h ffee05f libs/mediacenter/medialibrary.cpp 9c58b44 libs/test/itemcachetest.h 4fc6807 libs/test/itemcachetest.cpp e6dee33 libs/test/medialibrarytest.h 718457d libs/test/medialibrarytest.cpp e1a8c9b libs/test/mediatest.cpp 8fde60b libs/test/pmcalbumtest.cpp 9f2b4c8 Diff: https://git.reviewboard.kde.org/r/118697/diff/ Testing --- Unit tests pass. Added test to take care of the new scenario. Thanks, Shantanu Tushar
Re: Moving plasma-mediacenter to extragear
Hi, On Mon, Jul 18, 2016 at 2:29 PM, Bhushan Shah <bhus...@gmail.com> wrote: > Hello plasma team, > > I am not much happy to write this, but here it goes, > > Since last three Plasma release it always bugged me that there were no > usefull changes in plasma-mediacenter, mostly because after being > contracted by Bluesystems I got involved into Plasma Mobile related > work and didn't manage to find time to work on plasma-mediacenter. > > I already have some work branches in plasma-mediacenter which enables > the containment which can work nicely with remote controller, and other > local work branch for medialibrary improvements. However with the speed > plasma releases are happening I am too hesistent to merge those as it > would break things and I may or may not get time to fix those in time. > I kinda had this feeling when the PMC move to a core Plasma component which needs more cautious development. It was always easier to work on PMC treating it as a standalone component, so +1 from me. > So therefore, I am proposing to move plasma-mediacenter to extragear or > playground and not release it with Plasma/5.8 anymore. In extragear or > playground I will be able to define my own release scedule and cycle, > and merge my changes. > > If there are no objections within next week, I will file a sysadmin > request to move plasma-mediacenter to extragear/playground and request > Riddell to not release it anymore with Plasma/5.8. > > Thanks > > -- > Bhushan Shah > > http://bhush9.github.io > IRC Nick : bshah on Freenode > > ___ > Plasma-devel mailing list > Plasma-devel@kde.org > https://mail.kde.org/mailman/listinfo/plasma-devel > -- Shantanu Tushar(UTC +0530) http://www.shantanutushar.com ___ Plasma-devel mailing list Plasma-devel@kde.org https://mail.kde.org/mailman/listinfo/plasma-devel
Re: Review Request 125993: Implemented lastfmimagefetcher as a Plugin
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/125993/#review88715 --- Do the tests not work anymore? Just curious why were they removed. libs/mediacenter/lastfmimagefetcher.cpp (line 155) <https://git.reviewboard.kde.org/r/125993/#comment60828> remove the extra whitespace - Shantanu Tushar On Nov. 22, 2015, 2:23 p.m., Aditya Dev Sharma wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://git.reviewboard.kde.org/r/125993/ > --- > > (Updated Nov. 22, 2015, 2:23 p.m.) > > > Review request for Plasma, Ashish Bansal and Bhushan Shah. > > > Repository: plasma-mediacenter > > > Description > --- > > Implemented it like the other datasources. > > Signal newMedia from MediaLibrary is absorbed by handleLastFmNewMedia . > handleLastFmNewMedia then calls fetchImage for each newMedia in the > newMediaList. Created a QList for storing the new media. > > After the image is fetched, gotImage emits the signal imageFetched (from > mediaLibrary) with the QList we created earlier. This signal is absorbed by > handleNewMedia in PmcMetaDataModel which does the rest. > > > Diffs > - > > autotests/CMakeLists.txt 04ba1c3 > autotests/lastfmimagefetchertest.h eb7cb29 > autotests/lastfmimagefetchertest.cpp abb3fd6 > datasources/CMakeLists.txt a7171b7 > datasources/lastfm/CMakeLists.txt PRE-CREATION > datasources/lastfm/lastfmimagefetcher.desktop PRE-CREATION > libs/mediacenter/CMakeLists.txt 5a13449 > libs/mediacenter/lastfmimagefetcher.h ecff37e > libs/mediacenter/lastfmimagefetcher.cpp e005077 > libs/mediacenter/medialibrary.h 45744c1 > libs/mediacenter/pmcmetadatamodel.cpp 2fe6efe > > Diff: https://git.reviewboard.kde.org/r/125993/diff/ > > > Testing > --- > > Builds 100%. > However the audio scrobbler api doesn't seem to work which I'll fix after > this. > Also, I get the following Output : http://ix.io/mps > Apart from it, all the datasources are loaded and media plays as expected. > > > Thanks, > > Aditya Dev Sharma > > ___ Plasma-devel mailing list Plasma-devel@kde.org https://mail.kde.org/mailman/listinfo/plasma-devel
Re: Review Request 123467: Add CoverArt uri to the metadata exposed with MPRIS interfaces
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/123467/#review79349 --- The idea sounds, some nitpicks though. libs/mediacenter/mpris2/mpris2.cpp (line 33) https://git.reviewboard.kde.org/r/123467/#comment54195 Hardcoding '/' might not be portable, try using QDir's features to get what you want. I'd prefer having a class QDir var which is initialized to the full path (till covers/) in the ctor. libs/mediacenter/mpris2/mpris2.cpp (line 104) https://git.reviewboard.kde.org/r/123467/#comment54197 Make this a member as suggested above. libs/mediacenter/mpris2/mpris2.cpp (lines 106 - 108) https://git.reviewboard.kde.org/r/123467/#comment54196 Using the QDir member as suggested above, call absoluteFilePath instead of doing a string concatenate. - Shantanu Tushar On April 22, 2015, 1:44 p.m., Ashish Madeti wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/123467/ --- (Updated April 22, 2015, 1:44 p.m.) Review request for Plasma, Bhushan Shah, Shantanu Tushar, and Sinny Kumari. Repository: plasma-mediacenter Description --- Save Cover art to a temperoray folder. Include the path of the cover art in the metadata currently exposed with MPRIS interfaces. Diffs - libs/mediacenter/mpris2/mpris2.cpp f03d062 Diff: https://git.reviewboard.kde.org/r/123467/diff/ Testing --- Correct path is showing up in the metadata when tested with mpristester. Correct cover art is showing up in the mpris controller (appearing in system tray) of Plasma 5. Thanks, Ashish Madeti ___ Plasma-devel mailing list Plasma-devel@kde.org https://mail.kde.org/mailman/listinfo/plasma-devel
Re: plasma-mediacenter in kdereview from extragear
On Sat, Mar 7, 2015 at 3:19 AM, Albert Astals Cid aa...@kde.org wrote: El Dijous, 5 de març de 2015, a les 08:51:37, Bhushan Shah va escriure: Hello! On Thu, Feb 19, 2015 at 8:22 PM, Bhushan Shah bhus...@gmail.com wrote: We want to move plasma-mediacenter to kde/workspace for releasing it with Plasma 5.x release cycle, Plasma media center is living room solution of plasma workspaces, it is integrated as shell package for PMC. Given the two week has been passed in kdereview and as there are no objections I will file the sysadmin request to move the repository to kde/workspace Is there ever going to be another release of the stable kdelibs4-based 1.4 branch? I'm asking because it used to be in extragear and moved to workspace, and thus the l10n of the kde4 branch needs some engineering to still be in extragear, and it'd be just easier for us if you're not planning more releases since then we can just remove the l10n handlong in kde4 land altogether. No, the kdelibs4 based version will not see a new release. We are focusing all our efforts on the kf5/Plasma5 version now. Cheers, -- Shantanu Tushar(UTC +0530) http://www.shantanutushar.com Cheers, Albert Thanks! ___ Plasma-devel mailing list Plasma-devel@kde.org https://mail.kde.org/mailman/listinfo/plasma-devel
Re: Review Request 122745: Monitor baloo for new media files
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/122745/#review76752 --- Ship it! Ship It! - Shantanu Tushar On Feb. 27, 2015, 4:47 p.m., Bhushan Shah wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/122745/ --- (Updated Feb. 27, 2015, 4:47 p.m.) Review request for Plasma, Shantanu Tushar, Sinny Kumari, and Vishesh Handa. Repository: plasma-mediacenter Description --- This is uses the dbus signal provided by baloo, checks them mimetype of that file and passes that to related searchresulthandler Bonus point of this is in case properties of any files changes we also get updates. Diffs - mediasources/baloosearch/CMakeLists.txt 9bec5fd6c5af585b8faa53d5440878bf0dcf6566 mediasources/baloosearch/audiosearchresulthandler.h 3efb4bf1efc337a0a293bc1c391707302a76172c mediasources/baloosearch/audiosearchresulthandler.cpp 0c9d93b1988af62836f9c547707395810da311a2 mediasources/baloosearch/baloosearchmediasource.h e0289026e2cbf9c7e145f66b057b7ae6287b8e48 mediasources/baloosearch/baloosearchmediasource.cpp b90e9c03e352d1fd6962fbb28de9b8714e866fde mediasources/baloosearch/imagesearchresulthandler.h 72a2cb30617b91b92e9176688e21d63504876eb1 mediasources/baloosearch/imagesearchresulthandler.cpp 6df3a2c11092422a18b00bb668498dc1b5fcbc2e mediasources/baloosearch/searchresulthandler.h 176327409fa9acd92d21a69e2a91f035d9140f51 mediasources/baloosearch/videosearchresulthandler.h 00d048cc0bfa44ae323afd531c7b41a3bcbcd528 mediasources/baloosearch/videosearchresulthandler.cpp f1439e735be43be68d0b4625019493f9249ae1e5 Diff: https://git.reviewboard.kde.org/r/122745/diff/ Testing --- Moved some files from External drive to $HOME, they got added to medialibrary, verified by debug messages for images and manually for music files Thanks, Bhushan Shah ___ Plasma-devel mailing list Plasma-devel@kde.org https://mail.kde.org/mailman/listinfo/plasma-devel
Re: Review Request 121144: Remove MetadaBackendCommonModel
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/121144/#review70498 --- Ship it! Ship It! - Shantanu Tushar On Nov. 17, 2014, 5:55 a.m., Bhushan Shah wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/121144/ --- (Updated Nov. 17, 2014, 5:55 a.m.) Review request for Plasma, Shantanu Tushar, Sinny Kumari, and Sujith Haridasan. Repository: plasma-mediacenter Description --- this was just a wrapper around PmcMetadataModel and was not doing anything Diffs - browsingbackends/metadatavideobackend/metadatavideomodel.h 3e85f4a browsingbackends/metadatavideobackend/metadatavideomodel.cpp 417cddd browsingbackends/metadatabackendcommonmodel.h 0873bc0 browsingbackends/metadatabackendcommonmodel.cpp fbee1ff browsingbackends/metadatamusicbackend/CMakeLists.txt 48e98ea browsingbackends/metadatamusicbackend/metadatamusicbackend.h 836ee21 browsingbackends/metadatamusicbackend/metadatamusicbackend.cpp d9f4fa6 browsingbackends/metadatamusicbackend/metadatamusicsongsmodel.h 53efbd2 browsingbackends/metadatamusicbackend/metadatamusicsongsmodel.cpp 55debf0 browsingbackends/metadatavideobackend/CMakeLists.txt 2b81ab2 browsingbackends/metadatavideobackend/metadatavideobackend.cpp 1ed833d Diff: https://git.reviewboard.kde.org/r/121144/diff/ Testing --- everything works fine without it Thanks, Bhushan Shah ___ Plasma-devel mailing list Plasma-devel@kde.org https://mail.kde.org/mailman/listinfo/plasma-devel
Re: Review Request 121130: Remove okToLoad method of backend
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/121130/#review70417 --- Ship it! Ship It! - Shantanu Tushar On Nov. 15, 2014, 4:12 p.m., Bhushan Shah wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/121130/ --- (Updated Nov. 15, 2014, 4:12 p.m.) Review request for Plasma, Shantanu Tushar, Sinny Kumari, and Sujith Haridasan. Repository: plasma-mediacenter Description --- This was case for local filebrowsing on plasma active but since we already removed localfile browsing backends remove this method I cross checked this with other backends that might need such use case but I think those backends can return false in initImpl or choose not to give any data Diffs - libs/mediacenter/abstractbrowsingbackend.h 3a9dbea libs/mediacenter/abstractbrowsingbackend.cpp f770759 libs/mediacenter/backendsmodel.cpp a4fff82 Diff: https://git.reviewboard.kde.org/r/121130/diff/ Testing --- compiles fine Thanks, Bhushan Shah ___ Plasma-devel mailing list Plasma-devel@kde.org https://mail.kde.org/mailman/listinfo/plasma-devel
Re: Review Request 120980: Proper API for Play all function
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/120980/#review69831 --- Ship it! Looks good, some minor fixes and this can go in. browsingbackends/metadatamusicbackend/metadatamusicbackend.cpp https://git.reviewboard.kde.org/r/120980/#comment48875 pull m_musicFilteredModel-index(i, 0) into a variable browsingbackends/metadatamusicbackend/metadatamusicbackend.cpp https://git.reviewboard.kde.org/r/120980/#comment48876 use auto libs/mediacenter/abstractbrowsingbackend.cpp https://git.reviewboard.kde.org/r/120980/#comment48874 return QStringList(); - Shantanu Tushar On Nov. 4, 2014, 1:44 p.m., Bhushan Shah wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/120980/ --- (Updated Nov. 4, 2014, 1:44 p.m.) Review request for Plasma, Bhushan Shah, Shantanu Tushar, Sinny Kumari, and Sujith Haridasan. Repository: plasma-mediacenter Description --- - AbstractBrowsingBackend::allMedia() returns all media in QStringList - PlaylistModel::addAllToPlaylist() to add QStringList in current playlist - This all is handled by mediabrowser and playlist.qml Diffs - browsingbackends/metadatamusicbackend/metadatamusicbackend.h c9f10a77753f7427f669da9cb5e46cf66994f57d browsingbackends/metadatamusicbackend/metadatamusicbackend.cpp f4f908047586694188a354bc69ecc49824f15fe7 libs/mediacenter/abstractbrowsingbackend.h 910272a93ec047418ad1a4c6534592bb20fcba60 libs/mediacenter/abstractbrowsingbackend.cpp c24d5178d91a09b4e9f8b5b74d7a88616a8b9756 libs/mediacenter/playlistmodel.h d45611c1eaddb657ce3ed97731e9e70990b7a060 libs/mediacenter/playlistmodel.cpp 501e501888d53fd0fdd6574a72988408307b3f7b mediaelements/mediabrowser/MediaBrowser.qml 97942881d08c12539675ba08d7fc6a8b28206fb5 mediaelements/playlist/Playlist.qml f39be2a8a695499fc05d69cd0c13d70e18356142 shells/next/contents/views/Desktop.qml 8ed3d1e2e013cbfd0aac9d0fca099901fac3f237 Diff: https://git.reviewboard.kde.org/r/120980/diff/ Testing --- works fine Thanks, Bhushan Shah ___ Plasma-devel mailing list Plasma-devel@kde.org https://mail.kde.org/mailman/listinfo/plasma-devel
Re: Review Request 120855: Add Google two-step verification support to PMC Picasa login
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/120855/#review69698 --- Thanks for your time on this, good stuff :) Tried it and the only caveat is that after login is done I have to press the back button (the one on the pmc controller) to get my album list. That should be fixed. browsingbackends/onlineservices/picasa/picasamodel.h https://git.reviewboard.kde.org/r/120855/#comment48822 where do you get these from? - Shantanu Tushar On Nov. 2, 2014, 3:34 p.m., Ashish Bansal wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/120855/ --- (Updated Nov. 2, 2014, 3:34 p.m.) Review request for KDEPIM, KDEPIM-Libraries, Plasma, Bhushan Shah, Shantanu Tushar, Sinny Kumari, and Sujith Haridasan. Bugs: 336398 http://bugs.kde.org/show_bug.cgi?id=336398 Repository: plasma-mediacenter Description --- Changed auth method from Client Login to OAuth using libkgapi2 Diffs - browsingbackends/onlineservices/picasa/CMakeLists.txt bd5052d browsingbackends/onlineservices/picasa/picasabackend.h 7144dc7 browsingbackends/onlineservices/picasa/picasabackend.cpp 6317240 browsingbackends/onlineservices/picasa/picasacomponents/PicasaSidePanel.qml e8f97f7 browsingbackends/onlineservices/picasa/picasacomponents/qmldir c790310 browsingbackends/onlineservices/picasa/picasamodel.h 5bce055 browsingbackends/onlineservices/picasa/picasamodel.cpp 2b64156 Diff: https://git.reviewboard.kde.org/r/120855/diff/ Testing --- Everything is working fine except that after successful login, user has to press back to view all the albums. Thanks, Ashish Bansal ___ Plasma-devel mailing list Plasma-devel@kde.org https://mail.kde.org/mailman/listinfo/plasma-devel
Plasma Media Center and external USB drives
Hi folks, While we are busy implementing the new PMC design[1], we revisited the question on what happens to USB devices. Finally we think we will offer the following options to the user when a USB device is detected- 1. Mount the USB device and ask Baloo to index it so that the media show up in the usual PMC (Video/Songs/Pictures) screens. (This option might not be preferred if the user, for some reason, doesn't want the drive to be indexed and be available to other apps as well) 2. Mount the USB device and let the user browse through files and play them manually. Just like there are two options, we have two questions that we needed some usability thinking around- 1. Do these two options make sense? Or can someone think of even better ways of handling this? 2. How (and where) do we display the options to the user, in the new mockup? Ideas? [1] https://forum.kde.org/viewtopic.php?f=285t=121705#p316799 -- Shantanu Tushar(UTC +0530) http://www.shantanutushar.com ___ Plasma-devel mailing list Plasma-devel@kde.org https://mail.kde.org/mailman/listinfo/plasma-devel
Re: Review Request 120298: Enable lastfmimagefetchertest again,
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/120298/#review67100 --- libs/mediacenter/lastfmimagefetcher.cpp https://git.reviewboard.kde.org/r/120298/#comment46833 In addition to this, can you qDebug the error message so that when this fails in non-test scenarios, there is something to debug. - Shantanu Tushar On Sept. 20, 2014, 3:04 p.m., Bhushan Shah wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/120298/ --- (Updated Sept. 20, 2014, 3:04 p.m.) Review request for Plasma, Shantanu Tushar, Sinny Kumari, and Sujith Haridasan. Repository: plasma-mediacenter Description --- This enables lastfmimagefetchertest again with error handling Diffs - libs/mediacenter/lastfmimagefetcher.h 25d53fd libs/mediacenter/lastfmimagefetcher.cpp 8d9edf4 libs/test/lastfmimagefetchertest.h f315f7c libs/test/lastfmimagefetchertest.cpp e9ff4f1 Diff: https://git.reviewboard.kde.org/r/120298/diff/ Testing --- Checked without internet, test is getting skipped. Thanks, Bhushan Shah ___ Plasma-devel mailing list Plasma-devel@kde.org https://mail.kde.org/mailman/listinfo/plasma-devel
Re: Review Request 119218: Add option to toggle between PerspectiveCrop/Fit while viewing images
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/119218/#review67101 --- Ship it! Ship It! - Shantanu Tushar On Sept. 20, 2014, 1:59 p.m., R.Harish Navnit wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/119218/ --- (Updated Sept. 20, 2014, 1:59 p.m.) Review request for Plasma, Bhushan Shah, Shantanu Tushar, and Sinny Kumari. Repository: plasma-mediacenter Description --- Double clicking the image adds toggles between PerspectiveCrop/Fit. This feature is particularly useful while viewing landscape images. Diffs - mediaelements/imageviewer/ImageViewer.qml 81f9cc6a9268040e657216d1dee2e00abf76b4fa Diff: https://git.reviewboard.kde.org/r/119218/diff/ Testing --- Open new image, double click -- toggles. double click again -- toggles again. Thanks, R.Harish Navnit ___ Plasma-devel mailing list Plasma-devel@kde.org https://mail.kde.org/mailman/listinfo/plasma-devel
Re: Review Request 120298: Enable lastfmimagefetchertest again,
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/120298/#review67102 --- Ship it! Other than that, this is okay to go in. - Shantanu Tushar On Sept. 20, 2014, 3:04 p.m., Bhushan Shah wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/120298/ --- (Updated Sept. 20, 2014, 3:04 p.m.) Review request for Plasma, Shantanu Tushar, Sinny Kumari, and Sujith Haridasan. Repository: plasma-mediacenter Description --- This enables lastfmimagefetchertest again with error handling Diffs - libs/mediacenter/lastfmimagefetcher.h 25d53fd libs/mediacenter/lastfmimagefetcher.cpp 8d9edf4 libs/test/lastfmimagefetchertest.h f315f7c libs/test/lastfmimagefetchertest.cpp e9ff4f1 Diff: https://git.reviewboard.kde.org/r/120298/diff/ Testing --- Checked without internet, test is getting skipped. Thanks, Bhushan Shah ___ Plasma-devel mailing list Plasma-devel@kde.org https://mail.kde.org/mailman/listinfo/plasma-devel
Re: Review Request 118866: Start playing after Play All is used
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/118866/ --- (Updated Aug. 4, 2014, 4:01 p.m.) Status -- This change has been marked as submitted. Review request for Plasma. Repository: plasma-mediacenter Description --- We should ask the model to start playing once we add all the songs to the playlist as a result of the user using Play All. Diffs - libs/mediacenter/multipleplaylistmodel.h 1c7731b libs/mediacenter/multipleplaylistmodel.cpp 950675e libs/mediacenter/playlistmodel.h 50157ff libs/mediacenter/playlistmodel.cpp 44abbc1 browsingbackends/localfiles/localfilesabstractbackend.cpp bbd511b browsingbackends/metadatabackends/metadatamusicbackend/metadatamusicbackend.cpp 8b8e149 Diff: https://git.reviewboard.kde.org/r/118866/diff/ Testing --- Thanks, Shantanu Tushar ___ Plasma-devel mailing list Plasma-devel@kde.org https://mail.kde.org/mailman/listinfo/plasma-devel
Re: Review Request 118406: Notify the user if the location containing the media is inaccessible.
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/118406/#review63646 --- mediaelements/mediaplayer/MediaPlayer.qml https://git.reviewboard.kde.org/r/118406/#comment44341 Video should not know about playlistModel mediaelements/playlist/Playlist.qml https://git.reviewboard.kde.org/r/118406/#comment44342 what is this for? it won't do anything - Shantanu Tushar On Aug. 1, 2014, 4:03 p.m., R.Harish Navnit wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/118406/ --- (Updated Aug. 1, 2014, 4:03 p.m.) Review request for Plasma, Shantanu Tushar and Sinny Kumari. Bugs: 333764 http://bugs.kde.org/show_bug.cgi?id=333764 Repository: plasma-mediacenter Description --- If a media(in a playlist) is located in an inaccessible location, then the user is notified about the same. Diffs - mediaelements/mediaplayer/MediaPlayer.qml 98f1d2c mediaelements/playlist/Playlist.qml 5dde297 Diff: https://git.reviewboard.kde.org/r/118406/diff/ Testing --- 1. Load media to a playlist. 2. Unmount the device containing media. 3. Check if the user is notified of the location being inaccessible --yes, the user is notified 4. Mount the device containing media and play a media from playlist. -- The media plays properly. File Attachments wihtout_i18n.png https://git.reviewboard.kde.org/media/uploaded/files/2014/06/05/3dc148a5-c4da-4d27-a713-e63922cbcef8__wihtout_i18n.png Thanks, R.Harish Navnit ___ Plasma-devel mailing list Plasma-devel@kde.org https://mail.kde.org/mailman/listinfo/plasma-devel
Re: Reviews
This one needs to be discarded as well, https://git.reviewboard.kde.org/r/115865/ On Tue, Jul 29, 2014 at 5:15 PM, Bhushan Shah bhus...@gmail.com wrote: Hello, On Tue, Jul 29, 2014 at 3:37 PM, David Edmundson da...@davidedmundson.co.uk wrote: There are a /lot/ of Plasma Media Centre reviews open (about half are PMC), I'm not sure what the state of those is, are they awaiting reviews from the non-media centre devs, or just no longer relevant. If someone from PMC pings me I can help go through and discard outdated things. Following PMC reviews needs to be discarded, https://git.reviewboard.kde.org/r/116460/ (Fixed already) https://git.reviewboard.kde.org/r/116804/ (Ditto) https://git.reviewboard.kde.org/r/116731 (I think, it was discarded long ago, I don't know how it came back? bug was also closed) I'll let you know as I will go through other reviews. Looks like there is no way to filter search by repo name or am I missing something.? Lets see.. -- Bhushan Shah http://bhush9.github.io IRC Nick : bshah on Freenode ___ Plasma-devel mailing list Plasma-devel@kde.org https://mail.kde.org/mailman/listinfo/plasma-devel -- Shantanu Tushar(UTC +0530) http://www.shantanutushar.com ___ Plasma-devel mailing list Plasma-devel@kde.org https://mail.kde.org/mailman/listinfo/plasma-devel
Re: Review Request 118866: Start playing after Play All is used
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/118866/ --- (Updated July 29, 2014, 12:35 p.m.) Review request for Plasma. Changes --- Consistent behavior for Local Browsing backends as well Repository: plasma-mediacenter Description --- We should ask the model to start playing once we add all the songs to the playlist as a result of the user using Play All. Diffs (updated) - libs/mediacenter/multipleplaylistmodel.h 1c7731b libs/mediacenter/multipleplaylistmodel.cpp 950675e libs/mediacenter/playlistmodel.h 50157ff libs/mediacenter/playlistmodel.cpp 44abbc1 browsingbackends/localfiles/localfilesabstractbackend.cpp bbd511b browsingbackends/metadatabackends/metadatamusicbackend/metadatamusicbackend.cpp 8b8e149 Diff: https://git.reviewboard.kde.org/r/118866/diff/ Testing --- Thanks, Shantanu Tushar ___ Plasma-devel mailing list Plasma-devel@kde.org https://mail.kde.org/mailman/listinfo/plasma-devel
Re: Review Request 116898: Get rid of unnecessary Q_INVOKABLE declarations
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/116898/ --- (Updated July 29, 2014, 1:19 p.m.) Status -- This change has been discarded. Review request for Plasma and Sujith Haridasan. Repository: plasma-mediacenter Description --- Methods supposed to be used as READ of Q_PROPERTY properties need not be invokable. Same goes for slots. Diffs - libs/mpris2/mediaplayer2.h e68bc5c libs/mpris2/mediaplayer2player.h 203d681 libs/mpris2/mpris2.h 0df64f4 libs/mpris2/mpris2.cpp a8ad3ef shells/newshell/main.cpp bab6915 shells/newshell/mainwindow.cpp d2d71d4 Diff: https://git.reviewboard.kde.org/r/116898/diff/ Testing --- Thanks, Shantanu Tushar ___ Plasma-devel mailing list Plasma-devel@kde.org https://mail.kde.org/mailman/listinfo/plasma-devel
Re: Review Request 119250: Events and trips according to date ranges
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/119250/ --- (Updated July 22, 2014, 3:28 p.m.) Status -- This change has been marked as submitted. Review request for Plasma. Repository: plasma-mediacenter Description --- I got this idea on a recent trip that usually a family would have a device where they store their trips/events photos on. Easier than tagging everything, what if you could just say Hey, I went to Kashmir during 5 April to 14 April 2014 and voila! This backend implements this idea in a (hopefully) easy-to-use way. I'll love to hear some feedback around this :) Diffs - browsingbackends/metadatabackends/CMakeLists.txt ae56603 browsingbackends/metadatabackends/eventsbackend/CMakeLists.txt PRE-CREATION browsingbackends/metadatabackends/eventsbackend/eventsbackend.cpp PRE-CREATION browsingbackends/metadatabackends/eventsbackend/eventsbackend.desktop PRE-CREATION browsingbackends/metadatabackends/eventsbackend/eventsbackend.h PRE-CREATION browsingbackends/metadatabackends/eventsbackend/eventscomponents/DatePicker.qml PRE-CREATION browsingbackends/metadatabackends/eventsbackend/eventscomponents/Digit.qml PRE-CREATION browsingbackends/metadatabackends/eventsbackend/eventscomponents/EventsConfiguration.qml PRE-CREATION browsingbackends/metadatabackends/eventsbackend/eventscomponents/qmldir PRE-CREATION browsingbackends/metadatabackends/eventsbackend/eventsfiltermodel.h PRE-CREATION browsingbackends/metadatabackends/eventsbackend/eventsfiltermodel.cpp PRE-CREATION browsingbackends/metadatabackends/eventsbackend/eventsmodel.h PRE-CREATION browsingbackends/metadatabackends/eventsbackend/eventsmodel.cpp PRE-CREATION browsingbackends/metadatabackends/eventsbackend/eventspicturesmodel.h PRE-CREATION browsingbackends/metadatabackends/eventsbackend/eventspicturesmodel.cpp PRE-CREATION browsingbackends/onlineservices/picasa/picasabackend.h 83b3eb2 browsingbackends/onlineservices/picasa/picasabackend.cpp b01ba4a browsingbackends/onlineservices/picasa/picasacomponents/PicasaSidePanel.qml d15a46e libs/mediacenter/abstractbrowsingbackend.h 14ad502 libs/mediacenter/abstractbrowsingbackend.cpp 6cd32f8 libs/mediacenter/backendsmodel.cpp 2823fb6 libs/mediacenter/filtermediamodel.h b8272f6 mediaelements/mediabrowser/MediaBrowser.qml b4ebdf9 shells/newshell/package/contents/ui/mediacenter.qml 15c9cee Diff: https://git.reviewboard.kde.org/r/119250/diff/ Testing --- Seems to work as expected, I will be attaching screenshots. File Attachments Showing events https://git.reviewboard.kde.org/media/uploaded/files/2014/07/12/c4c99bbd-f77f-4167-922e-79f90949040b__events.png Showing photos from an event https://git.reviewboard.kde.org/media/uploaded/files/2014/07/12/a4ed7663-64b6-4963-aed3-5e41cc8e51ee__showing-event.png Editing dates of an existing event https://git.reviewboard.kde.org/media/uploaded/files/2014/07/12/965b667b-e209-44a3-bc64-95f81b18913e__editing-event.png Thanks, Shantanu Tushar ___ Plasma-devel mailing list Plasma-devel@kde.org https://mail.kde.org/mailman/listinfo/plasma-devel
Re: Review Request 119250: Events and trips according to date ranges
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/119250/ --- (Updated July 20, 2014, 3:08 p.m.) Review request for Plasma. Changes --- Fix some UI bugs Repository: plasma-mediacenter Description --- I got this idea on a recent trip that usually a family would have a device where they store their trips/events photos on. Easier than tagging everything, what if you could just say Hey, I went to Kashmir during 5 April to 14 April 2014 and voila! This backend implements this idea in a (hopefully) easy-to-use way. I'll love to hear some feedback around this :) Diffs (updated) - browsingbackends/metadatabackends/eventsbackend/eventsfiltermodel.cpp PRE-CREATION browsingbackends/metadatabackends/eventsbackend/eventsmodel.h PRE-CREATION browsingbackends/metadatabackends/eventsbackend/eventsmodel.cpp PRE-CREATION browsingbackends/metadatabackends/eventsbackend/eventspicturesmodel.h PRE-CREATION browsingbackends/metadatabackends/eventsbackend/eventspicturesmodel.cpp PRE-CREATION browsingbackends/onlineservices/picasa/picasabackend.h 83b3eb2 browsingbackends/onlineservices/picasa/picasabackend.cpp b01ba4a browsingbackends/onlineservices/picasa/picasacomponents/PicasaSidePanel.qml d15a46e libs/mediacenter/abstractbrowsingbackend.h 14ad502 libs/mediacenter/abstractbrowsingbackend.cpp 6cd32f8 libs/mediacenter/backendsmodel.cpp 2823fb6 libs/mediacenter/filtermediamodel.h b8272f6 mediaelements/mediabrowser/MediaBrowser.qml b4ebdf9 shells/newshell/package/contents/ui/mediacenter.qml 15c9cee browsingbackends/metadatabackends/CMakeLists.txt ae56603 browsingbackends/metadatabackends/eventsbackend/CMakeLists.txt PRE-CREATION browsingbackends/metadatabackends/eventsbackend/eventsbackend.cpp PRE-CREATION browsingbackends/metadatabackends/eventsbackend/eventsbackend.desktop PRE-CREATION browsingbackends/metadatabackends/eventsbackend/eventsbackend.h PRE-CREATION browsingbackends/metadatabackends/eventsbackend/eventscomponents/DatePicker.qml PRE-CREATION browsingbackends/metadatabackends/eventsbackend/eventscomponents/Digit.qml PRE-CREATION browsingbackends/metadatabackends/eventsbackend/eventscomponents/EventsConfiguration.qml PRE-CREATION browsingbackends/metadatabackends/eventsbackend/eventscomponents/qmldir PRE-CREATION browsingbackends/metadatabackends/eventsbackend/eventsfiltermodel.h PRE-CREATION Diff: https://git.reviewboard.kde.org/r/119250/diff/ Testing --- Seems to work as expected, I will be attaching screenshots. File Attachments Showing events https://git.reviewboard.kde.org/media/uploaded/files/2014/07/12/c4c99bbd-f77f-4167-922e-79f90949040b__events.png Showing photos from an event https://git.reviewboard.kde.org/media/uploaded/files/2014/07/12/a4ed7663-64b6-4963-aed3-5e41cc8e51ee__showing-event.png Editing dates of an existing event https://git.reviewboard.kde.org/media/uploaded/files/2014/07/12/965b667b-e209-44a3-bc64-95f81b18913e__editing-event.png Thanks, Shantanu Tushar ___ Plasma-devel mailing list Plasma-devel@kde.org https://mail.kde.org/mailman/listinfo/plasma-devel
Re: Review Request 119250: Events and trips according to date ranges
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/119250/ --- (Updated July 20, 2014, 5:14 p.m.) Review request for Plasma. Changes --- Prevent empty eventName or an existing eventName when not editing Also added keyboard trigerring Repository: plasma-mediacenter Description --- I got this idea on a recent trip that usually a family would have a device where they store their trips/events photos on. Easier than tagging everything, what if you could just say Hey, I went to Kashmir during 5 April to 14 April 2014 and voila! This backend implements this idea in a (hopefully) easy-to-use way. I'll love to hear some feedback around this :) Diffs (updated) - browsingbackends/metadatabackends/CMakeLists.txt ae56603 browsingbackends/metadatabackends/eventsbackend/CMakeLists.txt PRE-CREATION browsingbackends/metadatabackends/eventsbackend/eventsbackend.cpp PRE-CREATION browsingbackends/metadatabackends/eventsbackend/eventsbackend.desktop PRE-CREATION browsingbackends/metadatabackends/eventsbackend/eventsbackend.h PRE-CREATION browsingbackends/metadatabackends/eventsbackend/eventscomponents/DatePicker.qml PRE-CREATION browsingbackends/metadatabackends/eventsbackend/eventscomponents/Digit.qml PRE-CREATION browsingbackends/metadatabackends/eventsbackend/eventscomponents/EventsConfiguration.qml PRE-CREATION browsingbackends/metadatabackends/eventsbackend/eventscomponents/qmldir PRE-CREATION browsingbackends/metadatabackends/eventsbackend/eventsfiltermodel.h PRE-CREATION browsingbackends/metadatabackends/eventsbackend/eventsfiltermodel.cpp PRE-CREATION browsingbackends/metadatabackends/eventsbackend/eventsmodel.h PRE-CREATION browsingbackends/metadatabackends/eventsbackend/eventsmodel.cpp PRE-CREATION browsingbackends/metadatabackends/eventsbackend/eventspicturesmodel.h PRE-CREATION browsingbackends/metadatabackends/eventsbackend/eventspicturesmodel.cpp PRE-CREATION browsingbackends/onlineservices/picasa/picasabackend.h 83b3eb2 browsingbackends/onlineservices/picasa/picasabackend.cpp b01ba4a browsingbackends/onlineservices/picasa/picasacomponents/PicasaSidePanel.qml d15a46e libs/mediacenter/abstractbrowsingbackend.h 14ad502 libs/mediacenter/abstractbrowsingbackend.cpp 6cd32f8 libs/mediacenter/backendsmodel.cpp 2823fb6 libs/mediacenter/filtermediamodel.h b8272f6 mediaelements/mediabrowser/MediaBrowser.qml b4ebdf9 shells/newshell/package/contents/ui/mediacenter.qml 15c9cee Diff: https://git.reviewboard.kde.org/r/119250/diff/ Testing --- Seems to work as expected, I will be attaching screenshots. File Attachments Showing events https://git.reviewboard.kde.org/media/uploaded/files/2014/07/12/c4c99bbd-f77f-4167-922e-79f90949040b__events.png Showing photos from an event https://git.reviewboard.kde.org/media/uploaded/files/2014/07/12/a4ed7663-64b6-4963-aed3-5e41cc8e51ee__showing-event.png Editing dates of an existing event https://git.reviewboard.kde.org/media/uploaded/files/2014/07/12/965b667b-e209-44a3-bc64-95f81b18913e__editing-event.png Thanks, Shantanu Tushar ___ Plasma-devel mailing list Plasma-devel@kde.org https://mail.kde.org/mailman/listinfo/plasma-devel
Re: Review Request 119250: Events and trips according to date ranges
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/119250/ --- (Updated July 19, 2014, 2:37 p.m.) Review request for Plasma. Changes --- Fix bugs noted in the reviews. Note that this no longer lets the user edit the name of an existing event. Repository: plasma-mediacenter Description --- I got this idea on a recent trip that usually a family would have a device where they store their trips/events photos on. Easier than tagging everything, what if you could just say Hey, I went to Kashmir during 5 April to 14 April 2014 and voila! This backend implements this idea in a (hopefully) easy-to-use way. I'll love to hear some feedback around this :) Diffs (updated) - browsingbackends/metadatabackends/eventsbackend/eventspicturesmodel.h PRE-CREATION browsingbackends/metadatabackends/eventsbackend/eventspicturesmodel.cpp PRE-CREATION browsingbackends/onlineservices/picasa/picasabackend.h 83b3eb2 browsingbackends/metadatabackends/eventsbackend/eventsfiltermodel.h PRE-CREATION browsingbackends/metadatabackends/eventsbackend/eventsfiltermodel.cpp PRE-CREATION browsingbackends/metadatabackends/eventsbackend/eventsmodel.h PRE-CREATION browsingbackends/metadatabackends/eventsbackend/eventsmodel.cpp PRE-CREATION browsingbackends/metadatabackends/CMakeLists.txt ae56603 browsingbackends/metadatabackends/eventsbackend/CMakeLists.txt PRE-CREATION browsingbackends/metadatabackends/eventsbackend/eventsbackend.cpp PRE-CREATION browsingbackends/metadatabackends/eventsbackend/eventsbackend.desktop PRE-CREATION browsingbackends/metadatabackends/eventsbackend/eventsbackend.h PRE-CREATION browsingbackends/metadatabackends/eventsbackend/eventscomponents/DatePicker.qml PRE-CREATION browsingbackends/metadatabackends/eventsbackend/eventscomponents/Digit.qml PRE-CREATION browsingbackends/metadatabackends/eventsbackend/eventscomponents/EventsConfiguration.qml PRE-CREATION browsingbackends/metadatabackends/eventsbackend/eventscomponents/qmldir PRE-CREATION browsingbackends/onlineservices/picasa/picasabackend.cpp b01ba4a browsingbackends/onlineservices/picasa/picasacomponents/PicasaSidePanel.qml d15a46e libs/mediacenter/abstractbrowsingbackend.h 14ad502 libs/mediacenter/abstractbrowsingbackend.cpp 6cd32f8 shells/newshell/package/contents/ui/mediacenter.qml 15c9cee libs/mediacenter/backendsmodel.cpp 2823fb6 libs/mediacenter/filtermediamodel.h b8272f6 mediaelements/mediabrowser/MediaBrowser.qml b4ebdf9 Diff: https://git.reviewboard.kde.org/r/119250/diff/ Testing --- Seems to work as expected, I will be attaching screenshots. File Attachments Showing events https://git.reviewboard.kde.org/media/uploaded/files/2014/07/12/c4c99bbd-f77f-4167-922e-79f90949040b__events.png Showing photos from an event https://git.reviewboard.kde.org/media/uploaded/files/2014/07/12/a4ed7663-64b6-4963-aed3-5e41cc8e51ee__showing-event.png Editing dates of an existing event https://git.reviewboard.kde.org/media/uploaded/files/2014/07/12/965b667b-e209-44a3-bc64-95f81b18913e__editing-event.png Thanks, Shantanu Tushar ___ Plasma-devel mailing list Plasma-devel@kde.org https://mail.kde.org/mailman/listinfo/plasma-devel
Re: Plasma shell - Mediacenter
Hi, On Mon, Jul 14, 2014 at 2:16 PM, Marco Martin notm...@gmail.com wrote: On Saturday 12 July 2014, Shantanu Tushar Jha wrote: Hi folks, The frameworks branch of PMC now contains a Plasma Shell which basically loads the required QML components and looks/works exactly the same[1]. So technically we are good to go if a user wants her desktop to be a media center. However, it'll be nice if we can discuss on how the actual workflow could be for a user. Here are some things I can imagine- * User can have a mediacenter activity which she can switch to for getting the multimedia experience. I'm not sure how much it can work as activity, if you want that tough most o the mediacenter UI QML should be moved in a single containment (that may be good as well) while now is iirc mostly in the view of the desktop package Do you mean that the current implementation won't support it, or the idea itself is not good? * Or, maybe she can configure the mediacenter shell to activate whenever an external TV screen is connected (through HDMI or something). it could either switch the shell or just launch the standalone application (launching the application should be easy, maybe configurable from kscreen kcm?) Or, maybe plasmashell exposes a D-Bus API that can load shells on the fly? +1 for the kscreen kcm, I think it should be a nice feature in general. * The device is a dedicated mediacenter device - the user tells Plasma to always load mediacenter instead of deskop I think there should eventually be a session shipped by it so is just possible to login into it from the login manager Guess the session will always run the same startkde script, how can we tell plasmashell (rhymes :P) what to load? -- Marco Martin ___ Plasma-devel mailing list Plasma-devel@kde.org https://mail.kde.org/mailman/listinfo/plasma-devel -- Shantanu Tushar(UTC +0530) http://www.shantanutushar.com ___ Plasma-devel mailing list Plasma-devel@kde.org https://mail.kde.org/mailman/listinfo/plasma-devel
Re: Extending fullscreen option to all pages in PMC
KWin has a standard Ctrl+Shift+F shortcut to do that, but somehow it doesn't work. Maybe you can try and figure out why. On Tue, Jul 15, 2014 at 4:51 PM, Devanshu Jain devanshu.jain...@gmail.com wrote: Hi guys! I was wondering if it would be nice that the user can switch at any time to full screen, by pressing a key instead of going to home screen every time and going to settings menu. ___ Plasma-devel mailing list Plasma-devel@kde.org https://mail.kde.org/mailman/listinfo/plasma-devel -- Shantanu Tushar(UTC +0530) http://www.shantanutushar.com ___ Plasma-devel mailing list Plasma-devel@kde.org https://mail.kde.org/mailman/listinfo/plasma-devel
Re: Review Request 119250: Events and trips according to date ranges
On July 12, 2014, 7:39 p.m., R.Harish Navnit wrote: I liked the interface and the idea, +1 for that. Some of the issues that I faced are(brief testing): 1. Create an event - Exit PMC - add new event - unable to do so :( 2. [Logical] Was able to create an event/trip from October to July 3. Something that I couldn't figure out, how do I add content to the trip/event folders ? Edit option only allows me to change the timeline, should have an option to add/remove images. Otherwise, this would be nice feature to have in PMC. Besides, 2 would be a junior job that someone else could attempt to fix, not critical but pointed it out nevertheless. 1, not sure what you mean, I can create an event, exit pmc and can add more events as well, works fine. 2. good point, I should have a check for that. Will upload a fresh patch. 3. well the idea is that you dont manually add images (that you can anyway do using folders), but yes we can have an option of excluding some images. - Shantanu --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/119250/#review62215 --- On July 12, 2014, 6:13 p.m., Shantanu Tushar wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/119250/ --- (Updated July 12, 2014, 6:13 p.m.) Review request for Plasma. Repository: plasma-mediacenter Description --- I got this idea on a recent trip that usually a family would have a device where they store their trips/events photos on. Easier than tagging everything, what if you could just say Hey, I went to Kashmir during 5 April to 14 April 2014 and voila! This backend implements this idea in a (hopefully) easy-to-use way. I'll love to hear some feedback around this :) Diffs - browsingbackends/metadatabackends/eventsbackend/eventsmodel.cpp PRE-CREATION browsingbackends/metadatabackends/CMakeLists.txt ae56603 browsingbackends/metadatabackends/eventsbackend/CMakeLists.txt PRE-CREATION browsingbackends/metadatabackends/eventsbackend/eventsbackend.cpp PRE-CREATION browsingbackends/metadatabackends/eventsbackend/eventspicturesmodel.h PRE-CREATION browsingbackends/metadatabackends/eventsbackend/eventspicturesmodel.cpp PRE-CREATION libs/mediacenter/backendsmodel.cpp 2823fb6 libs/mediacenter/filtermediamodel.h b8272f6 browsingbackends/metadatabackends/eventsbackend/eventsbackend.desktop PRE-CREATION browsingbackends/metadatabackends/eventsbackend/eventsbackend.h PRE-CREATION browsingbackends/metadatabackends/eventsbackend/eventscomponents/DatePicker.qml PRE-CREATION browsingbackends/metadatabackends/eventsbackend/eventscomponents/Digit.qml PRE-CREATION browsingbackends/metadatabackends/eventsbackend/eventscomponents/EventsConfiguration.qml PRE-CREATION browsingbackends/metadatabackends/eventsbackend/eventscomponents/qmldir PRE-CREATION browsingbackends/metadatabackends/eventsbackend/eventsfiltermodel.h PRE-CREATION browsingbackends/metadatabackends/eventsbackend/eventsfiltermodel.cpp PRE-CREATION browsingbackends/metadatabackends/eventsbackend/eventsmodel.h PRE-CREATION Diff: https://git.reviewboard.kde.org/r/119250/diff/ Testing --- Seems to work as expected, I will be attaching screenshots. File Attachments Showing events https://git.reviewboard.kde.org/media/uploaded/files/2014/07/12/c4c99bbd-f77f-4167-922e-79f90949040b__events.png Showing photos from an event https://git.reviewboard.kde.org/media/uploaded/files/2014/07/12/a4ed7663-64b6-4963-aed3-5e41cc8e51ee__showing-event.png Editing dates of an existing event https://git.reviewboard.kde.org/media/uploaded/files/2014/07/12/965b667b-e209-44a3-bc64-95f81b18913e__editing-event.png Thanks, Shantanu Tushar ___ Plasma-devel mailing list Plasma-devel@kde.org https://mail.kde.org/mailman/listinfo/plasma-devel
Plasma shell - Mediacenter
Hi folks, The frameworks branch of PMC now contains a Plasma Shell which basically loads the required QML components and looks/works exactly the same[1]. So technically we are good to go if a user wants her desktop to be a media center. However, it'll be nice if we can discuss on how the actual workflow could be for a user. Here are some things I can imagine- * User can have a mediacenter activity which she can switch to for getting the multimedia experience. * Or, maybe she can configure the mediacenter shell to activate whenever an external TV screen is connected (through HDMI or something). * The device is a dedicated mediacenter device - the user tells Plasma to always load mediacenter instead of deskop Any other ideas? Thoughts? [1] http://i.imgur.com/J16niw7.jpg Cheers! -- Shantanu Tushar(UTC +0530) http://www.shantanutushar.com ___ Plasma-devel mailing list Plasma-devel@kde.org https://mail.kde.org/mailman/listinfo/plasma-devel
Review Request 119250: Events and trips according to date ranges
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/119250/ --- Review request for Plasma. Repository: plasma-mediacenter Description --- I got this idea on a recent trip that usually a family would have a device where they store their trips/events photos on. Easier than tagging everything, what if you could just say Hey, I went to Kashmir during 5 April to 14 April 2014 and voila! This backend implements this idea in a (hopefully) easy-to-use way. I'll love to hear some feedback around this :) Diffs - browsingbackends/metadatabackends/eventsbackend/eventsmodel.cpp PRE-CREATION browsingbackends/metadatabackends/CMakeLists.txt ae56603 browsingbackends/metadatabackends/eventsbackend/CMakeLists.txt PRE-CREATION browsingbackends/metadatabackends/eventsbackend/eventsbackend.cpp PRE-CREATION browsingbackends/metadatabackends/eventsbackend/eventspicturesmodel.h PRE-CREATION browsingbackends/metadatabackends/eventsbackend/eventspicturesmodel.cpp PRE-CREATION libs/mediacenter/backendsmodel.cpp 2823fb6 libs/mediacenter/filtermediamodel.h b8272f6 browsingbackends/metadatabackends/eventsbackend/eventsbackend.desktop PRE-CREATION browsingbackends/metadatabackends/eventsbackend/eventsbackend.h PRE-CREATION browsingbackends/metadatabackends/eventsbackend/eventscomponents/DatePicker.qml PRE-CREATION browsingbackends/metadatabackends/eventsbackend/eventscomponents/Digit.qml PRE-CREATION browsingbackends/metadatabackends/eventsbackend/eventscomponents/EventsConfiguration.qml PRE-CREATION browsingbackends/metadatabackends/eventsbackend/eventscomponents/qmldir PRE-CREATION browsingbackends/metadatabackends/eventsbackend/eventsfiltermodel.h PRE-CREATION browsingbackends/metadatabackends/eventsbackend/eventsfiltermodel.cpp PRE-CREATION browsingbackends/metadatabackends/eventsbackend/eventsmodel.h PRE-CREATION Diff: https://git.reviewboard.kde.org/r/119250/diff/ Testing --- Seems to work as expected, I will be attaching screenshots. Thanks, Shantanu Tushar ___ Plasma-devel mailing list Plasma-devel@kde.org https://mail.kde.org/mailman/listinfo/plasma-devel
Re: Review Request 119250: Events and trips according to date ranges
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/119250/ --- (Updated July 12, 2014, 6:13 p.m.) Review request for Plasma. Changes --- Adding screenshots Repository: plasma-mediacenter Description --- I got this idea on a recent trip that usually a family would have a device where they store their trips/events photos on. Easier than tagging everything, what if you could just say Hey, I went to Kashmir during 5 April to 14 April 2014 and voila! This backend implements this idea in a (hopefully) easy-to-use way. I'll love to hear some feedback around this :) Diffs - browsingbackends/metadatabackends/eventsbackend/eventsmodel.cpp PRE-CREATION browsingbackends/metadatabackends/CMakeLists.txt ae56603 browsingbackends/metadatabackends/eventsbackend/CMakeLists.txt PRE-CREATION browsingbackends/metadatabackends/eventsbackend/eventsbackend.cpp PRE-CREATION browsingbackends/metadatabackends/eventsbackend/eventspicturesmodel.h PRE-CREATION browsingbackends/metadatabackends/eventsbackend/eventspicturesmodel.cpp PRE-CREATION libs/mediacenter/backendsmodel.cpp 2823fb6 libs/mediacenter/filtermediamodel.h b8272f6 browsingbackends/metadatabackends/eventsbackend/eventsbackend.desktop PRE-CREATION browsingbackends/metadatabackends/eventsbackend/eventsbackend.h PRE-CREATION browsingbackends/metadatabackends/eventsbackend/eventscomponents/DatePicker.qml PRE-CREATION browsingbackends/metadatabackends/eventsbackend/eventscomponents/Digit.qml PRE-CREATION browsingbackends/metadatabackends/eventsbackend/eventscomponents/EventsConfiguration.qml PRE-CREATION browsingbackends/metadatabackends/eventsbackend/eventscomponents/qmldir PRE-CREATION browsingbackends/metadatabackends/eventsbackend/eventsfiltermodel.h PRE-CREATION browsingbackends/metadatabackends/eventsbackend/eventsfiltermodel.cpp PRE-CREATION browsingbackends/metadatabackends/eventsbackend/eventsmodel.h PRE-CREATION Diff: https://git.reviewboard.kde.org/r/119250/diff/ Testing --- Seems to work as expected, I will be attaching screenshots. File Attachments (updated) Showing events https://git.reviewboard.kde.org/media/uploaded/files/2014/07/12/c4c99bbd-f77f-4167-922e-79f90949040b__events.png Showing photos from an event https://git.reviewboard.kde.org/media/uploaded/files/2014/07/12/a4ed7663-64b6-4963-aed3-5e41cc8e51ee__showing-event.png Editing dates of an existing event https://git.reviewboard.kde.org/media/uploaded/files/2014/07/12/965b667b-e209-44a3-bc64-95f81b18913e__editing-event.png Thanks, Shantanu Tushar ___ Plasma-devel mailing list Plasma-devel@kde.org https://mail.kde.org/mailman/listinfo/plasma-devel
Re: Review Request 119219: Control volume from HomeScreen
On July 10, 2014, 6:36 p.m., Shantanu Tushar wrote: My only problem with this is that its inconsistent with the Up/Down arrow key combination that we have in MediaPlayer. Also, you can't make the player also use Ctrl+Arrow keys because then a remote control won't work. I'd suggest just making sure that the standard media keys work (play,pause etc) so that people can, for example, pause stuff when they get a phone call. To control the volume they can always press Esc and Up/Down. R.Harish Navnit wrote: Why is it inconsistent ? I mean I'm able to use both(just the arrow keys and the combination). I've no idea of how a remote control would affect things so I don't know what that would require. I think the standard media keys do work. Yes, you can play/pause, no problems there. but are you suggesting that I use the combination Esc + Up/Down ? Well, I think that's inconsistent since the Esc key is being used as a shortcut to go to the previous screen. However, we do have the backspace key doing the same. I could change that if that's what you mean ? P.S: I'm not sure if I've got what you're trying to convey. Is what I mentioned above right ? Shantanu Tushar wrote: Its inconsistent because the player needs only Up/Down (try it) but this one needs Ctrl as well (ideally you'd want the same shortcuts everywhere). Remote control concept is simple, every action should be possible use arrow keys, enter, esc. I meant you can press Esc from the home screen to go to the player, and then Up/Down to control the volume. R.Harish Navnit wrote: Yes, the player volume can be controlled using the arrow keys, but they won't work when you're not in the player screen. The user can get away if he's in the home screen, but if the user was, let's say browsing through his pictures collection then pressing the Esc key is not really a good choice(3/4 presses of Esc key would take you to the player and only then would pressing the arrow key work), isn't that inconvenient ? We can have the same shortcuts everywhere(merely arrows won't work, since they're used for navigation in the homescreen etc), we could have a combination for the same, if not ctrl + arrows, how about using shift ? Do you have any suggestions ? Yep, those are the media volume up/down keys (assuming your keyboard has them). - Shantanu --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/119219/#review62090 --- On July 10, 2014, 6:16 p.m., R.Harish Navnit wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/119219/ --- (Updated July 10, 2014, 6:16 p.m.) Review request for Plasma, Shantanu Tushar and Sinny Kumari. Repository: plasma-mediacenter Description --- AFAIK, volume of a playing media cannot be controlled from the HomeScreen. This patch enables increasing and decreasing the volume using the combination of Ctrl + Up/Down keys. Diffs - shells/newshell/package/contents/ui/mediacenter.qml 15c9cee7f86573939ac92872fb2201d944e0589f Diff: https://git.reviewboard.kde.org/r/119219/diff/ Testing --- Use the ctrl + up/down keys to increase/decrease the volume of the current playing media. seems to work fine. Thanks, R.Harish Navnit ___ Plasma-devel mailing list Plasma-devel@kde.org https://mail.kde.org/mailman/listinfo/plasma-devel
Re: cmake does not work
Hi Devanshu, What distro are you using? Also, how have you installed Qt? On Thu, Jul 10, 2014 at 4:48 PM, Devanshu Jain devanshu.jain...@gmail.com wrote: Hi Harish, Thanks for pastebin suggestion. I would keep it in mind next time. As for the typos: No I didn't make any typos. I double checked everything. On Thu, Jul 10, 2014 at 4:37 PM, R.Harish Navnit harishnav...@gmail.com wrote: On Thu, Jul 10, 2014 at 12:35 PM, Devanshu Jain devanshu.jain...@gmail.com wrote: Please find the enclosed cmake output log file Hi Devanshu, I would suggest using pastebin http://paste.kde.org instead of attaching the output logs in your mails. I think that's a lot easier and attachments in mails sent to ML's is not a good practice either. I don't think there's any hard and fast rule as such, but usually clones of projects that you wish to build and run are placed in the home dir, no ? (you may ignore this) and I hope you didn't make a typo while running the command that Sinny mentioned. Regards. R.Harish Navnit The Enigma http://harishnavnit.wordpress.com ___ Plasma-devel mailing list Plasma-devel@kde.org https://mail.kde.org/mailman/listinfo/plasma-devel ___ Plasma-devel mailing list Plasma-devel@kde.org https://mail.kde.org/mailman/listinfo/plasma-devel -- Shantanu Tushar(UTC +0530) http://www.shantanutushar.com ___ Plasma-devel mailing list Plasma-devel@kde.org https://mail.kde.org/mailman/listinfo/plasma-devel
Re: cmake does not work
Cool, though I had asked how have you installed Qt ;) Because I've never encountered this error. Anyway, you can even add the fPIC flag like this - cmake .. -DCMAKE_CXX_FLAGS=-fPIC -DCMAKE_INSTALL_PREFIX=`kde4-config --prefix` -DQT_QMAKE_EXECUTABLE=/usr/bin/qmake-qt4 -DKDE4_BUILD_TESTS=OFF On Thu, Jul 10, 2014 at 8:33 PM, Devanshu Jain devanshu.jain...@gmail.com wrote: Hi Shantanu, I am using Ubuntu 14.04 and yes I have Qt installed. On Thu, Jul 10, 2014 at 8:26 PM, Shantanu Tushar Jha shant...@kde.org wrote: Hi Devanshu, What distro are you using? Also, how have you installed Qt? On Thu, Jul 10, 2014 at 4:48 PM, Devanshu Jain devanshu.jain...@gmail.com wrote: Hi Harish, Thanks for pastebin suggestion. I would keep it in mind next time. As for the typos: No I didn't make any typos. I double checked everything. On Thu, Jul 10, 2014 at 4:37 PM, R.Harish Navnit harishnav...@gmail.com wrote: On Thu, Jul 10, 2014 at 12:35 PM, Devanshu Jain devanshu.jain...@gmail.com wrote: Please find the enclosed cmake output log file Hi Devanshu, I would suggest using pastebin http://paste.kde.org instead of attaching the output logs in your mails. I think that's a lot easier and attachments in mails sent to ML's is not a good practice either. I don't think there's any hard and fast rule as such, but usually clones of projects that you wish to build and run are placed in the home dir, no ? (you may ignore this) and I hope you didn't make a typo while running the command that Sinny mentioned. Regards. R.Harish Navnit The Enigma http://harishnavnit.wordpress.com ___ Plasma-devel mailing list Plasma-devel@kde.org https://mail.kde.org/mailman/listinfo/plasma-devel ___ Plasma-devel mailing list Plasma-devel@kde.org https://mail.kde.org/mailman/listinfo/plasma-devel -- Shantanu Tushar(UTC +0530) http://www.shantanutushar.com ___ Plasma-devel mailing list Plasma-devel@kde.org https://mail.kde.org/mailman/listinfo/plasma-devel ___ Plasma-devel mailing list Plasma-devel@kde.org https://mail.kde.org/mailman/listinfo/plasma-devel -- Shantanu Tushar(UTC +0530) http://www.shantanutushar.com ___ Plasma-devel mailing list Plasma-devel@kde.org https://mail.kde.org/mailman/listinfo/plasma-devel
Re: cmake does not work
try running this and pastebin the output - VERBOSE=1 make On Thu, Jul 10, 2014 at 9:57 PM, Devanshu Jain devanshu.jain...@gmail.com wrote: I started from scratch but make command is again giving the same error. On Thu, Jul 10, 2014 at 9:45 PM, Shantanu Tushar Jha shant...@kde.org wrote: Something tells me its still using Qt5 (and thats why it regards 4.8.6 as old), can you try removing your build dir and start from scratch? About Qt, do *not* use the one downloaded from the website, the one that you install using apt-get/synaptic is the way to go. On Thu, Jul 10, 2014 at 9:27 PM, Devanshu Jain devanshu.jain...@gmail.com wrote: Hi Shantanu, Sorry for misreading. But I downloaded Qt using synaptic. And I guess it is still causing troubles. When doing *make install*, another error is produced ( http://paste.kde.org/prtoragcp). It says: This file was generated using the moc from 4.8.6. It cannot be used with the include files from this version of Qt. (The moc has changed too much.) So I guess I will download it again from the official site. On Thu, Jul 10, 2014 at 9:15 PM, Shantanu Tushar Jha shaan...@gmail.com wrote: Cool, though I had asked how have you installed Qt ;) Because I've never encountered this error. Anyway, you can even add the fPIC flag like this - cmake .. -DCMAKE_CXX_FLAGS=-fPIC -DCMAKE_INSTALL_PREFIX=`kde4-config --prefix` -DQT_QMAKE_EXECUTABLE=/usr/bin/qmake-qt4 -DKDE4_BUILD_TESTS=OFF On Thu, Jul 10, 2014 at 8:33 PM, Devanshu Jain devanshu.jain...@gmail.com wrote: Hi Shantanu, I am using Ubuntu 14.04 and yes I have Qt installed. On Thu, Jul 10, 2014 at 8:26 PM, Shantanu Tushar Jha shant...@kde.org wrote: Hi Devanshu, What distro are you using? Also, how have you installed Qt? On Thu, Jul 10, 2014 at 4:48 PM, Devanshu Jain devanshu.jain...@gmail.com wrote: Hi Harish, Thanks for pastebin suggestion. I would keep it in mind next time. As for the typos: No I didn't make any typos. I double checked everything. On Thu, Jul 10, 2014 at 4:37 PM, R.Harish Navnit harishnav...@gmail.com wrote: On Thu, Jul 10, 2014 at 12:35 PM, Devanshu Jain devanshu.jain...@gmail.com wrote: Please find the enclosed cmake output log file Hi Devanshu, I would suggest using pastebin http://paste.kde.org instead of attaching the output logs in your mails. I think that's a lot easier and attachments in mails sent to ML's is not a good practice either. I don't think there's any hard and fast rule as such, but usually clones of projects that you wish to build and run are placed in the home dir, no ? (you may ignore this) and I hope you didn't make a typo while running the command that Sinny mentioned. Regards. R.Harish Navnit The Enigma http://harishnavnit.wordpress.com ___ Plasma-devel mailing list Plasma-devel@kde.org https://mail.kde.org/mailman/listinfo/plasma-devel ___ Plasma-devel mailing list Plasma-devel@kde.org https://mail.kde.org/mailman/listinfo/plasma-devel -- Shantanu Tushar(UTC +0530) http://www.shantanutushar.com ___ Plasma-devel mailing list Plasma-devel@kde.org https://mail.kde.org/mailman/listinfo/plasma-devel ___ Plasma-devel mailing list Plasma-devel@kde.org https://mail.kde.org/mailman/listinfo/plasma-devel -- Shantanu Tushar(UTC +0530) http://www.shantanutushar.com ___ Plasma-devel mailing list Plasma-devel@kde.org https://mail.kde.org/mailman/listinfo/plasma-devel ___ Plasma-devel mailing list Plasma-devel@kde.org https://mail.kde.org/mailman/listinfo/plasma-devel -- Shantanu Tushar(UTC +0530) http://www.shantanutushar.com ___ Plasma-devel mailing list Plasma-devel@kde.org https://mail.kde.org/mailman/listinfo/plasma-devel ___ Plasma-devel mailing list Plasma-devel@kde.org https://mail.kde.org/mailman/listinfo/plasma-devel -- Shantanu Tushar(UTC +0530) http://www.shantanutushar.com ___ Plasma-devel mailing list Plasma-devel@kde.org https://mail.kde.org/mailman/listinfo/plasma-devel
Re: cmake does not work
Hmm that is very strange. Edit the file build/libs/mediacenter/moc_mediasourcesloader.cpp and put these just after the != 63 condition- #error VERSION IS #error Q_MOC_OUTPUT_REVISION and then run make again and pastebin On Thu, Jul 10, 2014 at 10:35 PM, Devanshu Jain devanshu.jain...@gmail.com wrote: Here is the link to the output: http://paste.kde.org/pjyrnmzag On Thu, Jul 10, 2014 at 10:33 PM, Shantanu Tushar Jha shant...@kde.org wrote: try running this and pastebin the output - VERBOSE=1 make On Thu, Jul 10, 2014 at 9:57 PM, Devanshu Jain devanshu.jain...@gmail.com wrote: I started from scratch but make command is again giving the same error. On Thu, Jul 10, 2014 at 9:45 PM, Shantanu Tushar Jha shant...@kde.org wrote: Something tells me its still using Qt5 (and thats why it regards 4.8.6 as old), can you try removing your build dir and start from scratch? About Qt, do *not* use the one downloaded from the website, the one that you install using apt-get/synaptic is the way to go. On Thu, Jul 10, 2014 at 9:27 PM, Devanshu Jain devanshu.jain...@gmail.com wrote: Hi Shantanu, Sorry for misreading. But I downloaded Qt using synaptic. And I guess it is still causing troubles. When doing *make install*, another error is produced ( http://paste.kde.org/prtoragcp). It says: This file was generated using the moc from 4.8.6. It cannot be used with the include files from this version of Qt. (The moc has changed too much.) So I guess I will download it again from the official site. On Thu, Jul 10, 2014 at 9:15 PM, Shantanu Tushar Jha shaan...@gmail.com wrote: Cool, though I had asked how have you installed Qt ;) Because I've never encountered this error. Anyway, you can even add the fPIC flag like this - cmake .. -DCMAKE_CXX_FLAGS=-fPIC -DCMAKE_INSTALL_PREFIX=`kde4-config --prefix` -DQT_QMAKE_EXECUTABLE=/usr/bin/qmake-qt4 -DKDE4_BUILD_TESTS=OFF On Thu, Jul 10, 2014 at 8:33 PM, Devanshu Jain devanshu.jain...@gmail.com wrote: Hi Shantanu, I am using Ubuntu 14.04 and yes I have Qt installed. On Thu, Jul 10, 2014 at 8:26 PM, Shantanu Tushar Jha shant...@kde.org wrote: Hi Devanshu, What distro are you using? Also, how have you installed Qt? On Thu, Jul 10, 2014 at 4:48 PM, Devanshu Jain devanshu.jain...@gmail.com wrote: Hi Harish, Thanks for pastebin suggestion. I would keep it in mind next time. As for the typos: No I didn't make any typos. I double checked everything. On Thu, Jul 10, 2014 at 4:37 PM, R.Harish Navnit harishnav...@gmail.com wrote: On Thu, Jul 10, 2014 at 12:35 PM, Devanshu Jain devanshu.jain...@gmail.com wrote: Please find the enclosed cmake output log file Hi Devanshu, I would suggest using pastebin http://paste.kde.org instead of attaching the output logs in your mails. I think that's a lot easier and attachments in mails sent to ML's is not a good practice either. I don't think there's any hard and fast rule as such, but usually clones of projects that you wish to build and run are placed in the home dir, no ? (you may ignore this) and I hope you didn't make a typo while running the command that Sinny mentioned. Regards. R.Harish Navnit The Enigma http://harishnavnit.wordpress.com ___ Plasma-devel mailing list Plasma-devel@kde.org https://mail.kde.org/mailman/listinfo/plasma-devel ___ Plasma-devel mailing list Plasma-devel@kde.org https://mail.kde.org/mailman/listinfo/plasma-devel -- Shantanu Tushar(UTC +0530) http://www.shantanutushar.com ___ Plasma-devel mailing list Plasma-devel@kde.org https://mail.kde.org/mailman/listinfo/plasma-devel ___ Plasma-devel mailing list Plasma-devel@kde.org https://mail.kde.org/mailman/listinfo/plasma-devel -- Shantanu Tushar(UTC +0530) http://www.shantanutushar.com ___ Plasma-devel mailing list Plasma-devel@kde.org https://mail.kde.org/mailman/listinfo/plasma-devel ___ Plasma-devel mailing list Plasma-devel@kde.org https://mail.kde.org/mailman/listinfo/plasma-devel -- Shantanu Tushar(UTC +0530) http://www.shantanutushar.com ___ Plasma-devel mailing list Plasma-devel@kde.org https://mail.kde.org/mailman/listinfo/plasma-devel ___ Plasma-devel mailing list Plasma-devel@kde.org https://mail.kde.org/mailman/listinfo/plasma-devel -- Shantanu Tushar(UTC +0530) http://www.shantanutushar.com ___ Plasma-devel mailing list Plasma-devel@kde.org https://mail.kde.org/mailman/listinfo/plasma-devel ___ Plasma-devel mailing
Re: Review Request 119219: Control volume from HomeScreen
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/119219/#review62090 --- My only problem with this is that its inconsistent with the Up/Down arrow key combination that we have in MediaPlayer. Also, you can't make the player also use Ctrl+Arrow keys because then a remote control won't work. I'd suggest just making sure that the standard media keys work (play,pause etc) so that people can, for example, pause stuff when they get a phone call. To control the volume they can always press Esc and Up/Down. - Shantanu Tushar On July 10, 2014, 6:16 p.m., R.Harish Navnit wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/119219/ --- (Updated July 10, 2014, 6:16 p.m.) Review request for Plasma, Shantanu Tushar and Sinny Kumari. Repository: plasma-mediacenter Description --- AFAIK, volume of a playing media cannot be controlled from the HomeScreen. This patch enables increasing and decreasing the volume using the combination of Ctrl + Up/Down keys. Diffs - shells/newshell/package/contents/ui/mediacenter.qml 15c9cee7f86573939ac92872fb2201d944e0589f Diff: https://git.reviewboard.kde.org/r/119219/diff/ Testing --- Use the ctrl + up/down keys to increase/decrease the volume of the current playing media. seems to work fine. Thanks, R.Harish Navnit ___ Plasma-devel mailing list Plasma-devel@kde.org https://mail.kde.org/mailman/listinfo/plasma-devel
Re: Review Request 119219: Control volume from HomeScreen
On July 10, 2014, 6:36 p.m., Shantanu Tushar wrote: My only problem with this is that its inconsistent with the Up/Down arrow key combination that we have in MediaPlayer. Also, you can't make the player also use Ctrl+Arrow keys because then a remote control won't work. I'd suggest just making sure that the standard media keys work (play,pause etc) so that people can, for example, pause stuff when they get a phone call. To control the volume they can always press Esc and Up/Down. R.Harish Navnit wrote: Why is it inconsistent ? I mean I'm able to use both(just the arrow keys and the combination). I've no idea of how a remote control would affect things so I don't know what that would require. I think the standard media keys do work. Yes, you can play/pause, no problems there. but are you suggesting that I use the combination Esc + Up/Down ? Well, I think that's inconsistent since the Esc key is being used as a shortcut to go to the previous screen. However, we do have the backspace key doing the same. I could change that if that's what you mean ? P.S: I'm not sure if I've got what you're trying to convey. Is what I mentioned above right ? Its inconsistent because the player needs only Up/Down (try it) but this one needs Ctrl as well (ideally you'd want the same shortcuts everywhere). Remote control concept is simple, every action should be possible use arrow keys, enter, esc. I meant you can press Esc from the home screen to go to the player, and then Up/Down to control the volume. - Shantanu --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/119219/#review62090 --- On July 10, 2014, 6:16 p.m., R.Harish Navnit wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/119219/ --- (Updated July 10, 2014, 6:16 p.m.) Review request for Plasma, Shantanu Tushar and Sinny Kumari. Repository: plasma-mediacenter Description --- AFAIK, volume of a playing media cannot be controlled from the HomeScreen. This patch enables increasing and decreasing the volume using the combination of Ctrl + Up/Down keys. Diffs - shells/newshell/package/contents/ui/mediacenter.qml 15c9cee7f86573939ac92872fb2201d944e0589f Diff: https://git.reviewboard.kde.org/r/119219/diff/ Testing --- Use the ctrl + up/down keys to increase/decrease the volume of the current playing media. seems to work fine. Thanks, R.Harish Navnit ___ Plasma-devel mailing list Plasma-devel@kde.org https://mail.kde.org/mailman/listinfo/plasma-devel
Re: Review Request 119015: Fixing full screen settings on Plasma Mediacenter exit
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/119015/#review61494 --- Ship it! Looks to be good, just a small modification and this can go in. browsingbackends/utility/exit/exitbackend.cpp https://git.reviewboard.kde.org/r/119015/#comment42806 this should not be needed anymore - Shantanu Tushar On June 29, 2014, 3:53 p.m., Sujith Haridasan wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/119015/ --- (Updated June 29, 2014, 3:53 p.m.) Review request for Plasma, Shantanu Tushar and Sinny Kumari. Repository: plasma-mediacenter Description --- This patch fixes the full screen settings of plasma-mediacenter during exit. Though its bit hackish which I had done. I would like to make this done better. With this change the pmc will save the fullscreen settings before exit. Diffs - browsingbackends/utility/exit/CMakeLists.txt 48d12a7 browsingbackends/utility/exit/exitbackend.cpp 9fe6cad shells/newshell/mainwindow.cpp 97655d8 Diff: https://git.reviewboard.kde.org/r/119015/diff/ Testing --- Done testing with fullscreen enabled and exit fullscreen disabled and exit. Both results were as expected. Thanks, Sujith Haridasan ___ Plasma-devel mailing list Plasma-devel@kde.org https://mail.kde.org/mailman/listinfo/plasma-devel
Re: Review Request 116874: Patch for the next and previous button functionality in an inactive playlist.
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/116874/#review60666 --- Looks good to me, just one suggestion mediaelements/mediacontroller/MediaController.qml https://git.reviewboard.kde.org/r/116874/#comment42305 runtimeData.playing || runtimeData.paused should work better, thats usually the case in other media players - Shantanu Tushar On June 19, 2014, 9:04 p.m., R.Harish Navnit wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/116874/ --- (Updated June 19, 2014, 9:04 p.m.) Review request for Plasma, Shantanu Tushar and Sinny Kumari. Bugs: 330990 and Bug http://bugs.kde.org/show_bug.cgi?id=330990 http://bugs.kde.org/show_bug.cgi?id=Bug Repository: plasma-mediacenter Description --- Now the next and previous buttons do not work unless a media is playing. Diffs - mediaelements/mediacontroller/MediaController.qml 2fce0a0 Diff: https://git.reviewboard.kde.org/r/116874/diff/ Testing --- I've done the testing for this patch. These are the steps that I followed. 1. Added a new playlist and added a few songs to the playlist. 2. Re-opened Plasma Media Center 3. Clicked the Next and Previous buttons. 4. Nothing was played. 5. Selected a song from the list and then tested the next and previous buttons. 6. The buttons worked seamlessly. Thanks, R.Harish Navnit ___ Plasma-devel mailing list Plasma-devel@kde.org https://mail.kde.org/mailman/listinfo/plasma-devel
Re: Review Request 116874: Patch for the next and previous button functionality in an inactive playlist.
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/116874/#review60669 --- Ship it! Ship It! - Shantanu Tushar On June 21, 2014, 3:55 p.m., R.Harish Navnit wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/116874/ --- (Updated June 21, 2014, 3:55 p.m.) Review request for Plasma, Shantanu Tushar and Sinny Kumari. Bugs: 330990 and Bug http://bugs.kde.org/show_bug.cgi?id=330990 http://bugs.kde.org/show_bug.cgi?id=Bug Repository: plasma-mediacenter Description --- Now the next and previous buttons do not work unless a media is playing. Diffs - mediaelements/mediacontroller/MediaController.qml 2fce0a0 Diff: https://git.reviewboard.kde.org/r/116874/diff/ Testing --- I've done the testing for this patch. These are the steps that I followed. 1. Added a new playlist and added a few songs to the playlist. 2. Re-opened Plasma Media Center 3. Clicked the Next and Previous buttons. 4. Nothing was played. 5. Selected a song from the list and then tested the next and previous buttons. 6. The buttons worked seamlessly. Thanks, R.Harish Navnit ___ Plasma-devel mailing list Plasma-devel@kde.org https://mail.kde.org/mailman/listinfo/plasma-devel
Review Request 118866: Start playing after Play All is used
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/118866/ --- Review request for Plasma. Repository: plasma-mediacenter Description --- We should ask the model to start playing once we add all the songs to the playlist as a result of the user using Play All. Diffs - browsingbackends/metadatabackends/metadatamusicbackend/metadatamusicbackend.cpp 68eecf3 Diff: https://git.reviewboard.kde.org/r/118866/diff/ Testing --- Thanks, Shantanu Tushar ___ Plasma-devel mailing list Plasma-devel@kde.org https://mail.kde.org/mailman/listinfo/plasma-devel
Re: Review Request 118866: Start playing after Play All is used
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/118866/ --- (Updated June 21, 2014, 5:43 p.m.) Review request for Plasma. Changes --- switch to and clear the default playlist only Repository: plasma-mediacenter Description --- We should ask the model to start playing once we add all the songs to the playlist as a result of the user using Play All. Diffs (updated) - browsingbackends/metadatabackends/metadatamusicbackend/metadatamusicbackend.cpp 68eecf3 libs/mediacenter/multipleplaylistmodel.h 1c7731b libs/mediacenter/multipleplaylistmodel.cpp 4b475c7 libs/mediacenter/playlistmodel.h 50157ff libs/mediacenter/playlistmodel.cpp 44abbc1 Diff: https://git.reviewboard.kde.org/r/118866/diff/ Testing --- Thanks, Shantanu Tushar ___ Plasma-devel mailing list Plasma-devel@kde.org https://mail.kde.org/mailman/listinfo/plasma-devel
Re: Review Request 118824: Fix for Bug 336414 - Playlist switching doesnt move highlight
On June 19, 2014, 8:16 a.m., Shantanu Tushar wrote: Good analysis and fix. It is still broken if you do the following- 1. Have multiple playlists 2. Remove a playlist 3. Add a playlist After this the highlight again stops to work, can you check? Hmm I can no longer reproduce this, seems to work :/ Maybe I wasn't running the correct patched binary in a hurry at office. - Shantanu --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/118824/#review60467 --- On June 19, 2014, 6:51 a.m., Ashish Madeti wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/118824/ --- (Updated June 19, 2014, 6:51 a.m.) Review request for Plasma, Shantanu Tushar and Sinny Kumari. Bugs: 336414 http://bugs.kde.org/show_bug.cgi?id=336414 Repository: plasma-mediacenter Description --- Whenever we added / removed any playlist, the highlight was stuck at one point i.e changing the playlist after that was not changing the highlight. This was happening because we were directly assigning value to multiplePlaylistList.currentIndex in qml code, which removed its previous binding. Instead we should emit the currentIndexChanged() signal and let the ListView do the rest. Diffs - libs/mediacenter/multipleplaylistmodel.cpp 87601f5 mediaelements/playlist/MultiplePlaylists.qml 4b085fe Diff: https://git.reviewboard.kde.org/r/118824/diff/ Testing --- Changed playlist after adding / removing a playlist. Now highlight is working fine Thanks, Ashish Madeti ___ Plasma-devel mailing list Plasma-devel@kde.org https://mail.kde.org/mailman/listinfo/plasma-devel
Re: Review Request 118824: Fix for Bug 336414 - Playlist switching doesnt move highlight
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/118824/#review60517 --- Ship it! Ship It! - Shantanu Tushar On June 19, 2014, 6:51 a.m., Ashish Madeti wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/118824/ --- (Updated June 19, 2014, 6:51 a.m.) Review request for Plasma, Shantanu Tushar and Sinny Kumari. Bugs: 336414 http://bugs.kde.org/show_bug.cgi?id=336414 Repository: plasma-mediacenter Description --- Whenever we added / removed any playlist, the highlight was stuck at one point i.e changing the playlist after that was not changing the highlight. This was happening because we were directly assigning value to multiplePlaylistList.currentIndex in qml code, which removed its previous binding. Instead we should emit the currentIndexChanged() signal and let the ListView do the rest. Diffs - libs/mediacenter/multipleplaylistmodel.cpp 87601f5 mediaelements/playlist/MultiplePlaylists.qml 4b085fe Diff: https://git.reviewboard.kde.org/r/118824/diff/ Testing --- Changed playlist after adding / removing a playlist. Now highlight is working fine Thanks, Ashish Madeti ___ Plasma-devel mailing list Plasma-devel@kde.org https://mail.kde.org/mailman/listinfo/plasma-devel
Re: Review Request 118767: Fix for Bug 328532 - Remember the playlist I was using when I closed PMC last time
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/118767/#review60541 --- Ship it! Seems to work correctly, just a minor nitpick and this can go in. libs/mediacenter/playlistmodel.cpp https://git.reviewboard.kde.org/r/118767/#comment42270 The explicit call to QVariant should not be needed - Shantanu Tushar On June 19, 2014, 4:30 p.m., Ashish Madeti wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/118767/ --- (Updated June 19, 2014, 4:30 p.m.) Review request for Plasma, Bhushan Shah, Nikolaos Chatzidakis, Shantanu Tushar, Sinny Kumari, and Sujith Haridasan. Bugs: 328532 http://bugs.kde.org/show_bug.cgi?id=328532 Repository: plasma-mediacenter Description --- PMC didn't used to remember the playlist I was on the last time, when restarting it. Now it saves the lastPlaylist while closing in the settings, and when starting it loads the value of lastPlaylist in settings Diffs - libs/mediacenter/playlistmodel.cpp dccaf52 Diff: https://git.reviewboard.kde.org/r/118767/diff/ Testing --- Thanks, Ashish Madeti ___ Plasma-devel mailing list Plasma-devel@kde.org https://mail.kde.org/mailman/listinfo/plasma-devel
Re: Review Request 118767: Fix for Bug 328532 - Remember the playlist I was using when I closed PMC last time
Hi Harish, This is a known issue and is due to the fact that we don't save state while switching the playlists. Maybe someone should report a bug if not already. On Thu, Jun 19, 2014 at 10:44 PM, R.Harish Navnit harishnav...@gmail.com wrote: This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/118767/ On June 19th, 2014, 4:48 p.m. UTC, *Shantanu Tushar* wrote: libs/mediacenter/playlistmodel.cpp https://git.reviewboard.kde.org/r/118767/diff/2/?file=282626#file282626line56 (Diff revision 2) public: 56 d-playlistName = s.value(lastPlaylist, QVariant(DEFAULT_PLAYLIST_NAME)).toString(); The explicit call to QVariant should not be needed I have just one issue in this. This is how I tested it. 1. Play a media from a playlist 2. Switch to another playlist 3. Return back to the playlist playing the current song. 4. Next and prev don't work. 5. Once the track finshes playing, the next track is not played. The mediacenter stops playing any media and I have to mannually choose a new media again. Imagine doing this over and over again once playlists are switched. Does anyone else face the same issue ? - R.Harish On June 19th, 2014, 4:30 p.m. UTC, Ashish Madeti wrote: Review request for Plasma, Bhushan Shah, Nikolaos Chatzidakis, Shantanu Tushar, Sinny Kumari, and Sujith Haridasan. By Ashish Madeti. *Updated June 19, 2014, 4:30 p.m.* *Bugs: * 328532 http://bugs.kde.org/show_bug.cgi?id=328532 *Repository: * plasma-mediacenter Description PMC didn't used to remember the playlist I was on the last time, when restarting it. Now it saves the lastPlaylist while closing in the settings, and when starting it loads the value of lastPlaylist in settings Diffs - libs/mediacenter/playlistmodel.cpp (dccaf52) View Diff https://git.reviewboard.kde.org/r/118767/diff/ -- Shantanu Tushar(UTC +0530) http://www.shantanutushar.com ___ Plasma-devel mailing list Plasma-devel@kde.org https://mail.kde.org/mailman/listinfo/plasma-devel
Ideas around Highlight
Hey folks, I recently added a visual highlight in the All Music mode to help the user follow the current focus[1]. I am wondering if the simple highlight looks better than PlasmaComponents.Highlight, and I need your opinion. Please try the patch at http://paste.ubuntu.com/7647243 and see which one you prefer. For folks who don't have the code running, here's a before[2] and after[3]. Cheers! [1] commits.kde.org/plasma-mediacenter/1e77bc1e19fb9f59ebb82aad9e95ff7ad966f130 [2] http://i.imgur.com/DO5cCA3.jpg [3] http://i.imgur.com/CNxPWiz.jpg -- Shantanu Tushar(UTC +0530) http://www.shantanutushar.com ___ Plasma-devel mailing list Plasma-devel@kde.org https://mail.kde.org/mailman/listinfo/plasma-devel
Re: Review Request 118759: Add a separator to the path where PMC stores playlist file
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/118759/#review60125 --- libs/mediacenter/playlistmodel.cpp https://git.reviewboard.kde.org/r/118759/#comment41879 I'd prefer QDir(getPlaylistPath()).absoluteFilePath(d-playlistName) - Shantanu Tushar On June 15, 2014, 10:14 a.m., Ashish Madeti wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/118759/ --- (Updated June 15, 2014, 10:14 a.m.) Review request for Plasma, Shantanu Tushar and Sinny Kumari. Repository: plasma-mediacenter Description --- Due to a lack of separator, PMC was storing the file in a different directory and reading playlists from a different directory. Diffs - libs/mediacenter/playlistmodel.cpp 4c52f00 Diff: https://git.reviewboard.kde.org/r/118759/diff/ Testing --- Thanks, Ashish Madeti ___ Plasma-devel mailing list Plasma-devel@kde.org https://mail.kde.org/mailman/listinfo/plasma-devel
Re: Review Request 118759: Add a separator to the path where PMC stores playlist file
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/118759/#review60126 --- Ship it! Ship It! - Shantanu Tushar On June 15, 2014, 12:30 p.m., Ashish Madeti wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/118759/ --- (Updated June 15, 2014, 12:30 p.m.) Review request for Plasma, Shantanu Tushar and Sinny Kumari. Repository: plasma-mediacenter Description --- Due to a lack of separator, PMC was storing the file in a different directory and reading playlists from a different directory. Diffs - libs/mediacenter/playlistmodel.cpp 4c52f00 Diff: https://git.reviewboard.kde.org/r/118759/diff/ Testing --- Thanks, Ashish Madeti ___ Plasma-devel mailing list Plasma-devel@kde.org https://mail.kde.org/mailman/listinfo/plasma-devel
Re: Review Request 118734: Added MPRIS specifications Tracklist Interface
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/118734/#review60061 --- libs/mediacenter/mpris2/mediaplayer2player.cpp https://git.reviewboard.kde.org/r/118734/#comment41814 { } please libs/mediacenter/mpris2/mediaplayer2tracklist.h https://git.reviewboard.kde.org/r/118734/#comment41815 C++11 magic: you can just say- int tidCounter = 0; libs/mediacenter/mpris2/mediaplayer2tracklist.cpp https://git.reviewboard.kde.org/r/118734/#comment41816 not needed (see above) - Shantanu Tushar On June 13, 2014, 5:13 p.m., Ashish Madeti wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/118734/ --- (Updated June 13, 2014, 5:13 p.m.) Review request for Plasma, Emmanuel Pescosta, Shantanu Tushar, and Sinny Kumari. Repository: plasma-mediacenter Description --- Added a MPRIS specifications Tracklist Interface, which will expose and give control over the current playlist on dbus. http://specifications.freedesktop.org/mpris-spec/latest/Track_List_Interface.html Diffs - libs/mediacenter/CMakeLists.txt 8d95cfc libs/mediacenter/mpris2/mediaplayer2player.h 7a2f583 libs/mediacenter/mpris2/mediaplayer2player.cpp 2de56c2 libs/mediacenter/mpris2/mediaplayer2tracklist.h PRE-CREATION libs/mediacenter/mpris2/mediaplayer2tracklist.cpp PRE-CREATION libs/mediacenter/mpris2/mpris2.h 1dd78ca libs/mediacenter/mpris2/mpris2.cpp b64ba04 shells/newshell/mainwindow.cpp 3c888f8 Diff: https://git.reviewboard.kde.org/r/118734/diff/ Testing --- Tested the properties using qdbusviewer. I created a qt console application for testing whether the signals are being emitted correctly and the methods are working. Thanks, Ashish Madeti ___ Plasma-devel mailing list Plasma-devel@kde.org https://mail.kde.org/mailman/listinfo/plasma-devel
Re: Review Request 118734: Added MPRIS specifications Tracklist Interface
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/118734/#review60073 --- Ship it! Ship It! - Shantanu Tushar On June 14, 2014, 8:15 a.m., Ashish Madeti wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/118734/ --- (Updated June 14, 2014, 8:15 a.m.) Review request for Plasma, Emmanuel Pescosta, Shantanu Tushar, and Sinny Kumari. Repository: plasma-mediacenter Description --- Added a MPRIS specifications Tracklist Interface, which will expose and give control over the current playlist on dbus. http://specifications.freedesktop.org/mpris-spec/latest/Track_List_Interface.html Diffs - libs/mediacenter/CMakeLists.txt 8d95cfc libs/mediacenter/mpris2/mediaplayer2player.h 7a2f583 libs/mediacenter/mpris2/mediaplayer2player.cpp 2de56c2 libs/mediacenter/mpris2/mediaplayer2tracklist.h PRE-CREATION libs/mediacenter/mpris2/mediaplayer2tracklist.cpp PRE-CREATION libs/mediacenter/mpris2/mpris2.h 1dd78ca libs/mediacenter/mpris2/mpris2.cpp b64ba04 shells/newshell/mainwindow.cpp 3c888f8 Diff: https://git.reviewboard.kde.org/r/118734/diff/ Testing --- Tested the properties using qdbusviewer. I created a qt console application for testing whether the signals are being emitted correctly and the methods are working. Thanks, Ashish Madeti ___ Plasma-devel mailing list Plasma-devel@kde.org https://mail.kde.org/mailman/listinfo/plasma-devel
Re: Review Request 118691: Change currentIndex of playlistModel to -1 when a media is not from playlist
On June 12, 2014, 5:54 p.m., Mark Gaiser wrote: Why would you do it this way? I mean, you should have a model in the C++ side and you should have a view (probably ListView). Then you can just ask the view the count: http://qt-project.org/doc/qt-5/qml-qtquick-listview.html#count-prop i kinda miss the point of doing this via C++. But then again, i don't know the code in this project. Ashish Madeti wrote: I don't understand how 'count' is relevant here (I didn't get your point). We want to set the currentIndex (which points to the item currently playing in playlist) to -1 whenever a user plays something from other than playlist. Mark Gaiser wrote: Ahh, sorry. I was wrong about the count property. It has nothing to do with this. But you could have used the currentIndex property: http://qt-project.org/doc/qt-5/qml-qtquick-listview.html#currentIndex-prop But that would only work if you also use the highlight functionality in the views. In this case there should be no item highlighted which means the ListView currentIndex property is -1. Does it make a bit more sense now? :) We do use the currentIndex property of the view, we have two things- 1. Currently selected item in the View (the one with highlight), the user can move this using arrow keys/mouse. 2. Currently *playing* item in the playlist. For 1 we use currentIndex of the View and 2 is implemented using a currentIndex property in the model (could use a better name though) - Shantanu --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/118691/#review59895 --- On June 12, 2014, 5:51 p.m., Ashish Madeti wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/118691/ --- (Updated June 12, 2014, 5:51 p.m.) Review request for Plasma, Shantanu Tushar and Sinny Kumari. Repository: plasma-mediacenter Description --- Change the currentIndex of playlistModel to -1 when a media is played from somewhere other than playlist. Diffs - libs/mediacenter/playlistmodel.h 0de7c56 libs/mediacenter/playlistmodel.cpp c5ab1ab shells/newshell/package/contents/ui/mediacenter.qml a859225 Diff: https://git.reviewboard.kde.org/r/118691/diff/ Testing --- Thanks, Ashish Madeti ___ Plasma-devel mailing list Plasma-devel@kde.org https://mail.kde.org/mailman/listinfo/plasma-devel
Review Request 118697: Treat albums with different artists as different albums
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/118697/ --- Review request for Plasma. Repository: plasma-mediacenter Description --- This one has been pending for some time now. Our Album data model assumed that a album name is enough to uniquely identify an album. This creates problems because multiple artists may have albums with the same name. This patch makes Media Library and ItemCache aware of this fact. Diffs - libs/mediacenter/album.h 4926e48 libs/mediacenter/album.cpp 881f65a libs/mediacenter/itemcache.h ffee05f libs/mediacenter/medialibrary.cpp 9c58b44 libs/test/itemcachetest.h 4fc6807 libs/test/itemcachetest.cpp e6dee33 libs/test/medialibrarytest.h 718457d libs/test/medialibrarytest.cpp e1a8c9b libs/test/mediatest.cpp 8fde60b libs/test/pmcalbumtest.cpp 9f2b4c8 Diff: https://git.reviewboard.kde.org/r/118697/diff/ Testing --- Unit tests pass. Added test to take care of the new scenario. Thanks, Shantanu Tushar ___ Plasma-devel mailing list Plasma-devel@kde.org https://mail.kde.org/mailman/listinfo/plasma-devel
Re: Review Request 118691: Change currentIndex of playlistModel to -1 when a media is not from playlist
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/118691/#review59887 --- libs/mediacenter/playlistmodel.h https://git.reviewboard.kde.org/r/118691/#comment41712 currentIndex is not writable from QML so that PlaylistModel can have complete control over it. A better way is to create a slot called resetCurrentIndex in PlaylistModel which will set it to -1. - Shantanu Tushar On June 12, 2014, 8:58 a.m., Ashish Madeti wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/118691/ --- (Updated June 12, 2014, 8:58 a.m.) Review request for Plasma, Shantanu Tushar and Sinny Kumari. Repository: plasma-mediacenter Description --- Change the currentIndex of playlistModel to -1 when a media is played from somewhere other than playlist. Diffs - libs/mediacenter/playlistmodel.h 0de7c56 shells/newshell/package/contents/ui/mediacenter.qml a859225 Diff: https://git.reviewboard.kde.org/r/118691/diff/ Testing --- Thanks, Ashish Madeti ___ Plasma-devel mailing list Plasma-devel@kde.org https://mail.kde.org/mailman/listinfo/plasma-devel
Re: Review Request 118691: Change currentIndex of playlistModel to -1 when a media is not from playlist
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/118691/#review59891 --- Ship it! Ship It! - Shantanu Tushar On June 12, 2014, 5:22 p.m., Ashish Madeti wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/118691/ --- (Updated June 12, 2014, 5:22 p.m.) Review request for Plasma, Shantanu Tushar and Sinny Kumari. Repository: plasma-mediacenter Description --- Change the currentIndex of playlistModel to -1 when a media is played from somewhere other than playlist. Diffs - libs/mediacenter/playlistmodel.h 0de7c56 libs/mediacenter/playlistmodel.cpp c5ab1ab shells/newshell/package/contents/ui/mediacenter.qml a859225 Diff: https://git.reviewboard.kde.org/r/118691/diff/ Testing --- Thanks, Ashish Madeti ___ Plasma-devel mailing list Plasma-devel@kde.org https://mail.kde.org/mailman/listinfo/plasma-devel
Re: Review Request 118657: Use move() while moving items in playlist, instead of dataChanged() signals
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/118657/#review59816 --- Ship it! Ship It! - Shantanu Tushar On June 11, 2014, 5:51 a.m., Ashish Madeti wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/118657/ --- (Updated June 11, 2014, 5:51 a.m.) Review request for Plasma, Shantanu Tushar and Sinny Kumari. Repository: plasma-mediacenter Description --- Use beginMoveRows() and endMoveRows() instead of dataChanged() while moving items in playlistModel Diffs - libs/mediacenter/playlistmodel.cpp 158ca65 Diff: https://git.reviewboard.kde.org/r/118657/diff/ Testing --- Moved media around in playlist, its working fine. Thanks, Ashish Madeti ___ Plasma-devel mailing list Plasma-devel@kde.org https://mail.kde.org/mailman/listinfo/plasma-devel
Re: Review Request 118406: Notify the user if the location containing the media is inaccessible.
On June 5, 2014, 8:36 a.m., Thomas Pfeiffer wrote: Usability review: Since I lack the skills to picture it from the diff: When exactly is the notification shown? Is it shown as soon as the media is supposed to be played? If so, I think it could be done in a more subtle way: Grey out the entry in the playlist and just skip over it to the next entry int he playlist (like Amarok does) and show the message in a tooltip on mouseover on a pointer device or on tap on a touch device. If it's already done in a similar way, then it's fine for me ;) R.Harish Navnit wrote: I've added a screenshot which should help. This is a little different from the way amarok deals with the same error. If the media is inaccessible, the message is shown in the player screen. Is it okay, If I go ahead in the way I am or should look to implement something similar to what amarok does ?? Thomas Pfeiffer wrote: Does PMC really have to be restarted to detect that the location is now accessible? That's not really a nice situation. The whole message seems overly dramatic to me, especially in a playlist. Imagine you have a playlist with 50 songs in it, and three of them are on a location which isn't accessible. What would you prefer? a) Having to close PMC, make the location accessible and then restart it b) Just skip over those three songs and happily listen to the other 47 If we're not in a playlist, the problem should rarely occur because the browser only shows currently accessible media anyway, right? R.Harish Navnit wrote: So, like I'd mentioned in the mail(sorry about that). 1.PMC need not be restarted to get to detect that the location is accessible. I think the message might have been ambiguous. The user is expected to mount all drives containing media and then run the application again. 2. You can skip over to the next media in a playlist. That's how I envisage the fix. Please let me know things have to be done differently. Thomas Pfeiffer wrote: The problem is that currently, the message is very scary. The skip controls are still active, but the user is presented with that very prominent error message telling her that she has to make sure that the file becomes accessible and restart the application. Plus, the playlist won't continue unless the user actively skips to the next entry, right? The way Amarok does it is way less scary and disruptive since it silently skips. The only problem with Amarok's solution is that there is no way to find out why it skipped. That's why I suggested to offer a tooltip so users who wonder why a track was skipped can find out what the problem is. The message showing up in the tooltip could be like the one you've proposed, though I would change it to Could not open the file [path]. To play this file, make sure it is accessible and restart Plasma Mediacenter Shantanu Tushar wrote: Bah, this skipped me as well. There is absolutely no need to restart the app here, if the file is available, simply clicking play again should work. Also, after Thomas pointed it out, in a playlist it actually makes sense to simply skip the missing media. However, if you play a single media (without the playlist) then this message seems appropriate. @Thomas, does that sound right? Finally, I'd suggest and try again, insteasd of and restart the application. R.Harish Navnit wrote: I find another discrepancy here, simply play again doesn't work even if the file is available(after initially being inaccessible). There is no need to restart the app here, but the only way that I'm able to play the media is by playing either the next or previous media and then returning to the current media. Clicking play again doesn't seem to work for me. I don't find this too much of an issue btw, just reported though. From the discussions that we've had ^^, I'm inferring that it is expected that this fix works similar to the way in which amarok handles the same situation, and I shall proceed along the same lines. Well then its a bug, ideally clicking play again should work. This review should only go in once that bug is fixed imo (because it should change please restart to please try again) About the behavior, in my previous comment I have suggested that this error should only come when the played media is *not* in the current playlist. If it is in playlist, we should silently skip it. - Shantanu --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/118406/#review59281 --- On June 5, 2014, 8:47 a.m., R.Harish Navnit wrote: --- This is an automatically generated e-mail
Re: Review Request 118406: Notify the user if the location containing the media is inaccessible.
On June 5, 2014, 8:36 a.m., Thomas Pfeiffer wrote: Usability review: Since I lack the skills to picture it from the diff: When exactly is the notification shown? Is it shown as soon as the media is supposed to be played? If so, I think it could be done in a more subtle way: Grey out the entry in the playlist and just skip over it to the next entry int he playlist (like Amarok does) and show the message in a tooltip on mouseover on a pointer device or on tap on a touch device. If it's already done in a similar way, then it's fine for me ;) R.Harish Navnit wrote: I've added a screenshot which should help. This is a little different from the way amarok deals with the same error. If the media is inaccessible, the message is shown in the player screen. Is it okay, If I go ahead in the way I am or should look to implement something similar to what amarok does ?? Thomas Pfeiffer wrote: Does PMC really have to be restarted to detect that the location is now accessible? That's not really a nice situation. The whole message seems overly dramatic to me, especially in a playlist. Imagine you have a playlist with 50 songs in it, and three of them are on a location which isn't accessible. What would you prefer? a) Having to close PMC, make the location accessible and then restart it b) Just skip over those three songs and happily listen to the other 47 If we're not in a playlist, the problem should rarely occur because the browser only shows currently accessible media anyway, right? R.Harish Navnit wrote: So, like I'd mentioned in the mail(sorry about that). 1.PMC need not be restarted to get to detect that the location is accessible. I think the message might have been ambiguous. The user is expected to mount all drives containing media and then run the application again. 2. You can skip over to the next media in a playlist. That's how I envisage the fix. Please let me know things have to be done differently. Thomas Pfeiffer wrote: The problem is that currently, the message is very scary. The skip controls are still active, but the user is presented with that very prominent error message telling her that she has to make sure that the file becomes accessible and restart the application. Plus, the playlist won't continue unless the user actively skips to the next entry, right? The way Amarok does it is way less scary and disruptive since it silently skips. The only problem with Amarok's solution is that there is no way to find out why it skipped. That's why I suggested to offer a tooltip so users who wonder why a track was skipped can find out what the problem is. The message showing up in the tooltip could be like the one you've proposed, though I would change it to Could not open the file [path]. To play this file, make sure it is accessible and restart Plasma Mediacenter Bah, this skipped me as well. There is absolutely no need to restart the app here, if the file is available, simply clicking play again should work. Also, after Thomas pointed it out, in a playlist it actually makes sense to simply skip the missing media. However, if you play a single media (without the playlist) then this message seems appropriate. @Thomas, does that sound right? Finally, I'd suggest and try again, insteasd of and restart the application. - Shantanu --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/118406/#review59281 --- On June 5, 2014, 8:47 a.m., R.Harish Navnit wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/118406/ --- (Updated June 5, 2014, 8:47 a.m.) Review request for Plasma, Shantanu Tushar and Sinny Kumari. Bugs: 333764 http://bugs.kde.org/show_bug.cgi?id=333764 Repository: plasma-mediacenter Description --- If a media(in a playlist) is located in an inaccessible location, then the user is notified about the same. Diffs - mediaelements/mediaplayer/MediaPlayer.qml 98f1d2c Diff: https://git.reviewboard.kde.org/r/118406/diff/ Testing --- 1. Load media to a playlist. 2. Unmount the device containing media. 3. Check if the user is notified of the location being inaccessible --yes, the user is notified 4. Mount the device containing media and play a media from playlist. -- The media plays properly. File Attachments wihtout_i18n.png https://git.reviewboard.kde.org/media/uploaded/files/2014/06/05/3dc148a5-c4da-4d27-a713-e63922cbcef8__wihtout_i18n.png Thanks, R.Harish Navnit
Re: Review Request 118406: Notify the user if the location containing the media is inaccessible.
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/118406/#review59209 --- mediaelements/mediaplayer/MediaPlayer.qml https://git.reviewboard.kde.org/r/118406/#comment41223 Remove the trailing space in the i18n call (after location). Also, the i18n on video.source is of no use, people cant translate filenames, can they? :P mediaelements/mediaplayer/MediaPlayer.qml https://git.reviewboard.kde.org/r/118406/#comment41222 instead of guessing, why not use the error property of QML Video? http://doc.qt.digia.com/qtmobility-1.2.0/qml-video.html#error-prop - Shantanu Tushar On June 3, 2014, 5:05 p.m., R.Harish Navnit wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/118406/ --- (Updated June 3, 2014, 5:05 p.m.) Review request for Plasma, Shantanu Tushar and Sinny Kumari. Bugs: 333764 http://bugs.kde.org/show_bug.cgi?id=333764 Repository: plasma-mediacenter Description --- If a media(in a playlist) is located in an inaccessible location, then the user is notified about the same. Diffs - mediaelements/mediaplayer/MediaPlayer.qml 98f1d2c Diff: https://git.reviewboard.kde.org/r/118406/diff/ Testing --- 1. Load media to a playlist. 2. Unmount the device containing media. 3. Check if the user is notified of the location being inaccessible --yes, the user is notified 4. Mount the device containing media and play a media from playlist. -- The media plays properly. Thanks, R.Harish Navnit ___ Plasma-devel mailing list Plasma-devel@kde.org https://mail.kde.org/mailman/listinfo/plasma-devel
Re: Review Request 118468: Use the spacebar key to play/pause a media from the homescreen.
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/118468/#review59005 --- Ship it! Hmm, looks good to me. - Shantanu Tushar On June 2, 2014, 12:24 p.m., R.Harish Navnit wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/118468/ --- (Updated June 2, 2014, 12:24 p.m.) Review request for Plasma, Shantanu Tushar and Sinny Kumari. Repository: plasma-mediacenter Description --- As of now, PMC only uses the MediaPlay button to give the user an option to play/pause media from the home screen. However, not all keyboards have the MediaPlay button AFAIK. This patch gives the user the option to play/pause the media from the home screen using the spacebar key as another alternative. Diffs - shells/newshell/package/contents/ui/mediacenter.qml e1bc0e6 Diff: https://git.reviewboard.kde.org/r/118468/diff/ Testing --- Play any media, navigate to homescreen. Press the spacebar to check if the media plays/pauses. Seems to work fine to me. Thanks, R.Harish Navnit ___ Plasma-devel mailing list Plasma-devel@kde.org https://mail.kde.org/mailman/listinfo/plasma-devel
Re: Review Request 118092: Left and right movement in All music
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/118092/#review58089 --- Ship it! Ship It! - Shantanu Tushar On May 16, 2014, 6:36 p.m., Sujith Haridasan wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/118092/ --- (Updated May 16, 2014, 6:36 p.m.) Review request for Plasma, Shantanu Tushar and Sinny Kumari. Bugs: 334148 http://bugs.kde.org/show_bug.cgi?id=334148 Repository: plasma-mediacenter Description --- Left and right movement in All Music - Songs section. Now user can navigate left and right when in the songs section. And this is applicable only to the songs section. Diffs - components/listbrowser/ListBrowser.qml 202d406 Diff: https://git.reviewboard.kde.org/r/118092/diff/ Testing --- 1) Use down arrow key to reach songs. 2) Press left/right arrow key. 3) Press left/right arrow key(for some reason we have to press the same key twice). 4) User reaches Artists if pressed left or Albums if pressed right. Thanks, Sujith Haridasan ___ Plasma-devel mailing list Plasma-devel@kde.org https://mail.kde.org/mailman/listinfo/plasma-devel
Re: Review Request 118092: Left and right movement in All music
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/118092/#review58065 --- components/listbrowser/ListBrowser.qml https://git.reviewboard.kde.org/r/118092/#comment40377 A ListBrowser does not need to be in a TabBar, (even though in All Music it is). So as a safeguard, do a check that listBrowserRoot.topSibling.buttons should be defined. Something like- var buttons = listBrowserRoot.topSibling.buttons; if (buttons) { buttons.get(1).button.clicked(); } - Shantanu Tushar On May 16, 2014, 3:18 a.m., Sujith Haridasan wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/118092/ --- (Updated May 16, 2014, 3:18 a.m.) Review request for Plasma, Shantanu Tushar and Sinny Kumari. Bugs: 334148 http://bugs.kde.org/show_bug.cgi?id=334148 Repository: plasma-mediacenter Description --- Left and right movement in All Music - Songs section. Now user can navigate left and right when in the songs section. And this is applicable only to the songs section. Diffs - components/listbrowser/ListBrowser.qml 202d406 Diff: https://git.reviewboard.kde.org/r/118092/diff/ Testing --- 1) Use down arrow key to reach songs. 2) Press left/right arrow key. 3) Press left/right arrow key(for some reason we have to press the same key twice). 4) User reaches Artists if pressed left or Albums if pressed right. Thanks, Sujith Haridasan ___ Plasma-devel mailing list Plasma-devel@kde.org https://mail.kde.org/mailman/listinfo/plasma-devel
Re: Review Request 118092: Left and right movement in All music
On May 16, 2014, 4:20 a.m., Ashish Madeti wrote: Now if you are on Songs tab, a single left/right key press will take you to Artists/Albums section but when you return to Songs section, the focus is still on tab bar. Due to this user has to first go to the list by pressing down key once (not intuitive) whereas imho user should be able to continue from where he left. The bigger problem is that our up/down navigation is already not so clear in All Music. Fixing that should be better, i.e. to properly show that the focus is still on the TabBar. However, this is not in scope of this patch. - Shantanu --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/118092/#review58040 --- On May 16, 2014, 3:18 a.m., Sujith Haridasan wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/118092/ --- (Updated May 16, 2014, 3:18 a.m.) Review request for Plasma, Shantanu Tushar and Sinny Kumari. Bugs: 334148 http://bugs.kde.org/show_bug.cgi?id=334148 Repository: plasma-mediacenter Description --- Left and right movement in All Music - Songs section. Now user can navigate left and right when in the songs section. And this is applicable only to the songs section. Diffs - components/listbrowser/ListBrowser.qml 202d406 Diff: https://git.reviewboard.kde.org/r/118092/diff/ Testing --- 1) Use down arrow key to reach songs. 2) Press left/right arrow key. 3) Press left/right arrow key(for some reason we have to press the same key twice). 4) User reaches Artists if pressed left or Albums if pressed right. Thanks, Sujith Haridasan ___ Plasma-devel mailing list Plasma-devel@kde.org https://mail.kde.org/mailman/listinfo/plasma-devel
Re: Review Request 117984: Try to prioritize photos taken by a camera-like device
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/117984/ --- (Updated May 12, 2014, 2:14 p.m.) Status -- This change has been marked as submitted. Review request for Plasma. Repository: plasma-mediacenter Description --- The list of images that we get from Baloo has lot of noise because it usually contains small images from data files of software, games and so on. We do two things- * Ignore images unless they are wider than 500 pixels * Make images with EXIF data appear first by giving them preference in sorting by date/time Diffs - plugins/baloosearch/audiosearchresulthandler.h 11b7219 plugins/baloosearch/audiosearchresulthandler.cpp de98bb7 plugins/baloosearch/baloosearchmediasource.h 93e3d80 plugins/baloosearch/baloosearchmediasource.cpp 4180b1e plugins/baloosearch/imagesearchresulthandler.h 200451e plugins/baloosearch/imagesearchresulthandler.cpp 272b476 plugins/baloosearch/searchresulthandler.h 0df0ba2 plugins/baloosearch/searchresulthandler.cpp 78e41ea plugins/baloosearch/videosearchresulthandler.h 146efca plugins/baloosearch/videosearchresulthandler.cpp 1295b30 Diff: https://git.reviewboard.kde.org/r/117984/diff/ Testing --- Photos are fetched, the ones from my recent trip to Kashmir show up first ;) Thanks, Shantanu Tushar ___ Plasma-devel mailing list Plasma-devel@kde.org https://mail.kde.org/mailman/listinfo/plasma-devel
Re: Review Request 118092: Left and right movement in All music
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/118092/#review57797 --- components/listbrowser/ListBrowser.qml https://git.reviewboard.kde.org/r/118092/#comment40236 As this only puts focus on the tab bar, you have to press left/right twice to make it work. Instead one press of the key should work. - Shantanu Tushar On May 12, 2014, 4:29 a.m., Sujith Haridasan wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/118092/ --- (Updated May 12, 2014, 4:29 a.m.) Review request for Plasma, Shantanu Tushar and Sinny Kumari. Bugs: 334148 http://bugs.kde.org/show_bug.cgi?id=334148 Repository: plasma-mediacenter Description --- Left and right movement in All Music - Songs section. Now user can navigate left and right when in the songs section. And this is applicable only to the songs section. Diffs - components/listbrowser/ListBrowser.qml 202d406 Diff: https://git.reviewboard.kde.org/r/118092/diff/ Testing --- 1) Use down arrow key to reach songs. 2) Press left/right arrow key. 3) Press left/right arrow key(for some reason we have to press the same key twice). 4) User reaches Artists if pressed left or Albums if pressed right. Thanks, Sujith Haridasan ___ Plasma-devel mailing list Plasma-devel@kde.org https://mail.kde.org/mailman/listinfo/plasma-devel
Re: Review Request 118022: Replace hacky access to the declarative engine by a PmcRuntime class
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/118022/ --- (Updated May 7, 2014, 4:05 p.m.) Status -- This change has been marked as submitted. Review request for Plasma. Repository: plasma-mediacenter Description --- When backends needed to access some PMC objects like the playlist model or register image providers, they used to fetch a reference to the view's declarative engine. This is risky because it exposes the view to the backends which can then do anything with it. This patch adds a PmcRuntime class which serves as an official interface to these accesses cleanly. Diffs - browsingbackends/localfiles/localfilesabstractbackend.h 2d47eba browsingbackends/localfiles/localfilesabstractbackend.cpp b86a28a browsingbackends/localfiles/localpictures/localpicturesbackend.cpp f025c04 browsingbackends/localfiles/localpictures/localpicturesmodel.h b1a81b4 browsingbackends/localfiles/localpictures/localpicturesmodel.cpp c6b1816 browsingbackends/localfiles/localvideos/localvideosbackend.cpp 8c20f27 browsingbackends/localfiles/localvideos/localvideosmodel.h e836c14 browsingbackends/localfiles/localvideos/localvideosmodel.cpp ab24013 browsingbackends/metadatabackends/metadatamusicbackend/metadatamusicbackend.h 50af6c2 browsingbackends/metadatabackends/metadatamusicbackend/metadatamusicbackend.cpp 6991c97 libs/mediacenter/CMakeLists.txt f07d3ec libs/mediacenter/abstractbrowsingbackend.h 537d00d libs/mediacenter/abstractbrowsingbackend.cpp eea4ee5 libs/mediacenter/backendsmodel.h f98f60a libs/mediacenter/backendsmodel.cpp 3a5fc30 libs/mediacenter/pmcruntime.h PRE-CREATION libs/mediacenter/pmcruntime.cpp PRE-CREATION shells/newshell/mainwindow.h 0aaa513 shells/newshell/mainwindow.cpp e369055 shells/newshell/package/contents/ui/mediacenter.qml a62becc Diff: https://git.reviewboard.kde.org/r/118022/diff/ Testing --- Adding songs to playlist, local pictures previews, local videos previews etc work fine. Thanks, Shantanu Tushar ___ Plasma-devel mailing list Plasma-devel@kde.org https://mail.kde.org/mailman/listinfo/plasma-devel
Re: Review Request 118022: Replace hacky access to the declarative engine by a PmcRuntime class
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/118022/ --- (Updated May 6, 2014, 3:56 p.m.) Review request for Plasma. Changes --- Removed FIXME and obsolete PmcRuntime from mediacenter.qml Repository: plasma-mediacenter Description --- When backends needed to access some PMC objects like the playlist model or register image providers, they used to fetch a reference to the view's declarative engine. This is risky because it exposes the view to the backends which can then do anything with it. This patch adds a PmcRuntime class which serves as an official interface to these accesses cleanly. Diffs (updated) - browsingbackends/localfiles/localfilesabstractbackend.h 2d47eba browsingbackends/localfiles/localfilesabstractbackend.cpp b86a28a browsingbackends/localfiles/localpictures/localpicturesbackend.cpp f025c04 browsingbackends/localfiles/localpictures/localpicturesmodel.h b1a81b4 browsingbackends/localfiles/localpictures/localpicturesmodel.cpp c6b1816 browsingbackends/localfiles/localvideos/localvideosbackend.cpp 8c20f27 browsingbackends/localfiles/localvideos/localvideosmodel.h e836c14 browsingbackends/localfiles/localvideos/localvideosmodel.cpp ab24013 browsingbackends/metadatabackends/metadatamusicbackend/metadatamusicbackend.h 50af6c2 browsingbackends/metadatabackends/metadatamusicbackend/metadatamusicbackend.cpp 6991c97 libs/mediacenter/CMakeLists.txt f07d3ec libs/mediacenter/abstractbrowsingbackend.h 537d00d libs/mediacenter/abstractbrowsingbackend.cpp eea4ee5 libs/mediacenter/backendsmodel.h f98f60a libs/mediacenter/backendsmodel.cpp 3a5fc30 libs/mediacenter/pmcruntime.h PRE-CREATION libs/mediacenter/pmcruntime.cpp PRE-CREATION shells/newshell/mainwindow.h 0aaa513 shells/newshell/mainwindow.cpp e369055 shells/newshell/package/contents/ui/mediacenter.qml a62becc Diff: https://git.reviewboard.kde.org/r/118022/diff/ Testing --- Adding songs to playlist, local pictures previews, local videos previews etc work fine. Thanks, Shantanu Tushar ___ Plasma-devel mailing list Plasma-devel@kde.org https://mail.kde.org/mailman/listinfo/plasma-devel
Re: Plasma 5?
+1 And, its easier to say as well :) On Mon, May 5, 2014 at 8:51 PM, Marco Martin notm...@gmail.com wrote: On Monday 05 May 2014, Sebastian Kügler wrote: The baseline, to use Plasma as the brand, and only refer to the version as a technicality should of course stay the same. Why do I think 5 is better than 2014.6, or year.month of release? After trying (with not much success) to use publicly the next/2014.06 scheme, I have to agree that I would feel more confortable with either Plasma 5 or Plasma 2 To me either of which wouldn't make much difference, it's true tough that looks less confusing if it has the same number as frameworks and Qt -- Marco Martin ___ Plasma-devel mailing list Plasma-devel@kde.org https://mail.kde.org/mailman/listinfo/plasma-devel -- Shantanu Tushar(UTC +0530) http://www.shantanutushar.com ___ Plasma-devel mailing list Plasma-devel@kde.org https://mail.kde.org/mailman/listinfo/plasma-devel
Re: Review Request 117981: Filter out images with image width lesser than 500 pixel in Plasma media Center
On May 4, 2014, 5:40 a.m., Bhushan Shah wrote: Same should be applied to baloo mediasource. Nepomuk mediasource is kind of deprecated. I'm working on it and will be submitting a patch soon. - Shantanu --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/117981/#review57225 --- On May 4, 2014, 5:22 a.m., Sinny Kumari wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/117981/ --- (Updated May 4, 2014, 5:22 a.m.) Review request for Plasma. Repository: plasma-mediacenter Description --- When All Pictures gets opened within PMC, it shows all images indexed by nepomuk. Sometime many images appear which are very small in size and are not useful because those images come from some applications, system icons etc.This patch will filter out images whose width is lesser than 500 pixel. Diffs - plugins/kdedesktopsearch/CMakeLists.txt ece8ef2 plugins/kdedesktopsearch/kdemetadatamediasource.h 2d8e7e2 plugins/kdedesktopsearch/kdemetadatamediasource.cpp 94c390a Diff: https://git.reviewboard.kde.org/r/117981/diff/ Testing --- Works as expected Thanks, Sinny Kumari ___ Plasma-devel mailing list Plasma-devel@kde.org https://mail.kde.org/mailman/listinfo/plasma-devel
Review Request 117983: Nuke PmcImageProvider and streamline image providers
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/117983/ --- Review request for Plasma. Repository: plasma-mediacenter Description --- The code around fetching/storing covers and other such images from remote sources and local files had become quite confusing over time. This patch removes unnecessary things and moves methods to the right places. Diffs - libs/mediacenter/CMakeLists.txt 1072799 libs/mediacenter/pmccoverartprovider.h a4aeb83 libs/mediacenter/pmccoverartprovider.cpp c68afa2 libs/mediacenter/pmcimagecache.h 631d5bd libs/mediacenter/pmcimagecache.cpp d1b57b5 libs/mediacenter/pmcimageprovider.h e584810 libs/mediacenter/pmcimageprovider.cpp 4198f56 libs/mediacenter/pmcmetadatamodel.cpp a58a57b libs/test/lastfmimagefetchertest.cpp b0fcf6f mediaelements/mediaplayer/MusicStats.qml 178a37d mediaelements/popupmenu/PopupMenu.qml 1c87926 shells/newshell/mainwindow.cpp 764b228 Diff: https://git.reviewboard.kde.org/r/117983/diff/ Testing --- Tested the following things- * Album and Artist cover art * Cover in MusicStats.qml when playing from a file * Thumbnails in All Videos Thanks, Shantanu Tushar ___ Plasma-devel mailing list Plasma-devel@kde.org https://mail.kde.org/mailman/listinfo/plasma-devel
Review Request 117984: Try to prioritize photos taken by a camera-like device
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/117984/ --- Review request for Plasma. Repository: plasma-mediacenter Description --- The list of images that we get from Baloo has lot of noise because it usually contains small images from data files of software, games and so on. We do two things- * Ignore images unless they are wider than 500 pixels * Make images with EXIF data appear first by giving them preference in sorting by date/time Diffs - plugins/baloosearch/audiosearchresulthandler.h 11b7219 plugins/baloosearch/audiosearchresulthandler.cpp de98bb7 plugins/baloosearch/baloosearchmediasource.h 93e3d80 plugins/baloosearch/baloosearchmediasource.cpp 4180b1e plugins/baloosearch/imagesearchresulthandler.h 200451e plugins/baloosearch/imagesearchresulthandler.cpp 272b476 plugins/baloosearch/searchresulthandler.h 0df0ba2 plugins/baloosearch/searchresulthandler.cpp 78e41ea plugins/baloosearch/videosearchresulthandler.h 146efca plugins/baloosearch/videosearchresulthandler.cpp 1295b30 Diff: https://git.reviewboard.kde.org/r/117984/diff/ Testing --- Photos are fetched, the ones from my recent trip to Kashmir show up first ;) Thanks, Shantanu Tushar ___ Plasma-devel mailing list Plasma-devel@kde.org https://mail.kde.org/mailman/listinfo/plasma-devel
Re: Review Request 117981: Filter out images with image width lesser than 500 pixel in Plasma media Center
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/117981/#review57227 --- Ship it! Few nitpicks, looks okay otherwise. plugins/kdedesktopsearch/kdemetadatamediasource.cpp https://git.reviewboard.kde.org/r/117981/#comment39881 const QUrl plugins/kdedesktopsearch/kdemetadatamediasource.cpp https://git.reviewboard.kde.org/r/117981/#comment39880 Leftover? - Shantanu Tushar On May 4, 2014, 5:22 a.m., Sinny Kumari wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/117981/ --- (Updated May 4, 2014, 5:22 a.m.) Review request for Plasma. Repository: plasma-mediacenter Description --- When All Pictures gets opened within PMC, it shows all images indexed by nepomuk. Sometime many images appear which are very small in size and are not useful because those images come from some applications, system icons etc.This patch will filter out images whose width is lesser than 500 pixel. Diffs - plugins/kdedesktopsearch/CMakeLists.txt ece8ef2 plugins/kdedesktopsearch/kdemetadatamediasource.h 2d8e7e2 plugins/kdedesktopsearch/kdemetadatamediasource.cpp 94c390a Diff: https://git.reviewboard.kde.org/r/117981/diff/ Testing --- Works as expected Thanks, Sinny Kumari ___ Plasma-devel mailing list Plasma-devel@kde.org https://mail.kde.org/mailman/listinfo/plasma-devel
Re: Review Request 117983: Nuke PmcImageProvider and streamline image providers
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/117983/ --- (Updated May 4, 2014, 8:19 a.m.) Status -- This change has been marked as submitted. Review request for Plasma. Repository: plasma-mediacenter Description --- The code around fetching/storing covers and other such images from remote sources and local files had become quite confusing over time. This patch removes unnecessary things and moves methods to the right places. Diffs - libs/mediacenter/CMakeLists.txt 1072799 libs/mediacenter/pmccoverartprovider.h a4aeb83 libs/mediacenter/pmccoverartprovider.cpp c68afa2 libs/mediacenter/pmcimagecache.h 631d5bd libs/mediacenter/pmcimagecache.cpp d1b57b5 libs/mediacenter/pmcimageprovider.h e584810 libs/mediacenter/pmcimageprovider.cpp 4198f56 libs/mediacenter/pmcmetadatamodel.cpp a58a57b libs/test/lastfmimagefetchertest.cpp b0fcf6f mediaelements/mediaplayer/MusicStats.qml 178a37d mediaelements/popupmenu/PopupMenu.qml 1c87926 shells/newshell/mainwindow.cpp 764b228 Diff: https://git.reviewboard.kde.org/r/117983/diff/ Testing --- Tested the following things- * Album and Artist cover art * Cover in MusicStats.qml when playing from a file * Thumbnails in All Videos Thanks, Shantanu Tushar ___ Plasma-devel mailing list Plasma-devel@kde.org https://mail.kde.org/mailman/listinfo/plasma-devel
Re: Review Request 117915: Fetch photo-taken Date/Time for Images and file created Date/Time for other files
On May 2, 2014, 9:07 a.m., Vishesh Handa wrote: Looks fine from a Baloo point of view. I cannot comment on the PMC parts, I don't know the code. That is cool :) On May 2, 2014, 9:07 a.m., Vishesh Handa wrote: plugins/baloosearch/searchresulthandler.cpp, line 36 https://git.reviewboard.kde.org/r/117915/diff/1/?file=270496#file270496line36 Please keep in mind that this is sync, and you'll be blocking. You want to put it another thread via the Runnable if you want it to be async. That is fine, the media sources are themselves running in a separate thread. On May 2, 2014, 9:07 a.m., Vishesh Handa wrote: plugins/baloosearch/audiosearchresulthandler.cpp, line 60 https://git.reviewboard.kde.org/r/117915/diff/1/?file=270490#file270490line60 Considering that you're checking if the title is empty, do you want to do the same for the Artist and Album? Usually things like empty Album and Artist are handled by the Media Library which shows then as Unknown so there is no need to check this here. For the title, when we haven't fetched the File metadata, we would've put the filename (foo.mp3) as the title. This check is to make sure that we don't end up erasing foo.mp3 unless the title The Foo Song is available in Baloo::File. - Shantanu --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/117915/#review57102 --- On May 1, 2014, 6:06 a.m., Shantanu Tushar wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/117915/ --- (Updated May 1, 2014, 6:06 a.m.) Review request for Plasma and Vishesh Handa. Repository: plasma-mediacenter Description --- Just like the Nepomuk media source, the Baloo media source needs to provide the date/time information for media. This is used to sort the media to show the more recent media first. For images, the date/time when the photo was actually taken is used, and the file creation date/time is used for other media. Diffs - plugins/baloosearch/CMakeLists.txt 1ff81fb plugins/baloosearch/audiosearchresulthandler.h PRE-CREATION plugins/baloosearch/audiosearchresulthandler.cpp PRE-CREATION plugins/baloosearch/baloosearchmediasource.h e315de4 plugins/baloosearch/baloosearchmediasource.cpp 7ebfa61 plugins/baloosearch/imagesearchresulthandler.h PRE-CREATION plugins/baloosearch/imagesearchresulthandler.cpp PRE-CREATION plugins/baloosearch/searchresulthandler.h PRE-CREATION plugins/baloosearch/searchresulthandler.cpp PRE-CREATION plugins/baloosearch/videosearchresulthandler.h PRE-CREATION plugins/baloosearch/videosearchresulthandler.cpp PRE-CREATION Diff: https://git.reviewboard.kde.org/r/117915/diff/ Testing --- Tested with all three types of media, works fine. Unit tests for the new code to follow. Thanks, Shantanu Tushar ___ Plasma-devel mailing list Plasma-devel@kde.org https://mail.kde.org/mailman/listinfo/plasma-devel
Re: Review Request 117931: Restrict sourceSize to screen size
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/117931/ --- (Updated May 2, 2014, 2:22 p.m.) Status -- This change has been marked as submitted. Review request for Plasma. Bugs: 334176 http://bugs.kde.org/show_bug.cgi?id=334176 Repository: plasma-mediacenter Description --- This helps by limiting the image source width so that it doesnt load full versions of images with resolutions more than the screen size. Diffs - mediaelements/imageviewer/ImageViewer.qml b182bf2 Diff: https://git.reviewboard.kde.org/r/117931/diff/ Testing --- works with both portrait and landscape Thanks, Shantanu Tushar ___ Plasma-devel mailing list Plasma-devel@kde.org https://mail.kde.org/mailman/listinfo/plasma-devel
Re: Review Request 117915: Fetch photo-taken Date/Time for Images and file created Date/Time for other files
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/117915/ --- (Updated May 2, 2014, 2:22 p.m.) Status -- This change has been marked as submitted. Review request for Plasma and Vishesh Handa. Repository: plasma-mediacenter Description --- Just like the Nepomuk media source, the Baloo media source needs to provide the date/time information for media. This is used to sort the media to show the more recent media first. For images, the date/time when the photo was actually taken is used, and the file creation date/time is used for other media. Diffs - plugins/baloosearch/CMakeLists.txt 1ff81fb plugins/baloosearch/audiosearchresulthandler.h PRE-CREATION plugins/baloosearch/audiosearchresulthandler.cpp PRE-CREATION plugins/baloosearch/baloosearchmediasource.h e315de4 plugins/baloosearch/baloosearchmediasource.cpp 7ebfa61 plugins/baloosearch/imagesearchresulthandler.h PRE-CREATION plugins/baloosearch/imagesearchresulthandler.cpp PRE-CREATION plugins/baloosearch/searchresulthandler.h PRE-CREATION plugins/baloosearch/searchresulthandler.cpp PRE-CREATION plugins/baloosearch/videosearchresulthandler.h PRE-CREATION plugins/baloosearch/videosearchresulthandler.cpp PRE-CREATION Diff: https://git.reviewboard.kde.org/r/117915/diff/ Testing --- Tested with all three types of media, works fine. Unit tests for the new code to follow. Thanks, Shantanu Tushar ___ Plasma-devel mailing list Plasma-devel@kde.org https://mail.kde.org/mailman/listinfo/plasma-devel
Re: Review Request 117915: Fetch photo-taken Date/Time for Images and file created Date/Time for other files
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/117915/ --- (Updated May 1, 2014, 6:06 a.m.) Status -- This change has been marked as submitted. Review request for Plasma and Vishesh Handa. Repository: plasma-mediacenter Description --- Just like the Nepomuk media source, the Baloo media source needs to provide the date/time information for media. This is used to sort the media to show the more recent media first. For images, the date/time when the photo was actually taken is used, and the file creation date/time is used for other media. Diffs - plugins/baloosearch/CMakeLists.txt 1ff81fb plugins/baloosearch/audiosearchresulthandler.h PRE-CREATION plugins/baloosearch/audiosearchresulthandler.cpp PRE-CREATION plugins/baloosearch/baloosearchmediasource.h e315de4 plugins/baloosearch/baloosearchmediasource.cpp 7ebfa61 plugins/baloosearch/imagesearchresulthandler.h PRE-CREATION plugins/baloosearch/imagesearchresulthandler.cpp PRE-CREATION plugins/baloosearch/searchresulthandler.h PRE-CREATION plugins/baloosearch/searchresulthandler.cpp PRE-CREATION plugins/baloosearch/videosearchresulthandler.h PRE-CREATION plugins/baloosearch/videosearchresulthandler.cpp PRE-CREATION Diff: https://git.reviewboard.kde.org/r/117915/diff/ Testing --- Tested with all three types of media, works fine. Unit tests for the new code to follow. Thanks, Shantanu Tushar ___ Plasma-devel mailing list Plasma-devel@kde.org https://mail.kde.org/mailman/listinfo/plasma-devel
Re: Review Request 117915: Fetch photo-taken Date/Time for Images and file created Date/Time for other files
On May 1, 2014, 6:06 a.m., Commit Hook wrote: This review has been submitted with commit 121e51d74ff50a3644f275d5cfb571326c7a7286 by Shantanu Tushar to branch master. Had pushed this by mistake when trying to fix a bug, reverted it. The review is still open. - Shantanu --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/117915/#review57044 --- On May 1, 2014, 6:06 a.m., Shantanu Tushar wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/117915/ --- (Updated May 1, 2014, 6:06 a.m.) Review request for Plasma and Vishesh Handa. Repository: plasma-mediacenter Description --- Just like the Nepomuk media source, the Baloo media source needs to provide the date/time information for media. This is used to sort the media to show the more recent media first. For images, the date/time when the photo was actually taken is used, and the file creation date/time is used for other media. Diffs - plugins/baloosearch/CMakeLists.txt 1ff81fb plugins/baloosearch/audiosearchresulthandler.h PRE-CREATION plugins/baloosearch/audiosearchresulthandler.cpp PRE-CREATION plugins/baloosearch/baloosearchmediasource.h e315de4 plugins/baloosearch/baloosearchmediasource.cpp 7ebfa61 plugins/baloosearch/imagesearchresulthandler.h PRE-CREATION plugins/baloosearch/imagesearchresulthandler.cpp PRE-CREATION plugins/baloosearch/searchresulthandler.h PRE-CREATION plugins/baloosearch/searchresulthandler.cpp PRE-CREATION plugins/baloosearch/videosearchresulthandler.h PRE-CREATION plugins/baloosearch/videosearchresulthandler.cpp PRE-CREATION Diff: https://git.reviewboard.kde.org/r/117915/diff/ Testing --- Tested with all three types of media, works fine. Unit tests for the new code to follow. Thanks, Shantanu Tushar ___ Plasma-devel mailing list Plasma-devel@kde.org https://mail.kde.org/mailman/listinfo/plasma-devel
Re: Review Request 117923: Export everything by defult for test in plasmamediacentertest librray
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/117923/#review57046 --- Ship it! Cool, looks to do what is expected. Can go in with a minor fix- libs/mediacenter/CMakeLists.txt https://git.reviewboard.kde.org/r/117923/#comment39738 NEPOMUK_CORE_LIBRARY is not required - Shantanu Tushar On May 1, 2014, 7:32 a.m., Sinny Kumari wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/117923/ --- (Updated May 1, 2014, 7:32 a.m.) Review request for Plasma. Repository: plasma-mediacenter Description --- Currently test is getting compiled along with libs. Due to this same source was compiled multiple times. With this patch, Plasma-medacenter creates a new plasmamediacentertest library which has default all exported symbols from libs. Now, test can be linked with libplasmamediacentertest. Diffs - libs/CMakeLists.txt a0a0f72 libs/mediacenter/CMakeLists.txt 2dbec83 libs/mediacenter/test/fakemediavalidator.h 201973c libs/mediacenter/test/fakemediavalidator.cpp 9b2feca libs/mediacenter/test/lastfmimagefetchertest.h 6422824 libs/mediacenter/test/lastfmimagefetchertest.cpp b0fcf6f libs/mediacenter/test/mediacentertest.h 6997ae9 libs/mediacenter/test/mediacentertest.cpp 96a70a9 libs/mediacenter/test/medialibrarytest.h 718457d libs/mediacenter/test/medialibrarytest.cpp 08af239 libs/mediacenter/test/mediatest.h da16e59 libs/mediacenter/test/mediatest.cpp 832fedb libs/mediacenter/test/pmcmediatest.h 2322176 libs/mediacenter/test/pmcmediatest.cpp 0da8dd3 libs/mediacenter/test/singletonfactorytest.h 79e4197 libs/mediacenter/test/singletonfactorytest.cpp 308487d libs/mediacenter/test/testhelpers.h 9825e85 libs/test/CMakeLists.txt PRE-CREATION libs/test/fakemediavalidator.h PRE-CREATION libs/test/fakemediavalidator.cpp PRE-CREATION libs/test/lastfmimagefetchertest.h PRE-CREATION libs/test/lastfmimagefetchertest.cpp PRE-CREATION libs/test/mediacentertest.h PRE-CREATION libs/test/mediacentertest.cpp PRE-CREATION libs/test/medialibrarytest.h PRE-CREATION libs/test/medialibrarytest.cpp PRE-CREATION libs/test/mediatest.h PRE-CREATION libs/test/mediatest.cpp PRE-CREATION libs/test/pmcmediatest.h PRE-CREATION libs/test/pmcmediatest.cpp PRE-CREATION libs/test/singletonfactorytest.h PRE-CREATION libs/test/singletonfactorytest.cpp PRE-CREATION libs/test/testhelpers.h PRE-CREATION Diff: https://git.reviewboard.kde.org/r/117923/diff/ Testing --- Application runs as expected and all unit test passed Thanks, Sinny Kumari ___ Plasma-devel mailing list Plasma-devel@kde.org https://mail.kde.org/mailman/listinfo/plasma-devel
Re: Review Request 117801: Make PMC MPRIS compatible
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/117801/#review57048 --- Ship it! looks good now. Please make sure to run mpristester once more before pushing to master. - Shantanu Tushar On April 27, 2014, 7:12 p.m., Ashish Madeti wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/117801/ --- (Updated April 27, 2014, 7:12 p.m.) Review request for Plasma, Shantanu Tushar, Sinny Kumari, and Sujith Haridasan. Repository: plasma-mediacenter Description --- Add Dbus adaptors through which any other application can programmatically control PMC using MPRIS specifications. http://specifications.freedesktop.org/mpris-spec/latest/index.html Diffs - libs/mediacenter/CMakeLists.txt 37769f6 libs/mediacenter/mpris2/mediaplayer2.h PRE-CREATION libs/mediacenter/mpris2/mediaplayer2.cpp PRE-CREATION libs/mediacenter/mpris2/mediaplayer2player.h PRE-CREATION libs/mediacenter/mpris2/mediaplayer2player.cpp PRE-CREATION libs/mediacenter/mpris2/mpris2.h PRE-CREATION libs/mediacenter/mpris2/mpris2.cpp PRE-CREATION mediaelements/mediaplayer/MediaPlayer.qml 39ed617 shells/newshell/mainwindow.h 9cbf77c shells/newshell/mainwindow.cpp c1be61a shells/newshell/package/contents/ui/mediacenter.qml 50f3576 Diff: https://git.reviewboard.kde.org/r/117801/diff/ Testing --- Tested with qdbusviewer and mpristester : https://github.com/randomguy3/mpristester Thanks, Ashish Madeti ___ Plasma-devel mailing list Plasma-devel@kde.org https://mail.kde.org/mailman/listinfo/plasma-devel
Review Request 117931: Restrict sourceSize to screen size
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/117931/ --- Review request for Plasma. Bugs: 334176 http://bugs.kde.org/show_bug.cgi?id=334176 Repository: plasma-mediacenter Description --- This helps by limiting the image source width so that it doesnt load full versions of images with resolutions more than the screen size. Diffs - mediaelements/imageviewer/ImageViewer.qml b182bf2 Diff: https://git.reviewboard.kde.org/r/117931/diff/ Testing --- works with both portrait and landscape Thanks, Shantanu Tushar ___ Plasma-devel mailing list Plasma-devel@kde.org https://mail.kde.org/mailman/listinfo/plasma-devel
Re: Review Request 117867: Do not cache media lists to a database
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/117867/ --- (Updated April 30, 2014, 5:27 p.m.) Status -- This change has been marked as submitted. Review request for Plasma. Repository: plasma-mediacenter Description --- We started caching results from media sources due to our primary data source (Nepomuk) was quite slow and the experience was quite bad both for fetching media and searching through it. Going forward, Baloo is our primary data source, and was found to be quite performant. Given this, the additional complexity of maintaining a cache in DB is too much for the 1-2 second increase in Media Library load times. Note that we are still able to pull in data from multiple data sources, we just won't store it on disk. Diffs - README be3e060 cmake/FindQxOrm.cmake 3c1fae1 libs/CMakeLists.txt 32d45c2 libs/mediacenter/CMakeLists.txt 809d2c0 libs/mediacenter/album.h 64b1205 libs/mediacenter/album.cpp 391a420 libs/mediacenter/artist.h 064aa02 libs/mediacenter/artist.cpp 6b32269 libs/mediacenter/media.h 830ad57 libs/mediacenter/media.cpp a56a693 libs/mediacenter/medialibrary.h 8f9b5d3 libs/mediacenter/medialibrary.cpp 8f49c05 libs/mediacenter/pmcalbum.h 40c42fe libs/mediacenter/pmcartist.h f15a2e2 libs/mediacenter/pmcmedia.cpp d94d6c7 libs/mediacenter/precompiled.h 73490b8 libs/mediacenter/qxorm_export.h 33fbf11 libs/mediacenter/test/medialibrarytest.h 2f8b476 libs/mediacenter/test/medialibrarytest.cpp bb20a61 libs/mediacenter/test/mediatest.h 965bc7e libs/mediacenter/test/mediatest.cpp 4f44334 Diff: https://git.reviewboard.kde.org/r/117867/diff/ Testing --- Unit tests pass, functionality works as expected. Thanks, Shantanu Tushar ___ Plasma-devel mailing list Plasma-devel@kde.org https://mail.kde.org/mailman/listinfo/plasma-devel
Review Request 117915: Fetch photo-taken Date/Time for Images and file created Date/Time for other files
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/117915/ --- Review request for Plasma and Vishesh Handa. Repository: plasma-mediacenter Description --- Just like the Nepomuk media source, the Baloo media source needs to provide the date/time information for media. This is used to sort the media to show the more recent media first. For images, the date/time when the photo was actually taken is used, and the file creation date/time is used for other media. Diffs - plugins/baloosearch/CMakeLists.txt 1ff81fb plugins/baloosearch/audiosearchresulthandler.h PRE-CREATION plugins/baloosearch/audiosearchresulthandler.cpp PRE-CREATION plugins/baloosearch/baloosearchmediasource.h e315de4 plugins/baloosearch/baloosearchmediasource.cpp 7ebfa61 plugins/baloosearch/imagesearchresulthandler.h PRE-CREATION plugins/baloosearch/imagesearchresulthandler.cpp PRE-CREATION plugins/baloosearch/searchresulthandler.h PRE-CREATION plugins/baloosearch/searchresulthandler.cpp PRE-CREATION plugins/baloosearch/videosearchresulthandler.h PRE-CREATION plugins/baloosearch/videosearchresulthandler.cpp PRE-CREATION Diff: https://git.reviewboard.kde.org/r/117915/diff/ Testing --- Tested with all three types of media, works fine. Unit tests for the new code to follow. Thanks, Shantanu Tushar ___ Plasma-devel mailing list Plasma-devel@kde.org https://mail.kde.org/mailman/listinfo/plasma-devel
Re: Review Request 117800: Move tests inside libs/mediacenter and make them compile with the library sources instead of linking to libplasmamediacenter
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/117800/ --- (Updated April 29, 2014, 9:39 a.m.) Status -- This change has been marked as submitted. Review request for Plasma. Repository: plasma-mediacenter Description --- Doing so gives us two benefits- 1. We no longer have to export classes just to test them 2. We can use ORM mapped classes such as Artist, Album without extra magic for tests (the diff shows deleted/new files because of the move, reviewing CMakeLists.txt changes should be sufficient) Diffs - libs/CMakeLists.txt 59334e9 libs/mediacenter/CMakeLists.txt 37769f6 libs/mediacenter/lastfmimagefetcher.h 4541c66 libs/mediacenter/media.h 2d6cc73 libs/mediacenter/pmcimagecache.h d6d332e libs/mediacenter/subtitleprovider.h 6f0d0f6 libs/test/CMakeLists.txt 52ec617 libs/test/fakemediavalidator.h libs/test/fakemediavalidator.cpp c2fdc17 libs/test/lastfmimagefetchertest.h libs/test/lastfmimagefetchertest.cpp 9d06079 libs/test/mediacentertest.h libs/test/mediacentertest.cpp bb4c4d1 libs/test/medialibrarytest.h libs/test/medialibrarytest.cpp 746d5b2 libs/test/mediatest.h libs/test/mediatest.cpp 86ec0d0 libs/test/pmcmediatest.h libs/test/pmcmediatest.cpp 4bdcae6 libs/test/singletonfactorytest.h libs/test/singletonfactorytest.cpp b9fc987 libs/test/testhelpers.h Diff: https://git.reviewboard.kde.org/r/117800/diff/ Testing --- compiles, all unit tests pass Thanks, Shantanu Tushar ___ Plasma-devel mailing list Plasma-devel@kde.org https://mail.kde.org/mailman/listinfo/plasma-devel
Review Request 117867: Do not cache media lists to a database
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/117867/ --- Review request for Plasma. Repository: plasma-mediacenter Description --- We started caching results from media sources due to our primary data source (Nepomuk) was quite slow and the experience was quite bad both for fetching media and searching through it. Going forward, Baloo is our primary data source, and was found to be quite performant. Given this, the additional complexity of maintaining a cache in DB is too much for the 1-2 second increase in Media Library load times. Note that we are still able to pull in data from multiple data sources, we just won't store it on disk. Diffs - README be3e060 cmake/FindQxOrm.cmake 3c1fae1 libs/CMakeLists.txt 32d45c2 libs/mediacenter/CMakeLists.txt 809d2c0 libs/mediacenter/album.h 64b1205 libs/mediacenter/album.cpp 391a420 libs/mediacenter/artist.h 064aa02 libs/mediacenter/artist.cpp 6b32269 libs/mediacenter/media.h 830ad57 libs/mediacenter/media.cpp a56a693 libs/mediacenter/medialibrary.h 8f9b5d3 libs/mediacenter/medialibrary.cpp 8f49c05 libs/mediacenter/pmcalbum.h 40c42fe libs/mediacenter/pmcartist.h f15a2e2 libs/mediacenter/pmcmedia.cpp d94d6c7 libs/mediacenter/precompiled.h 73490b8 libs/mediacenter/qxorm_export.h 33fbf11 libs/mediacenter/test/medialibrarytest.h 2f8b476 libs/mediacenter/test/medialibrarytest.cpp bb20a61 libs/mediacenter/test/mediatest.h 965bc7e libs/mediacenter/test/mediatest.cpp 4f44334 Diff: https://git.reviewboard.kde.org/r/117867/diff/ Testing --- Unit tests pass, functionality works as expected. Thanks, Shantanu Tushar ___ Plasma-devel mailing list Plasma-devel@kde.org https://mail.kde.org/mailman/listinfo/plasma-devel
Review Request 117800: Move tests inside libs/mediacenter and make them compile with the library sources instead of linking to libplasmamediacenter
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/117800/ --- Review request for Plasma. Repository: plasma-mediacenter Description --- Doing so gives us two benefits- 1. We no longer have to export classes just to test them 2. We can use ORM mapped classes such as Artist, Album without extra magic for tests (the diff shows deleted/new files because of the move, reviewing CMakeLists.txt changes should be sufficient) Diffs - libs/CMakeLists.txt 59334e9 libs/mediacenter/CMakeLists.txt 37769f6 libs/mediacenter/test/CMakeLists.txt PRE-CREATION libs/mediacenter/test/fakemediavalidator.h PRE-CREATION libs/mediacenter/test/fakemediavalidator.cpp PRE-CREATION libs/mediacenter/test/lastfmimagefetchertest.h PRE-CREATION libs/mediacenter/test/lastfmimagefetchertest.cpp PRE-CREATION libs/mediacenter/test/mediacentertest.h PRE-CREATION libs/mediacenter/test/mediacentertest.cpp PRE-CREATION libs/mediacenter/test/medialibrarytest.h PRE-CREATION libs/mediacenter/test/medialibrarytest.cpp PRE-CREATION libs/mediacenter/test/mediatest.h PRE-CREATION libs/mediacenter/test/mediatest.cpp PRE-CREATION libs/mediacenter/test/pmcmediatest.h PRE-CREATION libs/mediacenter/test/pmcmediatest.cpp PRE-CREATION libs/mediacenter/test/singletonfactorytest.h PRE-CREATION libs/mediacenter/test/singletonfactorytest.cpp PRE-CREATION libs/mediacenter/test/testhelpers.h PRE-CREATION libs/test/CMakeLists.txt 52ec617 libs/test/fakemediavalidator.h 201973c libs/test/fakemediavalidator.cpp c2fdc17 libs/test/lastfmimagefetchertest.h 6422824 libs/test/lastfmimagefetchertest.cpp 9d06079 libs/test/mediacentertest.h 6997ae9 libs/test/mediacentertest.cpp bb4c4d1 libs/test/medialibrarytest.h 2f8b476 libs/test/medialibrarytest.cpp 746d5b2 libs/test/mediatest.h 965bc7e libs/test/mediatest.cpp 86ec0d0 libs/test/pmcmediatest.h 2322176 libs/test/pmcmediatest.cpp 4bdcae6 libs/test/singletonfactorytest.h 79e4197 libs/test/singletonfactorytest.cpp b9fc987 libs/test/testhelpers.h 9825e85 Diff: https://git.reviewboard.kde.org/r/117800/diff/ Testing --- compiles, all unit tests pass Thanks, Shantanu Tushar ___ Plasma-devel mailing list Plasma-devel@kde.org https://mail.kde.org/mailman/listinfo/plasma-devel
Re: Review Request 117800: Move tests inside libs/mediacenter and make them compile with the library sources instead of linking to libplasmamediacenter
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/117800/ --- (Updated April 27, 2014, 4:08 p.m.) Review request for Plasma. Changes --- Removed leftover CMakeLists.txt and removed unnecessary moc includes. Repository: plasma-mediacenter Description --- Doing so gives us two benefits- 1. We no longer have to export classes just to test them 2. We can use ORM mapped classes such as Artist, Album without extra magic for tests (the diff shows deleted/new files because of the move, reviewing CMakeLists.txt changes should be sufficient) Diffs (updated) - libs/CMakeLists.txt 59334e9 libs/mediacenter/CMakeLists.txt 37769f6 libs/mediacenter/lastfmimagefetcher.h 4541c66 libs/mediacenter/media.h 2d6cc73 libs/mediacenter/pmcimagecache.h d6d332e libs/mediacenter/subtitleprovider.h 6f0d0f6 libs/test/CMakeLists.txt 52ec617 libs/test/fakemediavalidator.h libs/test/fakemediavalidator.cpp c2fdc17 libs/test/lastfmimagefetchertest.h libs/test/lastfmimagefetchertest.cpp 9d06079 libs/test/mediacentertest.h libs/test/mediacentertest.cpp bb4c4d1 libs/test/medialibrarytest.h libs/test/medialibrarytest.cpp 746d5b2 libs/test/mediatest.h libs/test/mediatest.cpp 86ec0d0 libs/test/pmcmediatest.h libs/test/pmcmediatest.cpp 4bdcae6 libs/test/singletonfactorytest.h libs/test/singletonfactorytest.cpp b9fc987 libs/test/testhelpers.h Diff: https://git.reviewboard.kde.org/r/117800/diff/ Testing --- compiles, all unit tests pass Thanks, Shantanu Tushar ___ Plasma-devel mailing list Plasma-devel@kde.org https://mail.kde.org/mailman/listinfo/plasma-devel
Re: Review Request 117801: Make PMC MPRIS compatible
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/117801/#review56684 --- libs/mediacenter/CMakeLists.txt https://git.reviewboard.kde.org/r/117801/#comment39531 maybe keep mpris2 inside mediacenter/ ? libs/mpris2/mediaplayer2.cpp https://git.reviewboard.kde.org/r/117801/#comment39532 Instead of keeping a copy of the mainwindow, use a signal emitted from MediaPlayer2 and handle this in a MainWindow slot (you'll need to relay it through Mpris2). Its always a good idea to keep dependencies at the minimum. libs/mpris2/mediaplayer2player.cpp https://git.reviewboard.kde.org/r/117801/#comment39535 return mediaPlayerPresent() works fine for me libs/mpris2/mediaplayer2player.cpp https://git.reviewboard.kde.org/r/117801/#comment39533 Move this into a common function so that it can be reused at other places. libs/mpris2/mediaplayer2player.cpp https://git.reviewboard.kde.org/r/117801/#comment39534 if (media) { If you use KDevelop, goto Configure KDevelop Source formatter and choose KDElibs. Then you can use Format files on your code to auto format it. Otherwise you can use astyle. - Shantanu Tushar On April 27, 2014, 2:33 p.m., Ashish Madeti wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/117801/ --- (Updated April 27, 2014, 2:33 p.m.) Review request for Plasma, Shantanu Tushar, Sinny Kumari, and Sujith Haridasan. Repository: plasma-mediacenter Description --- Add Dbus adaptors through which any other application can programmatically control PMC using MPRIS specifications. http://specifications.freedesktop.org/mpris-spec/latest/index.html Diffs - libs/mediacenter/CMakeLists.txt 37769f6 libs/mpris2/mediaplayer2.h PRE-CREATION libs/mpris2/mediaplayer2.cpp PRE-CREATION libs/mpris2/mediaplayer2player.h PRE-CREATION libs/mpris2/mediaplayer2player.cpp PRE-CREATION libs/mpris2/mpris2.h PRE-CREATION libs/mpris2/mpris2.cpp PRE-CREATION mediaelements/mediaplayer/MediaPlayer.qml 39ed617 shells/newshell/mainwindow.cpp c1be61a shells/newshell/package/contents/ui/mediacenter.qml 50f3576 Diff: https://git.reviewboard.kde.org/r/117801/diff/ Testing --- Tested with qdbusviewer and mpristester : https://github.com/randomguy3/mpristester Thanks, Ashish Madeti ___ Plasma-devel mailing list Plasma-devel@kde.org https://mail.kde.org/mailman/listinfo/plasma-devel
Re: Review Request 117732: Move settings class to lib from shell
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/117732/#review56364 --- Ship it! Ship It! - Shantanu Tushar On April 24, 2014, 6:10 a.m., Bhushan Shah wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/117732/ --- (Updated April 24, 2014, 6:10 a.m.) Review request for Plasma, Shantanu Tushar, Sinny Kumari, and Sujith Haridasan. Repository: plasma-mediacenter Description --- As discussed on IRC. Diffs - libs/mediacenter/CMakeLists.txt f5aa827 libs/mediacenter/settings.h PRE-CREATION libs/mediacenter/settings.cpp PRE-CREATION shells/newshell/CMakeLists.txt 9d9ea29 shells/newshell/mainwindow.cpp 9735a0a shells/newshell/settings.h 1fdcb8d shells/newshell/settings.cpp ea7ea7d Diff: https://git.reviewboard.kde.org/r/117732/diff/ Testing --- Compiles Links Test passes Works Thanks, Bhushan Shah ___ Plasma-devel mailing list Plasma-devel@kde.org https://mail.kde.org/mailman/listinfo/plasma-devel
Re: Review Request 117691: Unload libplasma after using it in Plasma Media Center
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/117691/#review56459 --- Ship it! Ship It! - Shantanu Tushar On April 24, 2014, 5:47 p.m., Sinny Kumari wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/117691/ --- (Updated April 24, 2014, 5:47 p.m.) Review request for Plasma and Albert Astals Cid. Repository: plasma-mediacenter Description --- In Kubuntu 14.04 release, we found that Plasma Media Center was unable to play video/audio files. Reason behind it was PMC was linking with libplasma and QtMultimediaKit. libplasma links with libgstreamer1.0 while QtMultimediaKit links with libgstreamer0.10. This patch loads libplasma and unloads it after we are done as suggested on http://lists.kde.org/?l=kde-develm=139811564632441w=2 . Diffs - libs/CMakeLists.txt 7e0a9aa libs/plasmaadapter/CMakeLists.txt PRE-CREATION libs/plasmaadapter/plasmaadapter.cpp PRE-CREATION shells/newshell/CMakeLists.txt bc40044 shells/newshell/mainwindow.h eb2b276 shells/newshell/mainwindow.cpp cfbe097 Diff: https://git.reviewboard.kde.org/r/117691/diff/ Testing --- Able to play audio/video on Kubuntu 14.04 and Fedora 19. Thanks, Sinny Kumari ___ Plasma-devel mailing list Plasma-devel@kde.org https://mail.kde.org/mailman/listinfo/plasma-devel
Loading and unloading a shared library after use
Hi fellow devs, In the latest Kubuntu release, 14.04, Plasma Media Center's primary feature - playback of audio and video has stopped working. This is because we link to libplasma which in turn links to libgstreamer-1.0 while QtMultimediaKit links to libgstreamer-0.10. This upsets Glib because it doesn't want the same binary load both gstreamer-1.0 and gstreamer-0.10[1] Now, we use (and link to) Plasma for two things- - Setting the Oxygen theme - Plasma::Theme::defaultTheme()-setThemeName(oxygen) - Load PMC's main QML file and images which are part of a Plasma::Package Removing these two from the code, and removing the link to libplasma, we can get PMC to play stuff fine. However, this means we 1. use the Air theme which doesn't really look very nice[2] and 2. we need to figure out another way to locate the installed QML files. The latter is workable, but we really want to be able to use Oxygen Plasma theme. We do both these things when the app starts, so I am wondering if there is a way we could dynamically load libplasma, make these two method calls, and somehow unload libplasma. I have two questions - - Does that ^ even make sense? Will it solve the problem? - If yes, how exactly do you do it? Help appreciated, so that our users can play their audio and video again ;) Cheers, [1] http://cgit.freedesktop.org/gstreamer/gstreamer/tree/docs/random/porting-to-1.0.txtsection Troubleshooting [2] http://i.imgur.com/0k1TTwW.png -- Shantanu Tushar(UTC +0530) http://www.shantanutushar.com ___ Plasma-devel mailing list Plasma-devel@kde.org https://mail.kde.org/mailman/listinfo/plasma-devel
Re: Review Request 117368: Resolve first-time-run crash of PMC due to not existing data directory.
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/117368/#review54978 --- Ship it! Looks good to me, I have one suggestion below. Make sure that all tests pass before pushing. libs/mediacenter/mediacenter.cpp https://git.reviewboard.kde.org/r/117368/#comment38361 const QString pmc_path - Shantanu Tushar On April 4, 2014, 11:42 a.m., Nikolaos Chatzidakis wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/117368/ --- (Updated April 4, 2014, 11:42 a.m.) Review request for Plasma, Shantanu Tushar and Sinny Kumari. Repository: plasma-mediacenter Description --- This patch resolves the crash of PMC if it's running for the first time and its data directory does not exist in ~/.kde4/share/apps. Diffs - libs/mediacenter/mediacenter.cpp 35240d3 Diff: https://git.reviewboard.kde.org/r/117368/diff/ Testing --- Tested the fix, it creates the directory if it doesn't exist (first-time-run assumption). Thanks, Nikolaos Chatzidakis ___ Plasma-devel mailing list Plasma-devel@kde.org https://mail.kde.org/mailman/listinfo/plasma-devel
Re: Review Request 117365: Add Genre to metadata PMC keeps
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/117365/#review54982 --- Works fine, I can see the values in the table. However, the actual set of the value should happen inside Media instead of MediaLibrary as the comments below. libs/mediacenter/media.h https://git.reviewboard.kde.org/r/117365/#comment38362 doesnt need to be const QString for the return, just QString will do. The const QString you see at other places is quite some bad luck and needs to be fixed to QString as well, separately. For more http://techbase.kde.org/Policies/Library_Code_Policy#Const_References libs/mediacenter/medialibrary.cpp https://git.reviewboard.kde.org/r/117365/#comment38363 Instead of handling this here, you should let media-setValueForRole take care of updating the value. Just implement MediaCenter::GenreRole in Media::setValueForRole. Also, if you set wasUpdated to true always, the point of wasUpdated is lost. It is supposed to be set to true only when the old value and new value are different (which is when the setValueForRole method is supposed to return true). At the end, this else if should be removed (notice how there is no else if for createdAt (extractAndSaveDurationInfo should not have been there as well, it was missed and should be removed separately). libs/mediacenter/medialibrary.cpp https://git.reviewboard.kde.org/r/117365/#comment38364 same as above plugins/kdedesktopsearch/kdemetadatamediasource.cpp https://git.reviewboard.kde.org/r/117365/#comment38365 Just for consistency, move this to the line after CreatedAtRole - Shantanu Tushar On April 4, 2014, 10:51 a.m., Ashish Madeti wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/117365/ --- (Updated April 4, 2014, 10:51 a.m.) Review request for Plasma, Shantanu Tushar and Sinny Kumari. Repository: plasma-mediacenter Description --- Added genre to metadata pmc keeps so that it too can be retrieved from the media library when needed. Diffs - libs/mediacenter/media.h 2bdef4d libs/mediacenter/media.cpp 32b9f19 libs/mediacenter/mediacenter.h a2855dd libs/mediacenter/mediacenter.cpp 35240d3 libs/mediacenter/medialibrary.cpp 713827a libs/mediacenter/pmcmedia.h 68275f2 libs/mediacenter/pmcmedia.cpp a266a26 libs/mediacenter/pmcmetadatamodel.cpp 2d5fd6b plugins/kdedesktopsearch/kdemetadatamediasource.cpp e8f18eb Diff: https://git.reviewboard.kde.org/r/117365/diff/ Testing --- Thanks, Ashish Madeti ___ Plasma-devel mailing list Plasma-devel@kde.org https://mail.kde.org/mailman/listinfo/plasma-devel
Re: Review Request 117365: Add Genre to metadata PMC keeps
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/117365/#review55005 --- Ship it! Ship It! - Shantanu Tushar On April 4, 2014, 1:02 p.m., Ashish Madeti wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/117365/ --- (Updated April 4, 2014, 1:02 p.m.) Review request for Plasma, Shantanu Tushar and Sinny Kumari. Repository: plasma-mediacenter Description --- Added genre to metadata pmc keeps so that it too can be retrieved from the media library when needed. Diffs - libs/mediacenter/media.h 2bdef4d libs/mediacenter/media.cpp 32b9f19 libs/mediacenter/mediacenter.h a2855dd libs/mediacenter/mediacenter.cpp 35240d3 libs/mediacenter/pmcmedia.h 68275f2 libs/mediacenter/pmcmedia.cpp a266a26 libs/mediacenter/pmcmetadatamodel.cpp 2d5fd6b plugins/kdedesktopsearch/kdemetadatamediasource.cpp e8f18eb Diff: https://git.reviewboard.kde.org/r/117365/diff/ Testing --- Thanks, Ashish Madeti ___ Plasma-devel mailing list Plasma-devel@kde.org https://mail.kde.org/mailman/listinfo/plasma-devel
Re: Review Request 116874: Patch for the next and previous button functionality in an inactive playlist.
On March 27, 2014, 1:05 p.m., Sebastian Kügler wrote: mediaelements/playlist/Playlist.qml, line 159 https://git.reviewboard.kde.org/r/116874/diff/3/?file=257374#file257374line159 indenting is wrong now R.Harish Navnit wrote: I have added a new diff file after I pulled the latest source and built successfully. Please do check that patch(https://git.reviewboard.kde.org/media/uploaded/files/2014/03/27/7f72b75b-6d72-4a5f-9460-699bb3846624__updated_fix.patch), I've added {} to the if-statement in that one. It however, isn't disabled when the playlist isn't active. Can you update the diff in the review instead of attaching it? Otherwise its not possible to leave comments on code. - Shantanu --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/116874/#review54293 --- On March 27, 2014, 11:22 a.m., R.Harish Navnit wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/116874/ --- (Updated March 27, 2014, 11:22 a.m.) Review request for Plasma, Shantanu Tushar and Sinny Kumari. Bugs: 330990 and Bug http://bugs.kde.org/show_bug.cgi?id=330990 http://bugs.kde.org/show_bug.cgi?id=Bug Repository: plasma-mediacenter Description --- Now the next and previous buttons do not work unless a media is playing. Diffs - mediaelements/playlist/Playlist.qml fd83c21 Diff: https://git.reviewboard.kde.org/r/116874/diff/ Testing --- I've done the testing for this patch. These are the steps that I followed. 1. Added a new playlist and added a few songs to the playlist. 2. Re-opened Plasma Media Center 3. Clicked the Next and Previous buttons. 4. Nothing was played. 5. Selected a song from the list and then tested the next and previous buttons. 6. The buttons worked seamlessly. File Attachments pulled the latest source and made changes https://git.reviewboard.kde.org/media/uploaded/files/2014/03/27/7f72b75b-6d72-4a5f-9460-699bb3846624__updated_fix.patch Thanks, R.Harish Navnit ___ Plasma-devel mailing list Plasma-devel@kde.org https://mail.kde.org/mailman/listinfo/plasma-devel
Re: Review Request 117101: This patch fixes font size behaviour when pmc is resized.
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/117101/#review54353 --- Doesn't really look on my box http://i.imgur.com/nJMYi9E.jpg Also, I think the right fix for these things is to use one of PlasmaExtras.Heading, PlasmaExtras.Title (or PlasmaExtras.Paragraph when applicable). Can someone else comment on whether making the font depend on window size is a good idea? - Shantanu Tushar On March 26, 2014, 9:29 p.m., Nikolaos Chatzidakis wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/117101/ --- (Updated March 26, 2014, 9:29 p.m.) Review request for Plasma and Shantanu Tushar. Repository: plasma-mediacenter Description --- This is my first patch trying to hack PMC. Untill now, whenever PMC was resized, the menu strings font size remained fixed. Now it changes the font size according to parents dimensions. Diffs - mediaelements/mediawelcome/BackendsListDelegate.qml 4840982 mediaelements/mediawelcome/MediaWelcome.qml a3108d8 Diff: https://git.reviewboard.kde.org/r/117101/diff/ Testing --- The size of the icons in the menus remains the same, due to the file format (png). Converting these graphics to SVG may solve the graphics resize issue as well. Thanks, Nikolaos Chatzidakis ___ Plasma-devel mailing list Plasma-devel@kde.org https://mail.kde.org/mailman/listinfo/plasma-devel
Re: Review Request 116907: Implemented Player MPRIS spec adaptor
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/116907/#review54208 --- I have general feedback around the code below. It will be nice if you can poke someone who has written MPRIS-related code before to review this before finalizing. libs/mpris2/mediaplayer2.cpp https://git.reviewboard.kde.org/r/116907/#comment37904 This would look better- QDBusConnection::sessionBus().registerObject(/org/mpris/MediaPlayer2, this, QDBusConnection::ExportAllSlots | QDBusConnection::ExportAllProperties | QDBusConnection::ExportAllSignals); libs/mpris2/mediaplayer2player.h https://git.reviewboard.kde.org/r/116907/#comment37906 Are these supposed to be called on this object from outside? If yes, why are these protected? If no, why Q_INVOKABLE? libs/mpris2/mediaplayer2player.cpp https://git.reviewboard.kde.org/r/116907/#comment37907 How is 64 decided? libs/mpris2/mediaplayer2player.cpp https://git.reviewboard.kde.org/r/116907/#comment37909 if and else need braces, even when single-line, see http://techbase.kde.org/Policies/Kdelibs_Coding_Style libs/mpris2/mediaplayer2player.cpp https://git.reviewboard.kde.org/r/116907/#comment37910 const int shells/newshell/mainwindow.cpp https://git.reviewboard.kde.org/r/116907/#comment37911 whitespace shells/newshell/package/contents/ui/mediacenter.qml https://git.reviewboard.kde.org/r/116907/#comment37912 instead of this, can't you use onPlayRequested instead? Its better to avoid dependencies for components (In this case Playlist requires a mprisPlayer to work). - Shantanu Tushar On March 26, 2014, 6:46 a.m., Ashish Madeti wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/116907/ --- (Updated March 26, 2014, 6:46 a.m.) Review request for Plasma, Shantanu Tushar, Sinny Kumari, and Sujith Haridasan. Repository: plasma-mediacenter Description --- Implemented Player DBus adaptor of MPRIS specifications for Plasma Mediacenter. Specification reference: http://specifications.freedesktop.org/mpris-spec/latest/Player_Interface.html Some more work needs to be done in the adaptor which I plan to do soon. Diffs - libs/mpris2/mpris2.cpp a8ad3ef mediaelements/mediaplayer/MediaPlayer.qml 39ed617 mediaelements/playlist/Playlist.qml 5dde297 shells/newshell/mainwindow.cpp d2d71d4 shells/newshell/package/contents/ui/mediacenter.qml bac33c2 libs/mpris2/mediaplayer2.h e68bc5c libs/mpris2/mediaplayer2.cpp ff96618 libs/mpris2/mediaplayer2player.h 203d681 libs/mpris2/mediaplayer2player.cpp 7871efa Diff: https://git.reviewboard.kde.org/r/116907/diff/ Testing --- Tested with qdbusviewer, the properties and methods are working fine. Thanks, Ashish Madeti ___ Plasma-devel mailing list Plasma-devel@kde.org https://mail.kde.org/mailman/listinfo/plasma-devel
Review Request 116898: Get rid of unnecessary Q_INVOKABLE declarations
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/116898/ --- Review request for Plasma and Sujith Haridasan. Repository: plasma-mediacenter Description --- Methods supposed to be used as READ of Q_PROPERTY properties need not be invokable. Same goes for slots. Diffs - libs/mpris2/mediaplayer2.h e68bc5c libs/mpris2/mediaplayer2player.h 203d681 libs/mpris2/mpris2.h 0df64f4 libs/mpris2/mpris2.cpp a8ad3ef shells/newshell/main.cpp bab6915 shells/newshell/mainwindow.cpp d2d71d4 Diff: https://git.reviewboard.kde.org/r/116898/diff/ Testing --- Thanks, Shantanu Tushar ___ Plasma-devel mailing list Plasma-devel@kde.org https://mail.kde.org/mailman/listinfo/plasma-devel
Re: Review Request 116554: Implementation of autoplay when the PlayAll button in pressed
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/116554/#review53438 --- shells/newshell/mainwindow.cpp https://git.reviewboard.kde.org/r/116554/#comment37583 This processCommandLineArgs call (only the call) is still needed, otherwise media passed as cmd line args wont play. Please test before updating diff. - Shantanu Tushar On March 19, 2014, 6:30 a.m., Harshit Agarwal wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/116554/ --- (Updated March 19, 2014, 6:30 a.m.) Review request for Plasma, Akshay Ratan, Shantanu Tushar, Sinny Kumari, and Sujith Haridasan. Repository: plasma-mediacenter Description --- Referring to Bug #331040 The functionality of the media starting to play on the press of the Play All button has been implemented. It has been noted and implemented that the media doesn't start over when new songs/videos are added to an existing playlist. Diffs - browsingbackends/localfiles/localfilesabstractbackend.cpp faaafa7 shells/newshell/mainwindow.h f224060 shells/newshell/mainwindow.cpp f721d68 shells/newshell/package/contents/ui/mediacenter.qml bac33c2 Diff: https://git.reviewboard.kde.org/r/116554/diff/ Testing --- The testing has been done through various test scenarios. Thanks, Harshit Agarwal ___ Plasma-devel mailing list Plasma-devel@kde.org https://mail.kde.org/mailman/listinfo/plasma-devel
Re: Review Request 116554: Implementation of autoplay when the PlayAll button in pressed
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/116554/#review53472 --- Ship it! Ship It! - Shantanu Tushar On March 19, 2014, 6:40 p.m., Harshit Agarwal wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/116554/ --- (Updated March 19, 2014, 6:40 p.m.) Review request for Plasma, Akshay Ratan, Shantanu Tushar, Sinny Kumari, and Sujith Haridasan. Repository: plasma-mediacenter Description --- Referring to Bug #331040 The functionality of the media starting to play on the press of the Play All button has been implemented. It has been noted and implemented that the media doesn't start over when new songs/videos are added to an existing playlist. Diffs - browsingbackends/localfiles/localfilesabstractbackend.cpp faaafa7 shells/newshell/mainwindow.h f224060 shells/newshell/mainwindow.cpp f721d68 shells/newshell/package/contents/ui/mediacenter.qml bac33c2 Diff: https://git.reviewboard.kde.org/r/116554/diff/ Testing --- The testing has been done through various test scenarios. Thanks, Harshit Agarwal ___ Plasma-devel mailing list Plasma-devel@kde.org https://mail.kde.org/mailman/listinfo/plasma-devel