astippich added a comment.
In D19109#414968 <https://phabricator.kde.org/D19109#414968>, @bruns wrote: > In D19109#414758 <https://phabricator.kde.org/D19109#414758>, @astippich wrote: > > > A few general remarks: > > > > - I really do not like that there are two lists of supported mimetypes now which have to be kept in sync > > > I think this is trivial enough. Also this is covered by the unit test. My fear is that it is easily forgotten, but I did not see the autotest. Still, do you think it is feasible to generate the mimetype stringlist from the JSON data to remove the duplication? >> - Do we really need versioning per mimetype? IMHO it is sufficient to have a version number per extractor. From my experience, fixing an extractor usually impacts all its supported mimetypes, and rarily affects only one mimetype. > > Past experience tells otherwise. There have been feature extensions and bugfixes for specific mimetypes, just look at your own commits > > - "fix ape disc number extraction" > - "implement more tags for asf metadata" > - ... > > I want to reduce reindexing as much as possible. And I can give you examples where this was not the case :). This is also only the case because TagLibExtractor was stupidly written (which D18826 <https://phabricator.kde.org/D18826> fixes). The other extractors do not have that many special codepath. Well, I find it cumbersome to implement this fine-grained control, but otherwise people will probably yell because of high cpu usage... At least, I would like to group duplicated mimetypes such as audio/wav and audio/x-wav, but that is not possible with JSON, is it? >> Also, this makes the list hard to maintain, also regarding file types which have multiple mime types, e.g. audio/wav and audio/x-wav >> >> - Do we need an x.y version? I think a single integer is enough or what do you have in mind? > > Changes only affecting failed files are minor versions, changes affecting already indexed files (i.e. support for new properties) get a new major version. > >> - I prefer to directly construct the qvariantmap in the extractors, and re-use the mimetype list which is already available. > > Requires changing the plugin interface. Does not allow to query extractor properties without fully loading the plugin (which is expensive). Read https://vizzzion.org/blog/2013/08/ "K_PLUGIN_FACTORY_WITH_JSON or where is the metadata?" Thanks. REPOSITORY R286 KFileMetaData REVISION DETAIL https://phabricator.kde.org/D19109 To: bruns, #baloo, #frameworks, ngraham, astippich, poboiko Cc: kde-frameworks-devel, ashaposhnikov, michaelh, astippich, spoorun, ngraham, bruns, abrahams