D7750: fix crash when more than one instances of ExtractorCollection are destructed

2017-09-10 Thread Matthieu Gallien
mgallien added a comment. In https://phabricator.kde.org/D7750#144373, @anthonyfieroni wrote: > https://phabricator.kde.org/source/kfilemetadata/browse/master/src/extractorcollection.cpp;621101fd9e9d82be3d84f2140a4bf53ea13fd3f0$137 > Look it leak now, no? Sorry, I misread your

D7750: fix crash when more than one instances of ExtractorCollection are destructed

2017-09-12 Thread Matthieu Gallien
mgallien updated this revision to Diff 19450. mgallien added a comment. remove duplicated code REPOSITORY R286 KFileMetaData CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D7750?vs=19416=19450 BRANCH fixPluginDelete REVISION DETAIL https://phabricator.kde.org/D7750 AFFECTED

D7330: Don't enter test subdirectories if BUILD_TESTING=OFF

2017-09-30 Thread Matthieu Gallien
mgallien accepted this revision. This revision is now accepted and ready to land. REPOSITORY R293 Baloo BRANCH master REVISION DETAIL https://phabricator.kde.org/D7330 To: asturmlechner, #frameworks, dfaure, mgallien

D7750: fix crash when more than one instances of ExtractorCollection are destructed

2017-10-01 Thread Matthieu Gallien
mgallien updated this revision to Diff 20200. mgallien added a comment. add a setAutoDeletePlugin method and modify the name of the enum value to not use the shared word REPOSITORY R286 KFileMetaData CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D7750?vs=19918=20200 BRANCH

D7750: fix crash when more than one instances of ExtractorCollection are destructed

2017-10-02 Thread Matthieu Gallien
This revision was automatically updated to reflect the committed changes. Closed by commit R286:b53a72743601: fix crash when more than one instances of ExtractorCollection are destructed (authored by mgallien). REPOSITORY R286 KFileMetaData CHANGES SINCE LAST UPDATE

D7750: fix crash when more than one instances of ExtractorCollection are destructed

2017-09-25 Thread Matthieu Gallien
mgallien updated this revision to Diff 19917. mgallien added a comment. reduce the scope of modifications, use an enum to indicate if the plugin should be deleted and use private methods instead of direct access to private class REPOSITORY R286 KFileMetaData CHANGES SINCE LAST UPDATE

D7750: fix crash when more than one instances of ExtractorCollection are destructed

2017-09-25 Thread Matthieu Gallien
mgallien updated this revision to Diff 19918. mgallien marked 6 inline comments as done. mgallien added a comment. add missing noexcept REPOSITORY R286 KFileMetaData CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D7750?vs=19917=19918 BRANCH fixPluginDelete REVISION DETAIL

D7750: fix crash when more than one instances of ExtractorCollection are destructed

2017-09-25 Thread Matthieu Gallien
mgallien marked an inline comment as done. REPOSITORY R286 KFileMetaData REVISION DETAIL https://phabricator.kde.org/D7750 To: mgallien, #frameworks, dfaure Cc: dfaure, anthonyfieroni

D7750: fix crash when more than one instances of ExtractorCollection are destructed

2017-09-25 Thread Matthieu Gallien
mgallien added a comment. Reverted most of my changes and only use a private method with two parameters to set the ExtractorPlugin in Extractor class. The second parameter should indicate if the Extractor instance is owner of the plugin or not. The private class is no longer included in

D8007: popplerextractor: don't try to guess the title if there isn't one.

2017-11-27 Thread Matthieu Gallien
mgallien added a comment. @vhanda Thanks a lot to have took time to answer. I really appreciate your help. I took a long time to understand that fixing or modifying the extractors is not enough to get modified data when querying Baloo. I would like to fix that in the future. Do you

D8007: popplerextractor: don't try to guess the title if there isn't one.

2017-11-26 Thread Matthieu Gallien
mgallien added subscribers: vhanda, mgallien. mgallien accepted this revision. mgallien added a comment. This revision is now accepted and ready to land. Sorry for me being late to review your work. I had not noticed it. Thanks for your work. I am all for this change. The code you are

D5417: add an extractor using qtmultimedia

2017-12-02 Thread Matthieu Gallien
mgallien added a comment. In https://phabricator.kde.org/D5417#174323, @dfaure wrote: > This test seems to fail depending on how QtMultimedia was built. See failure from the flaska CI, below. Can something be done to detect this? The extractor is not working well. On Windows, it

D8886: remove extractor based on QtMultimedia

2017-12-03 Thread Matthieu Gallien
mgallien added a comment. In https://phabricator.kde.org/D8886#174348, @dfaure wrote: > Your call. But since that would also fix CI, I'm in favour. I did read the code of Qt in the gstreamer support and it seems that it is not possible to get metadata outside a QMediaPlayer. I

D8886: remove extractor based on QtMultimedia

2017-12-03 Thread Matthieu Gallien
This revision was automatically updated to reflect the committed changes. Closed by commit R286:182f76c25275: remove extractor based on QtMultimedia (authored by mgallien). REPOSITORY R286 KFileMetaData CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D8886?vs=22575=23399 REVISION

D4911: add Baloo DBus signals for moved or removed files

2017-11-19 Thread Matthieu Gallien
mgallien added a comment. Hello @ngraham and @cullmann, I lack time to properly maintain stuff I added to KFileMetaData. I do not think I may be able to put a lot of energy in Baloo. At the same time, this is a solution that quite work for me and the usage I do in Elisa. I am just trying

D6317: fix crash on Windows when deleting an instance of QtMultimediaExtractor

2017-11-18 Thread Matthieu Gallien
mgallien abandoned this revision. mgallien added a comment. I do not have time to properly work on that. See https://phabricator.kde.org/D8886 . REPOSITORY R286 KFileMetaData REVISION DETAIL https://phabricator.kde.org/D6317 To: mgallien, #frameworks, davidedmundson Cc: davidedmundson,

D8886: remove extractor based on QtMultimedia

2017-11-18 Thread Matthieu Gallien
mgallien created this revision. mgallien added a reviewer: Frameworks. Restricted Application added a project: Frameworks. REVISION SUMMARY It is not working well and do not provide much advantages on Linux and Windows platforms It is based on QMediaPlayer and that is not playing well

D4911: add Baloo DBus signals for moved or removed files

2017-11-18 Thread Matthieu Gallien
mgallien added a comment. Ping REPOSITORY R293 Baloo REVISION DETAIL https://phabricator.kde.org/D4911 To: mgallien, vhanda, dfaure Cc: cullmann, apol, #frameworks

D6317: fix crash on Windows when deleting an instance of QtMultimediaExtractor

2017-11-02 Thread Matthieu Gallien
mgallien added a comment. Sorry for the delay, currently I have no easy access to a development computer running Windows. David, I am afraid I have not understood correctly your suggestion. Did you suggest to make mMetadataExtractor have its thread as Qt parent ? REPOSITORY R286

D8330: Open files in TagLib extractor readonly

2017-11-08 Thread Matthieu Gallien
mgallien added a comment. Yes exactly. REPOSITORY R286 KFileMetaData BRANCH readOnly REVISION DETAIL https://phabricator.kde.org/D8330 To: davidk, #frameworks, vhanda, cgiboudeaux, dfaure, mgallien Cc: mgallien, ngraham, #frameworks

D8330: Open files in TagLib extractor readonly

2017-11-04 Thread Matthieu Gallien
mgallien accepted this revision. mgallien added a comment. This revision is now accepted and ready to land. If other extractors are already doing that,I guess it is good enough for now. I guess somebody may one day come up with better error logging (i.e. the user can discover it).

D9233: Fix build against older TagLib

2017-12-06 Thread Matthieu Gallien
mgallien accepted this revision. REPOSITORY R286 KFileMetaData REVISION DETAIL https://phabricator.kde.org/D9233 To: dvratil, mgallien, aacid Cc: #frameworks

D12156: implement reading of rating tag

2018-05-05 Thread Matthieu Gallien
mgallien added a comment. In D12156#254055 , @astippich wrote: > In D12156#252575 , @mgallien wrote: > > > If I have correctly understood the ideas behind the conception of Baloo, we should

D12197: autotests: Test for multiple values

2018-05-05 Thread Matthieu Gallien
mgallien accepted this revision. This revision is now accepted and ready to land. REPOSITORY R286 KFileMetaData BRANCH multi-value-test (branched from master) REVISION DETAIL https://phabricator.kde.org/D12197 To: michaelh, #baloo, #frameworks, mgallien, bruns Cc: bruns, ashaposhnikov,

Re: kfilemetadata compile failure

2018-05-14 Thread Matthieu Gallien
On lundi 14 mai 2018 19:57:58 CEST Albert Astals Cid wrote: > El diumenge, 13 de maig de 2018, a les 23:15:56 CEST, Matthieu Gallien va > > escriure: > > On dimanche 13 mai 2018 23:03:02 CEST Albert Astals Cid wrote: > > > El divendres, 11 de maig de 2018, a les 23:22:1

D12886: check that ffmpeg is at least version 3.1 that introduce the API we require

2018-05-14 Thread Matthieu Gallien
mgallien created this revision. mgallien added reviewers: dfaure, michaelh, jriddell. Restricted Application added projects: Frameworks, Baloo. Restricted Application added subscribers: Baloo, kde-frameworks-devel. mgallien requested review of this revision. REVISION SUMMARY check that ffmpeg

D4911: add Baloo DBus signals for moved or removed files

2018-04-28 Thread Matthieu Gallien
This revision was automatically updated to reflect the committed changes. Closed by commit R293:ef0e268c6577: add Baloo DBus signals for moved or removed files (authored by mgallien). CHANGED PRIOR TO COMMIT https://phabricator.kde.org/D4911?vs=14953=33246#toc REPOSITORY R293 Baloo CHANGES

D12578: fix some issues reported by clazy

2018-04-28 Thread Matthieu Gallien
mgallien created this revision. mgallien added reviewers: Baloo, Frameworks. Restricted Application added projects: Frameworks, Baloo. mgallien requested review of this revision. REVISION SUMMARY fix some issues reported by clazy TEST PLAN tests are OK REPOSITORY R293 Baloo BRANCH

D12320: [RFC] add ability to read embedded cover files

2018-05-12 Thread Matthieu Gallien
mgallien added a comment. I am away from keyboard. Can we move forward ? Does it make sense to aim for having only one copy of this code and makes the kio code depends on KFileMetaData ? REPOSITORY R286 KFileMetaData REVISION DETAIL https://phabricator.kde.org/D12320 To: astippich,

Re: kfilemetadata compile failure

2018-05-13 Thread Matthieu Gallien
On dimanche 13 mai 2018 23:03:02 CEST Albert Astals Cid wrote: > El divendres, 11 de maig de 2018, a les 23:22:15 CEST, Jonathan Riddell va > > escriure: > > On Thu, Apr 26, 2018 at 08:31:46AM +0200, Kevin Funk wrote: > > > On Wednesday, 25 April 2018 14:34:58 CEST Jonathan Riddell wrote: > > > >

D12578: fix some issues reported by clazy

2018-05-15 Thread Matthieu Gallien
mgallien added a comment. In D12578#262831 , @bruns wrote: > @mgallien - I think this is trivial to fix up - can you do, so we have one less request open? I will do but as I had started working on that as a low priority task, I have not

D12886: check that ffmpeg is at least version 3.1 that introduce the API we require

2018-05-15 Thread Matthieu Gallien
This revision was automatically updated to reflect the committed changes. Closed by commit R286:3415015e3d45: check that ffmpeg is at least version 3.1 that introduce the API we require (authored by mgallien). REPOSITORY R286 KFileMetaData CHANGES SINCE LAST UPDATE

D12578: fix some issues reported by clazy

2018-05-15 Thread Matthieu Gallien
mgallien updated this revision to Diff 34223. mgallien added a comment. fix some issues REPOSITORY R293 Baloo CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D12578?vs=33247=34223 BRANCH arcpatch-D12578 REVISION DETAIL https://phabricator.kde.org/D12578 AFFECTED FILES

D12156: implement reading of rating tag

2018-05-22 Thread Matthieu Gallien
mgallien added a subscriber: cgilles. mgallien added a comment. @cgilles Are you somehow using KFileMetaData (seems correct if one looks at the Debian package dependencies) ? This diff is about adding a way to read ratings from "tags" native to a specific file format. This would be in

D12992: New elisa icon

2018-05-22 Thread Matthieu Gallien
mgallien added subscribers: astippich, januz. mgallien added a comment. In D12992#266288 , @alex-l wrote: > F5863162: image.png > > ^ I'm for this one without the quaver I also prefer this one

D12578: fix some issues reported by clazy

2018-05-17 Thread Matthieu Gallien
mgallien marked an inline comment as done. mgallien added a comment. @bruns Thanks for the review and sorry for my slowness. REPOSITORY R293 Baloo REVISION DETAIL https://phabricator.kde.org/D12578 To: mgallien, #baloo, #frameworks, bruns Cc: kde-frameworks-devel, bruns, ashaposhnikov,

D12578: fix some issues reported by clazy

2018-05-17 Thread Matthieu Gallien
mgallien updated this revision to Diff 34389. mgallien added a comment. - fix one more issue REPOSITORY R293 Baloo CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D12578?vs=34223=34389 BRANCH arcpatch-D12578 REVISION DETAIL https://phabricator.kde.org/D12578 AFFECTED FILES

D12578: fix some issues reported by clazy

2018-05-17 Thread Matthieu Gallien
This revision was automatically updated to reflect the committed changes. Closed by commit R293:f887468b172a: fix some issues reported by clazy (authored by mgallien). REPOSITORY R293 Baloo CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D12578?vs=34389=34390 REVISION DETAIL

D13302: check for needed version of libavcode, libavformat and libavutil

2018-06-07 Thread Matthieu Gallien
This revision was automatically updated to reflect the committed changes. Closed by commit R286:461f8ec81b81: check for needed version of libavcode, libavformat and libavutil (authored by mgallien). REPOSITORY R286 KFileMetaData CHANGES SINCE LAST UPDATE

D12992: New elisa icon

2018-05-27 Thread Matthieu Gallien
mgallien added a subscriber: paullesur. mgallien added a comment. @januz and @astippich I really understand your point of view about the current icon. I share your point of view the need to have an icon easy to differentiate from many other players. For example, I would like Elisa be easy

D13302: [WIP] check for needed version of libavcode, libavformat and libavutil

2018-06-03 Thread Matthieu Gallien
mgallien created this revision. mgallien added reviewers: romangg, adridg. Restricted Application added projects: Frameworks, Baloo. Restricted Application added subscribers: Baloo, kde-frameworks-devel. mgallien requested review of this revision. REVISION SUMMARY check for needed version of

D13302: check for needed version of libavcode, libavformat and libavutil

2018-06-03 Thread Matthieu Gallien
mgallien updated this revision to Diff 35457. mgallien added a comment. - prefer usage of CHECK_STRUCT_HAS_MEMBER instead of having test code the test is shorter with CHECK_STRUCT_HAS_MEMBER (thanks @adridg for the help) REPOSITORY R286 KFileMetaData CHANGES SINCE LAST UPDATE

D13302: check for needed version of libavcode, libavformat and libavutil

2018-06-03 Thread Matthieu Gallien
mgallien retitled this revision from "[WIP] check for needed version of libavcode, libavformat and libavutil" to "check for needed version of libavcode, libavformat and libavutil". mgallien edited the test plan for this revision. REPOSITORY R286 KFileMetaData REVISION DETAIL

D13302: [WIP] check for needed version of libavcode, libavformat and libavutil

2018-06-03 Thread Matthieu Gallien
mgallien updated this revision to Diff 35456. mgallien added a comment. - check AVStrean structure have codecpar member working check for codecpar member in AVStream structure should I try to make the result variable be not in the cmake cache ? REPOSITORY R286 KFileMetaData

D12320: [RFC] add ability to read embedded cover files

2018-05-31 Thread Matthieu Gallien
mgallien added a comment. In D12320#271300 , @astippich wrote: > any more comments? would be nice to ship it with Kf 5.48 @bruns you requested changes. Can you have a look when you have time ? REPOSITORY R286 KFileMetaData REVISION

D12320: [RFC] add ability to read embedded cover files

2018-06-02 Thread Matthieu Gallien
mgallien added a comment. In D12320#272370 , @bruns wrote: > as the remaining issues are formatting only, this is an `accept` by me save the requested changes. > @mgallien - if i am late to accept, can you do it? Yes sure. I have just

D13302: check for needed version of libavcode, libavformat and libavutil

2018-06-06 Thread Matthieu Gallien
mgallien updated this revision to Diff 35712. mgallien added a comment. - checks if FFmpeg provides the new API and use it only in this case extend the existing configure header to allow to compile both versions of the code in the FFmpeg extractor in case the new API exists, we

D12992: New elisa icon

2018-06-06 Thread Matthieu Gallien
mgallien added a comment. In D12992#269849 , @abetts wrote: > In D12992#269803 , @lshoravi wrote: > > > @abetts Yes, I've been concepting a little on different ideas but not really coming anywhere

D13216: Overhaul the file index scheduler.

2018-06-06 Thread Matthieu Gallien
mgallien added a comment. Sounds good to me. I will try to do a proper review as soon as I can. Sorry for the delay. REPOSITORY R293 Baloo REVISION DETAIL https://phabricator.kde.org/D13216 To: smithjd, bruns, mgallien Cc: kde-frameworks-devel, #baloo, ashaposhnikov, michaelh,

Re: KDE Frameworks 5.47.0

2018-06-06 Thread Matthieu Gallien
ng understood that was the only feasible solution. > Cheers, > Albert > > > Jonathan Best regards -- Matthieu Gallien

D12156: implement reading of rating tag

2018-05-31 Thread Matthieu Gallien
mgallien accepted this revision. mgallien added a comment. This revision is now accepted and ready to land. I hope application developers will not complain about the need to support a second api to get ratings. I am still unsure this the best way to add this support. REPOSITORY R286

D12320: {RFC] add ability to read embedded cover files

2018-05-02 Thread Matthieu Gallien
mgallien added a comment. Thanks for your work. Please find a few remarks inline. INLINE COMMENTS > embeddedimagedata.h:25 > +#include "kfilemetadata_export.h" > +#include > + You can do a forward declaration of QMimeDatabase because it is only referred through a reference. >

D9408: extractors: Hide warnings from system headers

2018-04-26 Thread Matthieu Gallien
mgallien requested changes to this revision. mgallien added a comment. This revision now requires changes to proceed. Please have a look at the issue. Thanks INLINE COMMENTS > CMakeLists.txt:35 > +target_include_directories(kfilemetadata_exiv2extractor SYSTEM PRIVATE >

D9408: extractors: Hide warnings from system headers

2018-04-26 Thread Matthieu Gallien
mgallien accepted this revision. This revision is now accepted and ready to land. REPOSITORY R286 KFileMetaData BRANCH master REVISION DETAIL https://phabricator.kde.org/D9408 To: kfunk, mgallien Cc: #frameworks, ashaposhnikov, michaelh, astippich, spoorun, bruns

D9880: Fix build with >=attr-2.4.48

2018-01-15 Thread Matthieu Gallien
mgallien added a comment. Are you sure attr/xattr.h is deprecated ? I did look at what debian provides with version 2.4.47 and there seems to have a valid attr/xattr.h file. I also did found some partial information by searching the web but nothing conclusive. REPOSITORY R286

D9880: Fix build with >=attr-2.4.48

2018-01-16 Thread Matthieu Gallien
mgallien accepted this revision. mgallien added a comment. This revision is now accepted and ready to land. Sorry for not having checked the correct git repository. REPOSITORY R286 KFileMetaData BRANCH master REVISION DETAIL https://phabricator.kde.org/D9880 To: asturmlechner,

Re: A new tier1 for icontheme.rcc handling

2018-01-21 Thread Matthieu Gallien
ideally someone who > steps up for the once in a lifetime chance to become > maintainer of a glorious new tier1 framework. Thanks for the work on this. If nobody steps up, I can volunteer to maintain it if people are OK with this. The workload should be low ? I depend on this code for a good windows support. This is important for me. > Cheers, > > Hannah Best regards -- Matthieu Gallien

Re: A new tier1 for icontheme.rcc handling

2018-01-30 Thread Matthieu Gallien
Hello Hannah, On mercredi 24 janvier 2018 10:32:17 CET Hannah von Reth wrote: > Hi Matthieu, > > On 21/01/2018 21:19, Matthieu Gallien wrote: > > Hello, > > > > On jeudi 18 janvier 2018 08:45:51 CET Hannah von Reth wrote: > >> Hi everyone. > >

D10203: documenturldb: Temporarily remove Q_ASSERT, ignore empty filenames instead

2018-02-01 Thread Matthieu Gallien
mgallien added a comment. I do not reproduce your issue. At the same time I have wiped out my baloo db a lot of time so I may just have one that is correct given it is young. REPOSITORY R293 Baloo REVISION DETAIL https://phabricator.kde.org/D10203 To: michaelh, dfaure, vhanda,

D4911: add Baloo DBus signals for moved or removed files

2018-02-01 Thread Matthieu Gallien
mgallien added a comment. Ping REPOSITORY R293 Baloo REVISION DETAIL https://phabricator.kde.org/D4911 To: mgallien, vhanda, dfaure Cc: ngraham, cullmann, apol, #frameworks, michaelh

D10203: documenturldb: Temporarily remove Q_ASSERT, ignore empty filenames instead

2018-02-04 Thread Matthieu Gallien
mgallien added a comment. In https://phabricator.kde.org/D10203#200490, @michaelh wrote: > @mgallien: Here is way to come close to reproducing this without corrupting you db: > > $balooctl stop > > > - Take 2 pendrives A and B > - Plug them in in this order: A > B Assuming

D10365: New icon for Elisa music player

2018-02-10 Thread Matthieu Gallien
mgallien accepted this revision. mgallien added a comment. This revision is now accepted and ready to land. Thanks for your work. I am hesitant about the choice of an audio tape. I fear people under 30 will not recognize it. I would like to propose trying the tape with the current color and

D10365: New icon for Elisa music player

2018-02-14 Thread Matthieu Gallien
mgallien added a comment. In D10365#205750 , @andreask wrote: > can we get it now to master? > > @paullesur is hopefully interested in other icon's too. @paullesur do you have a KDE contributor account ? If not, I cannot push due to

D10694: epubextractor: Handle multiple subjects better

2018-02-20 Thread Matthieu Gallien
mgallien added a comment. The problem with any changes to the extractors its that the baloo database of users will not be refreshed. REPOSITORY R286 KFileMetaData REVISION DETAIL https://phabricator.kde.org/D10694 To: michaelh, mgallien, dfaure Cc: #frameworks, ashaposhnikov, michaelh,

D10365: New icon for Elisa music player

2018-02-22 Thread Matthieu Gallien
mgallien added a comment. In D10365#206602 , @mgallien wrote: > In D10365#205750 , @andreask wrote: > > > can we get it now to master? > > > > @paullesur is hopefully interested in other icon's

D10694: epubextractor: Handle multiple subjects better

2018-02-20 Thread Matthieu Gallien
mgallien added a comment. In D10694#210409 , @michaelh wrote: > @mgallien Then they won't benefit from this until a file is reindexed. > Users with baloo disabled will benefit, because in that case `baloo_filemetadata_temp_extractor` will

D10694: epubextractor: Handle multiple subjects better

2018-02-26 Thread Matthieu Gallien
mgallien added a comment. I have created T8079 to work on baloo database update when extractors are modified and returned different data. REPOSITORY R286 KFileMetaData REVISION DETAIL https://phabricator.kde.org/D10694 To: michaelh, mgallien, dfaure

D10803: handle more tags in taglibextractor

2018-02-26 Thread Matthieu Gallien
mgallien added a comment. I have created T8079 to work on baloo database update when extractors are modified and returned different data. REPOSITORY R286 KFileMetaData REVISION DETAIL https://phabricator.kde.org/D10803 To: astippich, mgallien Cc:

D13630: automatic tests: do not embed EmbeddedImageData already in the library

2018-06-20 Thread Matthieu Gallien
This revision was automatically updated to reflect the committed changes. Closed by commit R286:aa90123a8c18: automatic tests: do not embed EmbeddedImageData already in the library (authored by mgallien). REPOSITORY R286 KFileMetaData CHANGES SINCE LAST UPDATE

D13630: automatic tests: do not embed EmbeddedImageData already in the library

2018-06-20 Thread Matthieu Gallien
mgallien created this revision. mgallien added reviewers: dfaure, bcooksley, bruns, astippich. Restricted Application added projects: Frameworks, Baloo. Restricted Application added subscribers: Baloo, kde-frameworks-devel. mgallien requested review of this revision. REVISION SUMMARY should fix

D12320: add ability to read embedded cover files

2018-06-20 Thread Matthieu Gallien
mgallien added a comment. D13630 should fix the Windows build. Sorry for not noticing this before. I was mostly away from keyboard for the last two weeks after having been sick. REPOSITORY R286 KFileMetaData REVISION DETAIL

D14131: Add enum alias Property::Language for typo Property::Langauge

2018-08-11 Thread Matthieu Gallien
mgallien accepted this revision. mgallien added a comment. This revision is now accepted and ready to land. Thanks for your work and good idea. REPOSITORY R286 KFileMetaData BRANCH fixLangaugeEnumTypoNow REVISION DETAIL https://phabricator.kde.org/D14131 To: kossebau, mgallien Cc:

D13906: add missing include

2018-08-23 Thread Matthieu Gallien
mgallien accepted this revision. This revision is now accepted and ready to land. REPOSITORY R286 KFileMetaData BRANCH add_include REVISION DETAIL https://phabricator.kde.org/D13906 To: astippich, mgallien Cc: kde-frameworks-devel, #baloo, ashaposhnikov, michaelh, astippich, spoorun,

D13700: implement reading of the replaygain tags

2018-08-29 Thread Matthieu Gallien
mgallien requested changes to this revision. mgallien added a comment. This revision now requires changes to proceed. Some inline comments INLINE COMMENTS > taglibextractor.cpp:254-263 > +if(!strcmp( userTextFrame->description().toCString( true ), > "replaygain_track_gain" ) ) {

D15013: balootctl: fix 396535

2018-08-23 Thread Matthieu Gallien
mgallien added a comment. In D15013#313880 , @jausmus wrote: > In D15013#313867 , @anthonyfieroni wrote: > > > Do not use QDir::separator > > > > if (!folder.endsWith(QLatin1Char('/')) { > >

D14131: Add enum alias Property::Language for typo Property::Langauge

2018-07-18 Thread Matthieu Gallien
mgallien added a comment. In D14131#294528 , @aacid wrote: > Looks good to me. If @mgallien doesn't give you an "accept" in a reasonable timeframe i guess you can count this as me accepting it ;) I am in holidays for 3 weeks and unable

D13700: implement reading of the replaygain tags

2018-09-08 Thread Matthieu Gallien
mgallien accepted this revision. This revision is now accepted and ready to land. REPOSITORY R286 KFileMetaData BRANCH replaygain REVISION DETAIL https://phabricator.kde.org/D13700 To: astippich, mgallien, bruns Cc: kde-frameworks-devel, #baloo, ashaposhnikov, michaelh, astippich,

D13700: implement reading of the replaygain tags

2018-09-08 Thread Matthieu Gallien
mgallien added a comment. Thanks for your hard work. This is a really nice addition to audio tags. I no longer have objections. Please finish to take into account feedback from @bruns. REPOSITORY R286 KFileMetaData REVISION DETAIL https://phabricator.kde.org/D13700 To: astippich,

D13885: taglibextractor: Restore extracting audio props without tags existing

2018-07-05 Thread Matthieu Gallien
mgallien accepted this revision. mgallien added a comment. This revision is now accepted and ready to land. @kossebau thanks for your work on this patch @astippich thanks for your quick reaction and the work on this patch REPOSITORY R286 KFileMetaData BRANCH fix_empty_tags REVISION

D13914: Avoid compiler warnings for taglib headers

2018-07-06 Thread Matthieu Gallien
mgallien accepted this revision. mgallien added a comment. This revision is now accepted and ready to land. Thanks REPOSITORY R286 KFileMetaData BRANCH avoidtaglibheaderwarnings REVISION DETAIL https://phabricator.kde.org/D13914 To: kossebau, mgallien Cc: kde-frameworks-devel,

D12950: add test which checks the property types

2018-07-12 Thread Matthieu Gallien
mgallien added a comment. Sorry for the delay on my side. Why are you adding this test ? REPOSITORY R286 KFileMetaData REVISION DETAIL https://phabricator.kde.org/D12950 To: astippich, mgallien, bruns Cc: kde-frameworks-devel, #baloo, ashaposhnikov, michaelh, astippich, spoorun,

D10918: taglibextractor: Refactor for better readability

2018-02-28 Thread Matthieu Gallien
mgallien added a comment. Thanks for your work. I would prefer to wait for D10803 to land before doing any big changes like that. REPOSITORY R286 KFileMetaData REVISION DETAIL https://phabricator.kde.org/D10918 To: michaelh, mgallien, #baloo,

D10694: epubextractor: Handle multiple subjects better

2018-02-27 Thread Matthieu Gallien
mgallien requested changes to this revision. mgallien added a comment. This revision now requires changes to proceed. In D10694#214996 , @michaelh wrote: > In D10694#213712 , @mgallien wrote: > > >

D10803: handle more tags in taglibextractor

2018-02-27 Thread Matthieu Gallien
mgallien added a comment. In D10803#214655 , @astippich wrote: > In D10803#213783 , @michaelh wrote: > > > In D10803#213767 , @astippich wrote: > > >

D10694: epubextractor: Handle multiple subjects better

2018-03-13 Thread Matthieu Gallien
mgallien added a comment. In D10694#222674 , @michaelh wrote: > @mgallien: I think we have a beautiful misunderstanding here :-) > > In D10694#221719 , @michaelh wrote: > > > This is bad! ...

D10694: epubextractor: Handle multiple subjects better

2018-03-13 Thread Matthieu Gallien
mgallien added a comment. In D10694#224465 , @michaelh wrote: > In D10694#224440 , @mgallien wrote: > > > Could you please update your diff and we can land it ? This is a useful improvement. >

D10694: epubextractor: Handle multiple subjects better

2018-03-08 Thread Matthieu Gallien
mgallien added a comment. Sorry for being so late. In D10694#215663 , @michaelh wrote: > @mgallien : taglibextractor.cpp is very hard to read. Hopefully it is no longer. INLINE COMMENTS > michaelh wrote in epubextractor.cpp:94 > Did you

D10365: New icon for Elisa music player

2018-03-08 Thread Matthieu Gallien
mgallien added a comment. I believe there was no objections to this choice of icon. Can we ensure this gets landed soon ? I would prefer to be able to have the new icon as soon as possible. REPOSITORY R266 Breeze Icons BRANCH master REVISION DETAIL https://phabricator.kde.org/D10365

D10694: epubextractor: Handle multiple subjects better

2018-03-09 Thread Matthieu Gallien
mgallien added a comment. In D10694#221719 , @michaelh wrote: > This is bad! I have learned baloo itself doesn't handle stringlists. Which in my view would be the natural way to handle token-like items like tags, keywords and subject(s). Until

D12028: taglibextractortest: Add test for files with empty metadata

2018-04-11 Thread Matthieu Gallien
mgallien added a comment. I need more time. I will try to look at it today. By the way, the stack concept seems very useful. Thanks REPOSITORY R286 KFileMetaData REVISION DETAIL https://phabricator.kde.org/D12028 To: michaelh, mgallien, #baloo, #frameworks Cc: astippich, ashaposhnikov,

D12029: taglibextractor: Fix empty genre bug

2018-04-11 Thread Matthieu Gallien
mgallien accepted this revision. mgallien added a comment. This revision is now accepted and ready to land. Sorry for the delay. Thanks REPOSITORY R286 KFileMetaData BRANCH nometa-pass (branched from master) REVISION DETAIL https://phabricator.kde.org/D12029 To: michaelh, mgallien,

D10803: handle more tags in taglibextractor

2018-04-11 Thread Matthieu Gallien
mgallien added a comment. In D10803#244389 , @astippich wrote: > ping. Can I push? I'm asking again because I don't want to mess up anything before the frameworks release, and this one contains string changes I am not sure. I tried

D12028: taglibextractortest: Add test for files with empty metadata

2018-04-11 Thread Matthieu Gallien
mgallien requested changes to this revision. mgallien added a comment. This revision now requires changes to proceed. Thanks for this work. Fix the issues and we should be good to go. INLINE COMMENTS > taglibextractortest.cpp:173 > +const auto testpasses = QString(); > +QString

D12145: autotests: Do not use QScopedPointer for plugins

2018-04-12 Thread Matthieu Gallien
mgallien added a comment. In D12145#245023 , @alexeymin wrote: > Just curious, why is this needed? To avoid dynamic memory allocations? I asked on new code to avoid using an extractor allocated on heap instead of on stack. @michaelh

D12028: taglibextractortest: Add test for files with empty metadata

2018-04-12 Thread Matthieu Gallien
mgallien accepted this revision. mgallien added a comment. This revision is now accepted and ready to land. Thanks REPOSITORY R286 KFileMetaData BRANCH nometa (branched from master) REVISION DETAIL https://phabricator.kde.org/D12028 To: michaelh, mgallien, #baloo, #frameworks Cc:

D12145: autotests: Do not use QScopedPointer for plugins

2018-04-12 Thread Matthieu Gallien
mgallien accepted this revision. mgallien added a comment. This revision is now accepted and ready to land. Thanks for having done it everywhere. I had missed that "detail". REPOSITORY R286 KFileMetaData BRANCH plugin-simple (branched from master) REVISION DETAIL

D12156: implement reading of rating tag

2018-04-18 Thread Matthieu Gallien
mgallien added a comment. Thanks. Fix one issue and we should be good to go. Another nice thing is that we should have a portable way to have ratings in Elisa that could be read or write somewhere else. INLINE COMMENTS > taglibextractor.cpp:222-234 > +if (temp == 0) { > +

D12156: implement reading of rating tag

2018-04-18 Thread Matthieu Gallien
mgallien requested changes to this revision. This revision now requires changes to proceed. REPOSITORY R286 KFileMetaData REVISION DETAIL https://phabricator.kde.org/D12156 To: astippich, mgallien, michaelh Cc: #frameworks, ashaposhnikov, michaelh, astippich, spoorun, bruns

D12108: ffmpegextractor: Silence deprecation warnings

2018-04-18 Thread Matthieu Gallien
mgallien accepted this revision. mgallien added a comment. This revision is now accepted and ready to land. Thanks. I did try to fix it some months ago but did not finished however I noticed that there is no automatic tests for it. Do you have time to work on some ? REPOSITORY R286

D12197: autotests: Test for multiple values

2018-04-18 Thread Matthieu Gallien
mgallien accepted this revision. mgallien added a comment. This revision is now accepted and ready to land. Thanks REPOSITORY R286 KFileMetaData BRANCH multi-value-test REVISION DETAIL https://phabricator.kde.org/D12197 To: michaelh, #baloo, #frameworks, mgallien Cc: bruns,

<    1   2   3   >