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
