bruns requested changes to this revision.
bruns added inline comments.
This revision now requires changes to proceed.
INLINE COMMENTS
> embeddedimagedata.cpp:69
> +
> + if (types == EmbeddedImageData::FrontCover) {
> +
> imageData.insert(EmbeddedImageData::FrontCover,d->getFrontCover(fileUrl,mimeType));
should be `types & EmbeddedImageData::FrontCover`
> embeddedimagedata.cpp:149
> + if (pictureFlac->type() == pictureFlac->FrontCover) {
> + return
> QByteArray(pictureFlac->data().data(),pictureFlac->data().size());
> + }
missing space after `,`
> embeddedimagedata.cpp:170
> + // Attached Picture.
> + if (!lstPic.isEmpty()) {
> + for (TagLib::List<TagLib::FLAC::Picture *>::Iterator it =
> lstPic.begin(); it != lstPic.end(); ++it) {
This is identical to the "audio/flac" case, can you use the same variable names
etc. in both places?
> astippich wrote in embeddedimagedata.h:40
> std::make_unique is C++14, right? According to
> https://community.kde.org/Frameworks/Policies#Frameworks_Qt_requirements
> C++11 is the currently minimum version, so I left it unchanged. This is also
> similar to the other classes in KFileMetaData
`std::make_unique` is just a convenience helper
`auto foo = std::make_unique<Foo>` is exactly the same as `auto foo =
std::unique_ptr<Foo>(new Foo());`
see http://en.cppreference.com/w/cpp/memory/unique_ptr/make_unique
REPOSITORY
R286 KFileMetaData
REVISION DETAIL
https://phabricator.kde.org/D12320
To: astippich, mgallien, michaelh, bruns
Cc: kde-frameworks-devel, #baloo, bruns, ashaposhnikov, michaelh, astippich,
spoorun, ngraham
