This is used only when adding the album (insertAlbum function). It is not added to the tracks in the database, so I believe this change is correct. This is also done the same way a few lines above when checking for albums already in the database. I will change the title to make that clear.

In D20526#449780, @bruns wrote:

In D20526#449560, @astippich wrote:

IMHO for searching, the separation from a single string into multiple entries is the best thing to do, from a metadata editing point of view not so much.
How about inserting the TStringList entry by entry, without using the contactsFromString method? That would maintain the exact representation of metadata tags also for the baloo database.

When searching is left to baloo, it already splits up strings at whitespace etc., so you can provide the string as is.

Elisa is barely usable for me without this as most of my files apparently do not have an album artist tag.
Please have a look if I correctly changed all the tests, especially the addTwoAlbumsWithDifferentPathsAndSameTracks test

• fix typo, check entry count

IMHO for searching, the separation from a single string into multiple entries is the best thing to do, from a metadata editing point of view not so much.
How about inserting the TStringList entry by entry, without using the contactsFromString method? That would maintain the exact representation of metadata tags also for the baloo database.

There is an icon in the git repo:

@bruns this still applies cleanly to current master, do you think this and the dependent revision are still worth it to merge?

• use localized strings

And I propose a different one :)
IMHO bundling these two is sub-optimal as it creates an unnecessary dependency on kde-specific libraries and limits its deployment. To me, this is the equivalent of bundling the audio thumbnailer with taglib.

• add comment

It creates a dependency loop as you pointed out in the first comment

• remove unrequired parenthesis

In D17500#447049, @aacid wrote:

In D17500#446937, @astippich wrote:

The plan I was about to propose is to move the thumbnailer to kdegraphics-thumbnailers and remove it from the QMobipocket library, lifting the KIO dependency of QMobipocket. I think a KIO dependency for such a library is inconvenient.

qmobipocket only depends on Qt::Core and Qt::Gui.

It is a documentation bug.

The plan I was about to propose is to move the thumbnailer to kdegraphics-thumbnailers and remove it from the QMobipocket library, lifting the KIO dependency of QMobipocket. I think a KIO dependency for such a library is inconvenient.

• rebase and update
• update test
• disable mobi extractor

• remove superfluous check

https://bugreports.qt.io/browse/QTBUG-75069

windows build fails
https://build.kde.org/job/Frameworks/job/kfilemetadata/job/kf5-qt5%20WindowsMSVCQt5.11/143/console

ping

There already was a bug report some time ago
https://bugreports.qt.io/browse/QTBUG-15488

• use enum

@bruns this diff tries to accomplish the same as D19268, and can be closed, right?

only one minor nitpick

FYI, writing rating information was added to KF 5.57 via the TagLib writer plugin

ping

Still kind of hard to read, especially on phabricator. Anyway, a definite improvement.
Untested on anything besides linux

works nicely!

• rebase on master

I think @bruns is much more qualified than me as a maintainer as I lack overall experience in programming. He still finds wrong stuff in almost any revision of mine :)
also +1 for baloo

• add friend and use member function

• cleanup and formatting

This causes one test failure on FreeBSD:
https://build.kde.org/job/Frameworks/job/kfilemetadata/job/kf5-qt5%20FreeBSDQt5.12/51/

Oh, another idea: Cannot this be used to determine that the extractor supports the inheriting mimetype as required in D20121? Would need to be public for this, though, or the test needs to be made a friend class.

Forget my comment, I thought it would be possible to share some code with the ExtractorCollection, but the implementation is too different

• rebase
• simplify id3 rating to array

• rebase

• check that file is valid
• rebase

ping. I would like to land this for KF 5.57 to fix crashes in the taglib extractors because of invalid files once and for all.

ping. This will be beneficial for handling lists regardless of the discussion in D19087

Shouldn't this go into MimeUtils?

My main concern is that this changes current behavior and as such is not backwards compatible. The rest is a bonus.
That is why my plan was to make the extractors output (string-)lists as outlined in T8196 (this task has been there for quite a while, feedback would have been nice).
This way, current behavior can be preserved while also fixing all bugs with multi-value properties.
If changing this behavior is okay, then you should at least post patches that adapt all clients. I know of Baloo-widgets, Dolphin and Elisa.
But there will be combinations of frameworks and applications that will not work correctly for multi-value properties.
What do others think of this? @mgallien?

After some thinking, I have split up the function for writing specific Ape and Vorbis tags, as many more tags like "Opus" etc. are in use for Vorbis tags and not for Ape, and can be easily added this way. I am still using the property map, though.
Are you fine with this?

• split ape and vorbis tag writing functions