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&id=46982#toc
REPOSITORY
R286
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 for
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 co
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&id=46880
BRANCH
refactor_embedded_image
REVISION DETAIL
https://phabri
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
https://phabric
bruns added inline comments.
INLINE COMMENTS
> embeddedimagedata.cpp:134
> TagLib::FLAC::File flacFile(&stream,
> TagLib::ID3v2::FrameFactory::instance(), true);
> -TagLib::List lstPic =
> flacFile.pictureList();
> -
> -if (!lstPic.isEmpty()) {
> -for (TagLi
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, astippich
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&id=46860
BRANCH
refactor_embedded_image
REVISION DETAIL
https://phabricator.kde.org/D16671
AFFECTED
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, ashaposhniko
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
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 cod
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() - data
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&id=44982
BRANCH
refactor_embedded_image
REVISION DETAIL
https://phabricator.kde.org/D16671
AF
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
support
14 matches
Mail list logo
