D13700: implement reading of the replaygain tags

Stefan BrĂ¼ns noreply at phabricator.kde.org
Sun Sep 2 19:26:09 BST 2018


bruns added inline comments.

INLINE COMMENTS

> taglibextractor.cpp:50
>  {
>      return QString::fromWCharArray((const wchar_t*)t.toCWString(), t.length());
>  }

This is definitely broken:
http://taglib.org/api/classTagLib_1_1String.html#a0ef8ad270d710863e0bb1c1b18cdb95d

`const wchar_t* TagLib::String::toCWString 	() 	const`

> Returns a standard C-style (null-terminated) wide character version of this String. The returned string is encoded in UTF-16 (without BOM/CPU byte order), not UTF-32 even if wchar_t is 32-bit wide.

https://github.com/qt/qtbase/blob/c5307203f5c0b0e588cc93e70764c090dd4c2ce0/src/corelib/tools/qstring.h#L1027

  inline QString QString::fromWCharArray(const wchar_t *string, int size)
  {
      return sizeof(wchar_t) == sizeof(QChar) ? fromUtf16(reinterpret_cast<const ushort *>(string), size)
                                              : fromUcs4(reinterpret_cast<const uint *>(string), size);
  }

Fortunately, platforms with wchar_t == wchar32_t are uncommon ...

> taglibextractor.cpp:249
> +
> +    // User Text Frame (TXXX) for ReplayGain
> +    lstID3v2 = mpegFile.ID3v2Tag()->frameListMap()["TXXX"];

Maybe better `// Check if there are any User Text Identification Frames (TXXX) at all`
Technically, the `UserTextIdentificationFrame::find(...)` below would be sufficient to lookup the tags.

> taglibextractor.cpp:255
> +        auto trackGainFrame = IdFrame::find(mpegFile.ID3v2Tag(), "replaygain_track_gain");
> +        if (trackGainFrame && !trackGainFrame->fieldList().isEmpty() ) {
> +            data.replayGainTrackGain = convertWCharsToQString(trackGainFrame->fieldList().back());

Stray space in `if (...isEmpty() )`

REPOSITORY
  R286 KFileMetaData

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

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/20180902/5f53ccfe/attachment.html>


More information about the Kde-frameworks-devel mailing list