D16671: Refactor embedded image extractor for greater extensibility

2018-12-06 Thread Alexander Stippich
This revision was automatically updated to reflect the committed changes. Closed by commit R286:7929a896b5e1: Refactor embedded image extractor for greater extensibility (authored by astippich). CHANGED PRIOR TO COMMIT https://phabricator.kde.org/D16671?vs=46880=46982#toc REPOSITORY R286

D16671: Refactor embedded image extractor for greater extensibility

2018-12-05 Thread Stefan Brüns
bruns accepted this revision. bruns added inline comments. This revision is now accepted and ready to land. INLINE COMMENTS > embeddedimagedata.cpp:207 > +int dataPosition = pictureData.find('\0'); > +if (dataPosition == -1) { > +return QByteArray(); You can check

D16671: Refactor embedded image extractor for greater extensibility

2018-12-04 Thread Alexander Stippich
astippich marked 2 inline comments as done. astippich added inline comments. INLINE COMMENTS > bruns wrote in embeddedimagedata.cpp:134 > Does Taglib signal an error when it fails to parse the file? Or is calling > pictureList() always safe? Docs don't say, so I fed a musepack file into this

D16671: Refactor embedded image extractor for greater extensibility

2018-12-04 Thread Alexander Stippich
astippich updated this revision to Diff 46880. astippich added a comment. - new lines, handle dataPosition == -1 REPOSITORY R286 KFileMetaData CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D16671?vs=46860=46880 BRANCH refactor_embedded_image REVISION DETAIL

D16671: Refactor embedded image extractor for greater extensibility

2018-12-04 Thread Stefan Brüns
bruns added inline comments. INLINE COMMENTS > embeddedimagedata.cpp:181 > } > +QByteArray > +EmbeddedImageData::Private::getFrontCoverFromMp4(TagLib::MP4::Tag* mp4Tags) > const Nitpick - add an empty line in-between functions REPOSITORY R286 KFileMetaData REVISION DETAIL

D16671: Refactor embedded image extractor for greater extensibility

2018-12-04 Thread Stefan Brüns
bruns added inline comments. INLINE COMMENTS > embeddedimagedata.cpp:134 > TagLib::FLAC::File flacFile(, > TagLib::ID3v2::FrameFactory::instance(), true); > -TagLib::List lstPic = > flacFile.pictureList(); > - > -if (!lstPic.isEmpty()) { > -for

D16671: Refactor embedded image extractor for greater extensibility

2018-12-04 Thread Alexander Stippich
astippich added a dependent revision: D17357: extend list of supported mimetypes for embedded image extractor. REPOSITORY R286 KFileMetaData REVISION DETAIL https://phabricator.kde.org/D16671 To: astippich, mgallien, bruns Cc: kde-frameworks-devel, #baloo, ashaposhnikov, michaelh,

D16671: Refactor embedded image extractor for greater extensibility

2018-12-04 Thread Alexander Stippich
astippich updated this revision to Diff 46860. astippich added a comment. - rebase REPOSITORY R286 KFileMetaData CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D16671?vs=44982=46860 BRANCH refactor_embedded_image REVISION DETAIL https://phabricator.kde.org/D16671 AFFECTED

D16671: Refactor embedded image extractor for greater extensibility

2018-12-02 Thread Alexander Stippich
astippich added a comment. anyone? I would like to extend the support to the same file types as supported by the taglibextractor REPOSITORY R286 KFileMetaData REVISION DETAIL https://phabricator.kde.org/D16671 To: astippich, mgallien, bruns Cc: kde-frameworks-devel, #baloo,

D16671: Refactor embedded image extractor for greater extensibility

2018-11-24 Thread Alexander Stippich
astippich added a comment. ping REPOSITORY R286 KFileMetaData REVISION DETAIL https://phabricator.kde.org/D16671 To: astippich, mgallien, bruns Cc: kde-frameworks-devel, #baloo, ashaposhnikov, michaelh, astippich, spoorun, ngraham, bruns, abrahams

D16671: Refactor embedded image extractor for greater extensibility

2018-11-10 Thread Alexander Stippich
astippich added inline comments. INLINE COMMENTS > bruns wrote in embeddedimagedata.cpp:204 > I know you have only moved this code here ... > > `TagLib::ByteVector::find` returns `-1` on not found, and we add 1, so this > is safe here. But maybe we should return `QByteArray()` instead? The

D16671: Refactor embedded image extractor for greater extensibility

2018-11-08 Thread Stefan Brüns
bruns added inline comments. INLINE COMMENTS > embeddedimagedata.cpp:204 > +TagLib::ByteVector pictureData = (*itApe).second.binaryData(); > +int dataPosition = pictureData.find('\0') + 1; > +return QByteArray(pictureData.data() + dataPosition, > pictureData.size() -

D16671: Refactor embedded image extractor for greater extensibility

2018-11-06 Thread Alexander Stippich
astippich updated this revision to Diff 44982. astippich added a comment. - more cleanup REPOSITORY R286 KFileMetaData CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D16671?vs=44868=44982 BRANCH refactor_embedded_image REVISION DETAIL https://phabricator.kde.org/D16671

D16671: Refactor embedded image extractor for greater extensibility

2018-11-04 Thread Alexander Stippich
astippich created this revision. astippich added reviewers: mgallien, bruns. Herald added projects: Frameworks, Baloo. Herald added subscribers: Baloo, kde-frameworks-devel. astippich requested review of this revision. REVISION SUMMARY refactor the embedded image data extractor so that