D16283: implement more tags for asf metadata

Stefan BrĂ¼ns noreply at phabricator.kde.org
Wed Oct 17 22:42:36 BST 2018


bruns added inline comments.

INLINE COMMENTS

> taglibextractor.cpp:720
> +        //map the rating values of WMP to Baloo rating
> +        //0->0, 1->2, 25->4, 50->6,7 5->8, 99->10
> +        if (rating == 0) {

`// 0->0, 1->2, 25->4, 50->6, 75->8, 99->10`

> taglibextractor.cpp:736
> +    lstASF = asfTags->attribute("WM/AlbumArtist");
> +    if (!lstASF.isEmpty()) {
> +        for (TagLib::ASF::AttributeList::ConstIterator it = lstASF.begin(); it != lstASF.end(); ++it) {

you don't need the `isEmpty()` here, as you iterate over it.

> taglibextractor.cpp:737
> +    if (!lstASF.isEmpty()) {
> +        for (TagLib::ASF::AttributeList::ConstIterator it = lstASF.begin(); it != lstASF.end(); ++it) {
> +            if (!data.albumArtists.isEmpty()) {

I think you can write instead:
`for (const auto& attribute : qAsConst(lstAsf)) { ... }`,
as `TagLib::List<T>` has `begin()` and `end()`

> taglibextractor.cpp:739
> +            if (!data.albumArtists.isEmpty()) {
> +                data.albumArtists += ", ";
> +            }

this makes me always wonder why we don't do:

  // decltype(data.albumArtists) == QStringList
  for (attribute : lstAsf) {
    QString t = TStringToQString(attribute.toString());
    data.albumArtists << contactsFromString(t);
  }

> taglibextractor.h:28
>  #include <tfilestream.h>
>  
>  namespace TagLib

Side note - I think `tfilestream.h` is no longer needed.

REPOSITORY
  R286 KFileMetaData

REVISION DETAIL
  https://phabricator.kde.org/D16283

To: astippich, mgallien, bruns
Cc: kde-frameworks-devel, #baloo, ashaposhnikov, michaelh, astippich, spoorun, ngraham, bruns, abrahams
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-frameworks-devel/attachments/20181017/1c5dec85/attachment.html>


More information about the Kde-frameworks-devel mailing list