D12320: [RFC] add ability to read embedded cover files

[Stefan Brüns]

bruns added a comment.


  as the remaining issues are formatting only, this is an `accept` by me save the requested changes.
  @mgallien - if i am late to accept, can you do it?

INLINE COMMENTS

> embeddedimagedata.cpp:59
> +
> +QMap<EmbeddedImageData::ImageType, QByteArray> EmbeddedImageData::imageData(const QString &fileUrl, const EmbeddedImageData::ImageTypes types) const
> +{

nitpick - excessively long line

  QMap<EmbeddedImageData::ImageType, QByteArray>
  EmbeddedImageData::imageData(const QString &fileUrl, 
                               const EmbeddedImageData::ImageTypes types) const
  {

> embeddedimagedata.cpp:73
> +
> +QByteArray EmbeddedImageData::Private::getFrontCover(const QString &fileUrl, const QString &mimeType) const
> +{

dito, long line

> embeddedimagedata.cpp:95
> +            for (TagLib::ID3v2::FrameList::ConstIterator it = lstID3v2.begin(); it != lstID3v2.end(); ++it) {
> +                TagLib::ID3v2::AttachedPictureFrame *frontCoverFrame = static_cast<TagLib::ID3v2::AttachedPictureFrame *>(*it);
> +                if (frontCoverFrame->type() == frontCoverFrame->FrontCover) {

long line, just make it `auto`, the type is obvious from the `static_cast<T>`

> embeddedimagedata.h:35
> + * \brief The EmbeddedImageData is a class which extracts the images
> + * stored in the metadata tags of files as byte arrays.
> + *

Can you add one or two more lines what format to expect inside the byte array?

> embeddedimagedata.h:51
> +     * Extracts the images stored in the metadata tags from a file.
> +     * By default, the front covers are extracted.
> +     */

nitpick - singular, front cover

REPOSITORY
  R286 KFileMetaData

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

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