D10803: handle more tags in taglibextractor

This revision was automatically updated to reflect the committed changes. Closed by commit R286:7f9de32eedff: handle more tags in taglibextractor (authored by astippich). CHANGED PRIOR TO COMMIT https://phabricator.kde.org/D10803?vs=30956=31950#toc REPOSITORY R286 KFileMetaData CHANGES

D10803: handle more tags in taglibextractor

dfaure added a comment. No. As soon as the RC1 tags are there, the message freeze is lifted. In case of a bugfix that comes in after RC1, I cherry-pick it into the release branch, so that master doesn't have to stay frozen for a week. REPOSITORY R286 KFileMetaData BRANCH

D10803: handle more tags in taglibextractor

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

D10803: handle more tags in taglibextractor

astippich added a comment. 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 REPOSITORY R286 KFileMetaData BRANCH enhance_taglib REVISION DETAIL https://phabricator.kde.org/D10803 To:

D10803: handle more tags in taglibextractor

bruns added inline comments. INLINE COMMENTS > astippich wrote in taglibextractortest.cpp:82 > Hmm, I disagree since they are simple and they don't leave room for ambiguity Maybe just add a Prefix or Suffix for all QStrings? Same applies to any Int values, you can never tell if the right

D10803: handle more tags in taglibextractor

astippich added a comment. @mgallien this should be safe to land now since KF 5.45 has been tagged, right? REPOSITORY R286 KFileMetaData BRANCH enhance_taglib REVISION DETAIL https://phabricator.kde.org/D10803 To: astippich, mgallien, ngraham, michaelh Cc: vhanda, dfaure, michaelh,

D10803: handle more tags in taglibextractor

astippich edited the summary of this revision. REPOSITORY R286 KFileMetaData BRANCH enhance_taglib REVISION DETAIL https://phabricator.kde.org/D10803 To: astippich, mgallien, ngraham, michaelh Cc: vhanda, dfaure, michaelh, ngraham, #frameworks, ashaposhnikov, astippich, spoorun, bruns,

D10803: handle more tags in taglibextractor

michaelh accepted this revision. michaelh added a comment. This revision is now accepted and ready to land. Thank you. REPOSITORY R286 KFileMetaData BRANCH enhance_taglib REVISION DETAIL https://phabricator.kde.org/D10803 To: astippich, mgallien, ngraham, michaelh Cc: vhanda, dfaure,

D10803: handle more tags in taglibextractor

astippich updated this revision to Diff 30956. astippich added a comment. - remove duplicate genre property for mp3 REPOSITORY R286 KFileMetaData CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D10803?vs=30955=30956 BRANCH enhance_taglib REVISION DETAIL

D10803: handle more tags in taglibextractor

astippich updated this revision to Diff 30955. astippich marked an inline comment as done. astippich added a comment. - fix formatting issues during rebase REPOSITORY R286 KFileMetaData CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D10803?vs=30951=30955 BRANCH enhance_taglib

D10803: handle more tags in taglibextractor

astippich marked 3 inline comments as done. astippich added a comment. In D10803#237133 , @michaelh wrote: > Except for the white space stuff I'm still fine with this. Oops, sorry, messed that up during rebase. INLINE COMMENTS >

D10803: handle more tags in taglibextractor

michaelh requested changes to this revision. michaelh added a comment. This revision now requires changes to proceed. Except for the white space stuff I'm still fine with this. INLINE COMMENTS > taglibextractor.cpp:237 > data.discNumber = itDiscNumber->second.toInt(); > +}

D10803: handle more tags in taglibextractor

astippich added a comment. Okay, I changed my mind, I rebased on top of master. Since the other revision needs more time and the new properties behave like the old ones, I'd like to end this long journey and land this revision if no-one objects :) REPOSITORY R286 KFileMetaData BRANCH

D10803: handle more tags in taglibextractor

astippich updated this revision to Diff 30951. astippich added a comment. - update value types REPOSITORY R286 KFileMetaData CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D10803?vs=30950=30951 BRANCH enhance_taglib REVISION DETAIL https://phabricator.kde.org/D10803

D10803: handle more tags in taglibextractor

astippich updated this revision to Diff 30950. astippich added a comment. - rebase on top of master REPOSITORY R286 KFileMetaData CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D10803?vs=28467=30950 BRANCH enhance_taglib REVISION DETAIL https://phabricator.kde.org/D10803

D10803: handle more tags in taglibextractor

astippich added a comment. All of a sudden :) I've been quiet about this because I'd like to resolve the issues regarding strings/stringlists first, so if you have some spare time, please have a look at D11365 . I will rebase after this one landed

D10803: handle more tags in taglibextractor

ngraham accepted this revision. ngraham added a comment. Thanks for the heads-up, Matthieu. Looks good and works well for me too! REPOSITORY R286 KFileMetaData BRANCH enhance_taglib REVISION DETAIL https://phabricator.kde.org/D10803 To: astippich, mgallien, ngraham, michaelh Cc:

D10803: handle more tags in taglibextractor

michaelh accepted this revision. michaelh added a comment. Tooltips cope well REPOSITORY R286 KFileMetaData BRANCH enhance_taglib REVISION DETAIL https://phabricator.kde.org/D10803 To: astippich, mgallien, ngraham, michaelh Cc: vhanda, dfaure, michaelh, ngraham, #frameworks,

D10803: handle more tags in taglibextractor

michaelh added a reviewer: michaelh. REPOSITORY R286 KFileMetaData BRANCH enhance_taglib REVISION DETAIL https://phabricator.kde.org/D10803 To: astippich, mgallien, ngraham, michaelh Cc: vhanda, dfaure, michaelh, ngraham, #frameworks, ashaposhnikov, astippich, spoorun, nicolasfella,

D10803: handle more tags in taglibextractor

mgallien accepted this revision. mgallien added a comment. This revision is now accepted and ready to land. I am really sorry. I had missed that you had modified the diff to only include the safe set of changes. I should really apologize. Thanks for your work, it is very much appreciated.

D10803: handle more tags in taglibextractor

michaelh added a comment. In D10694#222086 , @astippich wrote: > Also, since this discussion also applies for the taglibextractor: Is a string list preferred for new properties in KFileMetadata when multiple entries are possible? I think right

D10803: handle more tags in taglibextractor

michaelh added a comment. I played around a bit with your previous diff (lyrics included). And modified some `test.ogg` and `test.mp3`. Lyrics is not metadata, but if we include them they will be searchable: $ baloosearch lyrics:carefree ~/samplefiles/test.ogg

D10803: handle more tags in taglibextractor

astippich updated this revision to Diff 28467. astippich added a comment. - remove lyrics and map encodedby to generator REPOSITORY R286 KFileMetaData CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D10803?vs=28300=28467 BRANCH enhance_taglib REVISION DETAIL

D10803: handle more tags in taglibextractor

ngraham added a comment. Phew, what a conversation! For the issue of the rating, perhaps we could sync the tag metadata with the baloo metadata, if the baloo rating metadata is already empty. I might even go so far as to overwrite the baloo rating metadata with the tag rating metadata;

D10803: handle more tags in taglibextractor

michaelh added inline comments. INLINE COMMENTS > taglibextractortest.cpp:82 > > QCOMPARE(resultFlac.properties().value(Property::Title), > QVariant(QStringLiteral("Title"))); > QCOMPARE(resultFlac.properties().value(Property::Artist), > QVariant(QStringLiteral("Artist"))); Just a

D10803: handle more tags in taglibextractor

michaelh added a comment. In D10803#215270 , @mgallien wrote: > This is not necessary. You could use a QImage or a QPixmap. You can put them in a QVariant and they should be converted to an empty QString. Please consider writing a plugin

D10803: handle more tags in taglibextractor

mgallien added a comment. In D10803#214655 , @astippich wrote: > In D10803#213783 , @michaelh wrote: > > > In D10803#213767 , @astippich wrote: > > >

D10803: handle more tags in taglibextractor

michaelh added a comment. A thumbnail extractor might be better suited for this job. On my system(tumbleweed) there is a `kde-audio-thumbnail.` I don't know anything more about it. Maybe `ffmpeg-thumbnails` can do that? REPOSITORY R286 KFileMetaData REVISION DETAIL

D10803: handle more tags in taglibextractor

astippich planned changes to this revision. astippich added a comment. In D10803#213783 , @michaelh wrote: > In D10803#213767 , @astippich wrote: > > > I don't know dolphin works, but given how

D10803: handle more tags in taglibextractor

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:

D10803: handle more tags in taglibextractor

michaelh added a comment. In D10803#213767 , @astippich wrote: > I don't know dolphin works, but given how KFileMetadata is designed, the new tags should be ignored until explicit support is added in dolphin. Please try that. For

D10803: handle more tags in taglibextractor

mgallien added a subscriber: dfaure. mgallien added inline comments. INLINE COMMENTS > astippich wrote in properties.h:168 > I noticed the typo as well, but I think this is a matter of API > compatibility, so I left it unchanged. Matthieu should know. It is part of the source compatibility

D10803: handle more tags in taglibextractor

astippich added a comment. I don't know dolphin works, but given how KFileMetadata is designed, the new tags should be ignored until explicit support is added in dolphin. INLINE COMMENTS > michaelh wrote in properties.h:164 > ? Oops, seems like a wrong copy & paste :) will fix > michaelh

D10803: handle more tags in taglibextractor

michaelh added a comment. Nice! How do the infopanel or tooltips of dolphin look with this? INLINE COMMENTS > properties.h:164 > /** > - * The language the document is written in. This directly maps to the > + * The language the document is written in. Thiof a media file.s >

D10803: handle more tags in taglibextractor

astippich added a comment. First of all, please review carefully for ABI compatibility, I do not know what to watch for that. So I decided to walk into the minefield that tags are. Everyone implements it differently. I've implemented some tags that seem to be supported across a wide

D10803: handle more tags in taglibextractor

astippich created this revision. astippich added a reviewer: mgallien. Restricted Application added a project: Frameworks. Restricted Application added a subscriber: Frameworks. astippich requested review of this revision. REVISION SUMMARY adds the ability to read more tags through taglib when