Differential D16671 Diff 46982 src/embeddedimagedata.cpp

Changeset View

Standalone View

View Options

src/embeddedimagedata.cpp

Show All 39 Lines
40 40​
41using namespace KFileMetaData; 41​using namespace KFileMetaData;
42 42​
43class Q_DECL_HIDDEN EmbeddedImageData::Private 43​class Q_DECL_HIDDEN EmbeddedImageData::Private
44{ 44​{
45public: 45​public:
46 QMimeDatabase mMimeDatabase; 46​ QMimeDatabase mMimeDatabase;
47 QByteArray getFrontCover(const QString &fileUrl, const QString &mimeType) const; 47​ QByteArray getFrontCover(const QString &fileUrl, const QString &mimeType) const;
48​ QByteArray getFrontCoverFromID3(TagLib::ID3v2::Tag* Id3Tags) const;
49​ QByteArray getFrontCoverFromFlacPicture(TagLib::List<TagLib::FLAC::Picture *> lstPic) const;
50​ QByteArray getFrontCoverFromMp4(TagLib::MP4::Tag* mp4Tags) const;
51​ QByteArray getFrontCoverFromApe(TagLib::APE::Tag* apeTags) const;
48 static const QStringList mMimetypes; 52​ static const QStringList mMimetypes;
49}; 53​};
50 54​
51const QStringList EmbeddedImageData::Private::mMimetypes = 55​const QStringList EmbeddedImageData::Private::mMimetypes =
52{ 56​{
53 QStringLiteral("audio/flac"), 57​ QStringLiteral("audio/flac"),
54 QStringLiteral("audio/mp4"), 58​ QStringLiteral("audio/mp4"),
55 QStringLiteral("audio/mpeg"), 59​ QStringLiteral("audio/mpeg"),
▲ Show 20 Lines • Show All 45 Lines • ▼ Show 20 Line(s)103 if (!stream.isOpen()) {
101 return QByteArray(); 105​ return QByteArray();
102 } 106​ }
103 if ((mimeType == QLatin1String("audio/mpeg")) 107​ if ((mimeType == QLatin1String("audio/mpeg"))
104 || (mimeType == QLatin1String("audio/mpeg3")) 108​ || (mimeType == QLatin1String("audio/mpeg3"))
105 || (mimeType == QLatin1String("audio/x-mpeg"))) { 109​ || (mimeType == QLatin1String("audio/x-mpeg"))) {
106 110​
107 // Handling multiple tags in mpeg files. 111​ // Handling multiple tags in mpeg files.
108 TagLib::MPEG::File mpegFile(&stream, TagLib::ID3v2::FrameFactory::instance(), true); 112​ TagLib::MPEG::File mpegFile(&stream, TagLib::ID3v2::FrameFactory::instance(), true);
109 if (!mpegFile.ID3v2Tag() || mpegFile.ID3v2Tag()->isEmpty()) { 113​ if (mpegFile.ID3v2Tag()) {
110 return QByteArray(); 114​ return getFrontCoverFromID3(mpegFile.ID3v2Tag());
111 } ​
112 ​
113 TagLib::ID3v2::FrameList lstID3v2; ​
114 // Attached Front Picture. ​
115 lstID3v2 = mpegFile.ID3v2Tag()->frameListMap()["APIC"]; ​
116 if (!lstID3v2.isEmpty()) { ​
117 for (TagLib::ID3v2::FrameList::ConstIterator it = lstID3v2.begin(); it != lstID3v2.end(); ++it) { ​
118 auto *frontCoverFrame = static_cast<TagLib::ID3v2::AttachedPictureFrame *>(*it); ​
119 if (frontCoverFrame->type() == frontCoverFrame->FrontCover) { ​
120 return QByteArray(frontCoverFrame->picture().data(), frontCoverFrame->picture().size()); ​
121 } ​
122 } ​
123 } 115​ }
124 116​
125 } else if (mimeType == QLatin1String("audio/mp4")) { 117​ } else if (mimeType == QLatin1String("audio/mp4")) {
126 118​
127 TagLib::MP4::File mp4File(&stream, true); 119​ TagLib::MP4::File mp4File(&stream, true);
128 if (!mp4File.tag() || mp4File.tag()->isEmpty()) { 120​ if (mp4File.tag()) {
129 return QByteArray(); 121​ return getFrontCoverFromMp4(mp4File.tag());
130 } ​
131 TagLib::MP4::Item coverArtItem = mp4File.tag()->item("covr"); ​
132 if (coverArtItem.isValid()) ​
133 { ​
134 TagLib::MP4::CoverArtList coverArtList = coverArtItem.toCoverArtList(); ​
135 TagLib::MP4::CoverArt& frontCover = coverArtList.front(); ​
136 return QByteArray(frontCover.data().data(), frontCover.data().size()); ​
137 } 122​ }
138 123​
139 } else if (mimeType == QLatin1String("audio/x-musepack")) { 124​ } else if (mimeType == QLatin1String("audio/x-musepack")) {
140 125​
141 TagLib::MPC::File mpcFile(&stream, true); 126​ TagLib::MPC::File mpcFile(&stream, true);
142 if (!mpcFile.tag() || mpcFile.tag()->isEmpty()) { 127​ if (mpcFile.APETag()) {
143 return QByteArray(); 128​ return getFrontCoverFromApe(mpcFile.APETag());
144 } ​
145 ​
146 TagLib::APE::ItemListMap lstMusepack = mpcFile.APETag()->itemListMap(); ​
147 TagLib::APE::ItemListMap::ConstIterator itMPC; ​
148 ​
149 /* The cover art tag for APEv2 tags starts with the filename as string ​
150 * with zero termination followed by the actual picture data */ ​
151 itMPC = lstMusepack.find("COVER ART (FRONT)"); ​
152 if (itMPC != lstMusepack.end()) { ​
153 TagLib::ByteVector pictureData = (*itMPC).second.binaryData(); ​
154 int dataPosition = pictureData.find('\0') + 1; ​
155 return QByteArray(pictureData.data() + dataPosition, pictureData.size() - dataPosition); ​
156 } 129​ }
157 130​
158 } else if (mimeType == QLatin1String("audio/flac")) { 131​ } else if (mimeType == QLatin1String("audio/flac")) {
159 132​
160 TagLib::FLAC::File flacFile(&stream, TagLib::ID3v2::FrameFactory::instance(), true); 133​ TagLib::FLAC::File flacFile(&stream, TagLib::ID3v2::FrameFactory::instance(), true);
161 TagLib::List<TagLib::FLAC::Picture *> lstPic = flacFile.pictureList(); 134​ return getFrontCoverFromFlacPicture(flacFile.pictureList());
162 135​
163 if (!lstPic.isEmpty()) { 136​ } else if ((mimeType == QLatin1String("audio/ogg"))
164 for (TagLib::List<TagLib::FLAC::Picture *>::Iterator it = lstPic.begin(); it != lstPic.end(); ++it) { ​
165 TagLib::FLAC::Picture *picture = *it; ​
166 if (picture->type() == picture->FrontCover) { ​
167 return QByteArray(picture->data().data(), picture->data().size()); ​
168 } ​
169 } ​
170 } ​
171 ​
172 } else { ​
173 ​
174 TagLib::List<TagLib::FLAC::Picture *> lstPic; ​
175 if ((mimeType == QLatin1String("audio/ogg")) ​
176 || (mimeType == QLatin1String("audio/x-vorbis+ogg"))) { 137​ || (mimeType == QLatin1String("audio/x-vorbis+ogg"))) {
138​
177 TagLib::Ogg::Vorbis::File oggFile(&stream, true); 139​ TagLib::Ogg::Vorbis::File oggFile(&stream, true);
178 if (oggFile.tag() && !oggFile.tag()->isEmpty()) { 140​ if (oggFile.tag()) {
179 lstPic = oggFile.tag()->pictureList(); 141​ return getFrontCoverFromFlacPicture(oggFile.tag()->pictureList());
180 } 142​ }
181 } 143​ }
182 if ((mimeType == QLatin1String("audio/opus")) 144​ else if ((mimeType == QLatin1String("audio/opus"))
183 || (mimeType == QLatin1String("audio/x-opus+ogg"))) { 145​ || (mimeType == QLatin1String("audio/x-opus+ogg"))) {
brunsUnsubmitted
Not Done

Does Taglib signal an error when it fails to parse the file? Or is calling pictureList() always safe?

bruns: Does Taglib signal an error when it fails to parse the file? Or is calling pictureList() always…
astippichAuthorUnsubmitted
Not Done

Docs don't say, so I fed a musepack file into this codepath and it still ran without issues (no cover extracted of course). Good enough?

astippich: Docs don't say, so I fed a musepack file into this codepath and it still ran without issues (no…
brunsUnsubmitted
Not Done

Yes, thanks.

bruns: Yes, thanks.
146​
184 TagLib::Ogg::Opus::File opusFile(&stream, true); 147​ TagLib::Ogg::Opus::File opusFile(&stream, true);
185 if (opusFile.tag() && !opusFile.tag()->isEmpty()) { 148​ if (opusFile.tag()) {
186 lstPic = opusFile.tag()->pictureList(); 149​ return getFrontCoverFromFlacPicture(opusFile.tag()->pictureList());
150​ }
187 } 151​ }
152​ return QByteArray();
188 } 153​}
189 if (!lstPic.isEmpty()) { 154​
190 for (TagLib::List<TagLib::FLAC::Picture *>::Iterator it = lstPic.begin(); it != lstPic.end(); ++it) { 155​QByteArray
191 TagLib::FLAC::Picture *picture = *it; 156​EmbeddedImageData::Private::getFrontCoverFromID3(TagLib::ID3v2::Tag* Id3Tags) const
brunsUnsubmitted
Not Done

Not sure if this is an issue, but the old code only called tag()->pictureList() after !tag()->isEmpty() - is this safe without?
Dito for oggFile above.

bruns: Not sure if this is an issue, but the old code only called `tag()->pictureList()` after `!tag()…
astippichAuthorUnsubmitted
Not Done

Yes, the list will be empty.

astippich: Yes, the list will be empty.
157​{
158​ TagLib::ID3v2::FrameList lstID3v2;
159​ // Attached Front Picture.
160​ lstID3v2 = Id3Tags->frameListMap()["APIC"];
161​ for (const auto& frame : qAsConst(lstID3v2))
162​ {
163​ const auto *frontCoverFrame = static_cast<TagLib::ID3v2::AttachedPictureFrame *>(frame);
164​ if (frontCoverFrame->type() == frontCoverFrame->FrontCover) {
165​ return QByteArray(frontCoverFrame->picture().data(), frontCoverFrame->picture().size());
166​ }
167​ }
168​ return QByteArray();
169​}
170​
171​QByteArray
172​EmbeddedImageData::Private::getFrontCoverFromFlacPicture(TagLib::List<TagLib::FLAC::Picture *> lstPic) const
173​{
174​ for (const auto &picture : qAsConst(lstPic)) {
192 if (picture->type() == picture->FrontCover) { 175​ if (picture->type() == picture->FrontCover) {
193 return QByteArray(picture->data().data(), picture->data().size()); 176​ return QByteArray(picture->data().data(), picture->data().size());
194 } 177​ }
195 } 178​ }
179​ return QByteArray();
180​}
181​
182​QByteArray
183​EmbeddedImageData::Private::getFrontCoverFromMp4(TagLib::MP4::Tag* mp4Tags) const
184​{
185​ TagLib::MP4::Item coverArtItem = mp4Tags->item("covr");
186​ if (coverArtItem.isValid())
187​ {
188​ TagLib::MP4::CoverArtList coverArtList = coverArtItem.toCoverArtList();
189​ TagLib::MP4::CoverArt& frontCover = coverArtList.front();
190​ return QByteArray(frontCover.data().data(), frontCover.data().size());
191​ }
192​ return QByteArray();
193​}
194​
195​QByteArray
196​EmbeddedImageData::Private::getFrontCoverFromApe(TagLib::APE::Tag* apeTags) const
197​{
198​ TagLib::APE::ItemListMap lstApe = apeTags->itemListMap();
199​ TagLib::APE::ItemListMap::ConstIterator itApe;
200​
201​ /* The cover art tag for APEv2 tags starts with the filename as string
202​ * with zero termination followed by the actual picture data */
203​ itApe = lstApe.find("COVER ART (FRONT)");
204​ if (itApe != lstApe.end()) {
205​ TagLib::ByteVector pictureData = (*itApe).second.binaryData();
206​ int position = pictureData.find('\0');
207​ if (position >= 0) {
brunsUnsubmitted
Not Done

You can check for >= 0 and let the -1 case fall through ...

bruns: You can check for `>= 0` and let the `-1` case fall through ...
208​ position += 1;
209​ return QByteArray(pictureData.data() + position, pictureData.size() - position);
196 } 210​ }
197 } 211​ }
198 return QByteArray(); 212​ return QByteArray();
199} 213​}
brunsUnsubmitted
Not Done

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?

bruns: I know you have only moved this code here ... `TagLib::ByteVector::find` returns `-1` on not…
astippichAuthorUnsubmitted
Not Done

The code also works if only the picture data is there without the name.
I could return an empty QByteArray if dataPosition == -1 and pictureData.size == 0.
Do you prefer if I do that separately?

astippich: The code also works if only the picture data is there without the name. I could return an empty…
brunsUnsubmitted
Done

Is it safe to call find on an empty ByteVector - most likely yes ...
Then return QByteArray on dataPosition == -1, so it is obvious the error case is handled, and keep the rest as is.

bruns: Is it safe to call find on an empty ByteVector - most likely yes ... Then return QByteArray on…
brunsUnsubmitted
Done

Nitpick - add an empty line in-between functions

bruns: Nitpick - add an empty line in-between functions