Skip to content

Commit

Permalink
Merge pull request #2507 from uklotzde/coverimagehash
Browse files Browse the repository at this point in the history
Cover Image Hash: Refactoring
  • Loading branch information
daschuer authored Feb 21, 2020
2 parents 030e792 + 4f5b064 commit 618cb62
Show file tree
Hide file tree
Showing 7 changed files with 104 additions and 70 deletions.
73 changes: 70 additions & 3 deletions src/library/coverart.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -3,9 +3,12 @@
#include "library/coverart.h"
#include "library/coverartutils.h"
#include "util/debug.h"
#include "util/logger.h"

namespace {

mixxx::Logger kLogger("CoverArt");

QString sourceToString(CoverInfo::Source source) {
switch (source) {
case CoverInfo::UNKNOWN:
Expand Down Expand Up @@ -43,12 +46,26 @@ QString coverInfoToString(const CoverInfo& info) {
}
} // anonymous namespace

//static
quint16 CoverImageUtils::calculateHash(
const QImage& image) {
const auto hash = qChecksum(
reinterpret_cast<const char*>(image.constBits()),
#if QT_VERSION >= QT_VERSION_CHECK(5, 10, 0)
image.sizeInBytes()
#else
image.byteCount()
#endif
);
DEBUG_ASSERT(image.isNull() || isValidHash(hash));
DEBUG_ASSERT(!image.isNull() || hash == defaultHash());
return hash;
}

CoverInfoRelative::CoverInfoRelative()
: source(UNKNOWN),
type(NONE),
hash(0) {
// The default hash value should match the calculated hash for a null image
DEBUG_ASSERT(CoverArtUtils::calculateHash(QImage()) == hash);
hash(CoverImageUtils::defaultHash()) {
}

bool operator==(const CoverInfoRelative& a, const CoverInfoRelative& b) {
Expand All @@ -67,6 +84,56 @@ QDebug operator<<(QDebug dbg, const CoverInfoRelative& infoRelative) {
.arg(coverInfoRelativeToString(infoRelative));
}

QImage CoverInfo::loadImage(
const SecurityTokenPointer& pTrackLocationToken) const {
if (type == CoverInfo::METADATA) {
VERIFY_OR_DEBUG_ASSERT(!trackLocation.isEmpty()) {
kLogger.warning()
<< "loadImage"
<< type
<< "cover with empty trackLocation";
return QImage();
}
return CoverArtUtils::extractEmbeddedCover(
TrackFile(trackLocation),
pTrackLocationToken);
} else if (type == CoverInfo::FILE) {
VERIFY_OR_DEBUG_ASSERT(!trackLocation.isEmpty()) {
kLogger.warning()
<< "loadImage"
<< type
<< "cover with empty trackLocation."
<< "Relative paths will not work.";
SecurityTokenPointer pToken =
Sandbox::openSecurityToken(
QFileInfo(coverLocation),
true);
return QImage(coverLocation);
}

const QFileInfo coverFile(
TrackFile(trackLocation).directory(),
coverLocation);
const QString coverFilePath = coverFile.filePath();
if (!coverFile.exists()) {
kLogger.warning()
<< "loadImage"
<< type
<< "cover does not exist:"
<< coverFilePath;
return QImage();
}
SecurityTokenPointer pToken =
Sandbox::openSecurityToken(coverFile, true);
return QImage(coverFilePath);
} else if (type == CoverInfo::NONE) {
return QImage();
} else {
DEBUG_ASSERT(!"unhandled CoverInfo::Type");
return QImage();
}
}

bool operator==(const CoverInfo& a, const CoverInfo& b) {
return static_cast<const CoverInfoRelative&>(a) ==
static_cast<const CoverInfoRelative&>(b) &&
Expand Down
31 changes: 23 additions & 8 deletions src/library/coverart.h
Original file line number Diff line number Diff line change
@@ -1,11 +1,25 @@
#ifndef COVERART_H
#define COVERART_H
#pragma once

#include <QImage>
#include <QString>
#include <QObject>
#include <QtDebug>
#include <QtGlobal>

#include "util/sandbox.h"

class CoverImageUtils {
public:
static quint16 calculateHash(
const QImage& image);

static constexpr quint16 defaultHash() {
return 0;
}

static constexpr bool isValidHash(
quint16 hash) {
return hash != defaultHash();
}
};

class CoverInfoRelative {
public:
Expand Down Expand Up @@ -64,6 +78,9 @@ class CoverInfo : public CoverInfoRelative {
CoverInfo(CoverInfo&&) = default;
CoverInfo& operator=(CoverInfo&&) = default;

QImage loadImage(
const SecurityTokenPointer& pTrackLocationToken = SecurityTokenPointer()) const;

QString trackLocation;
};

Expand All @@ -89,12 +106,10 @@ class CoverArt : public CoverInfo {
CoverArt(CoverArt&&) = default;
CoverArt& operator=(CoverArt&&) = default;

// it is not a QPixmap, because it is not safe to use pixmaps
// it is not a QPixmap, because it is not safe to use pixmaps
// outside the GUI thread
QImage image;
QImage image;
int resizedToWidth;
};

QDebug operator<<(QDebug dbg, const CoverArt& art);

#endif /* COVERART_H */
2 changes: 1 addition & 1 deletion src/library/coverartcache.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -133,7 +133,7 @@ CoverArtCache::FutureResult CoverArtCache::loadCover(
<< info << desiredWidth << signalWhenDone;
}

QImage image = CoverArtUtils::loadCover(info);
QImage image = info.loadImage();

// TODO(XXX) Should we re-hash here? If the cover file (or track metadata)
// has changed then info.hash may be incorrect. The fix
Expand Down
43 changes: 3 additions & 40 deletions src/library/coverartutils.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -42,43 +42,6 @@ QImage CoverArtUtils::extractEmbeddedCover(
std::move(trackFile), std::move(pToken));
}

//static
QImage CoverArtUtils::loadCover(const CoverInfo& info) {
if (info.type == CoverInfo::METADATA) {
if (info.trackLocation.isEmpty()) {
qDebug() << "CoverArtUtils::loadCover METADATA cover with empty trackLocation.";
return QImage();
}
const TrackFile trackFile(info.trackLocation);
return extractEmbeddedCover(trackFile);
} else if (info.type == CoverInfo::FILE) {
if (info.trackLocation.isEmpty()) {
qDebug() << "CoverArtUtils::loadCover FILE cover with empty trackLocation."
<< "Relative paths will not work.";
SecurityTokenPointer pToken = Sandbox::openSecurityToken(
QFileInfo(info.coverLocation), true);
return QImage(info.coverLocation);
}

QFileInfo coverFile(TrackFile(info.trackLocation).directory(), info.coverLocation);
QString coverFilePath = coverFile.filePath();
if (!coverFile.exists()) {
qDebug() << "CoverArtUtils::loadCover FILE cover does not exist:"
<< coverFilePath;
return QImage();
}
SecurityTokenPointer pToken = Sandbox::openSecurityToken(
coverFile, true);
return QImage(coverFilePath);
} else if (info.type == CoverInfo::NONE) {
return QImage();
} else {
qDebug() << "CoverArtUtils::loadCover unhandled type";
DEBUG_ASSERT(false);
return QImage();
}
}

//static
QList<QFileInfo> CoverArtUtils::findPossibleCoversInFolder(const QString& folder) {
// Search for image files in the track directory.
Expand Down Expand Up @@ -117,7 +80,7 @@ CoverInfoRelative CoverArtUtils::selectCoverArtForTrack(
const QList<QFileInfo>& covers) {
CoverInfoRelative coverInfoRelative;
DEBUG_ASSERT(coverInfoRelative.type == CoverInfo::NONE);
DEBUG_ASSERT(coverInfoRelative.hash == 0);
DEBUG_ASSERT(!CoverImageUtils::isValidHash(coverInfoRelative.hash));
DEBUG_ASSERT(coverInfoRelative.coverLocation.isNull());
coverInfoRelative.source = CoverInfo::GUESSED;
if (covers.isEmpty()) {
Expand Down Expand Up @@ -181,7 +144,7 @@ CoverInfoRelative CoverArtUtils::selectCoverArtForTrack(
if (!image.isNull()) {
coverInfoRelative.type = CoverInfo::FILE;
// TODO() here we may introduce a duplicate hash code
coverInfoRelative.hash = calculateHash(image);
coverInfoRelative.hash = CoverImageUtils::calculateHash(image);
coverInfoRelative.coverLocation = bestInfo->fileName();
}
}
Expand All @@ -197,7 +160,7 @@ CoverInfoRelative CoverInfoGuesser::guessCoverInfo(
CoverInfoRelative coverInfo;
coverInfo.source = CoverInfo::GUESSED;
coverInfo.type = CoverInfo::METADATA;
coverInfo.hash = CoverArtUtils::calculateHash(embeddedCover);
coverInfo.hash = CoverImageUtils::calculateHash(embeddedCover);
DEBUG_ASSERT(coverInfo.coverLocation.isNull());
return coverInfo;
}
Expand Down
11 changes: 0 additions & 11 deletions src/library/coverartutils.h
Original file line number Diff line number Diff line change
Expand Up @@ -26,17 +26,6 @@ class CoverArtUtils {
TrackFile trackFile,
SecurityTokenPointer pToken);

static QImage loadCover(const CoverInfo& info);
static quint16 calculateHash(const QImage& image) {
return qChecksum(reinterpret_cast<const char*>(image.constBits()),
#if QT_VERSION >= QT_VERSION_CHECK(5, 10, 0)
image.sizeInBytes()
#else
image.byteCount()
#endif
);
}

static QStringList supportedCoverArtExtensions();
static QString supportedCoverArtExtensionsRegex();

Expand Down
12 changes: 6 additions & 6 deletions src/test/coverartutils_test.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -142,7 +142,7 @@ TEST_F(CoverArtUtilTest, searchImage) {
// stuff below.

result.trackLocation = kTrackLocationTest;
const QImage img = CoverArtUtils::loadCover(result);
const QImage img = result.loadImage();
EXPECT_EQ(img.isNull(), false);

QString trackBaseName = "cover-test";
Expand Down Expand Up @@ -172,7 +172,7 @@ TEST_F(CoverArtUtilTest, searchImage) {

// looking for cover in an directory with one image will select that one.
expected2.coverLocation = "foo.jpg";
expected2.hash = CoverArtUtils::calculateHash(QImage(cLoc_foo));
expected2.hash = CoverImageUtils::calculateHash(QImage(cLoc_foo));
covers << QFileInfo(cLoc_foo);
CoverInfoRelative res2 = CoverArtUtils::selectCoverArtForTrack(
trackBaseName, trackAlbum, covers);
Expand Down Expand Up @@ -244,7 +244,7 @@ TEST_F(CoverArtUtilTest, searchImage) {
} else {
expected2.type = CoverInfo::FILE;
expected2.coverLocation = cover.fileName();
expected2.hash = CoverArtUtils::calculateHash(QImage(cover.filePath()));
expected2.hash = CoverImageUtils::calculateHash(QImage(cover.filePath()));
}
res2 = CoverArtUtils::selectCoverArtForTrack(trackBaseName, trackAlbum,
prefCovers);
Expand Down Expand Up @@ -272,7 +272,7 @@ TEST_F(CoverArtUtilTest, searchImage) {

res2 = CoverArtUtils::selectCoverArtForTrack(trackBaseName, trackAlbum,
prefCovers);
expected2.hash = CoverArtUtils::calculateHash(QImage(cLoc_coverJPG));
expected2.hash = CoverImageUtils::calculateHash(QImage(cLoc_coverJPG));
expected2.coverLocation = "cover.JPG";
EXPECT_EQ(expected2, res2);

Expand All @@ -289,7 +289,7 @@ TEST_F(CoverArtUtilTest, searchImage) {
prefCovers.append(QFileInfo(cLoc_albumName));
res2 = CoverArtUtils::selectCoverArtForTrack(trackBaseName, trackAlbum,
prefCovers);
expected2.hash = CoverArtUtils::calculateHash(QImage(cLoc_albumName));
expected2.hash = CoverImageUtils::calculateHash(QImage(cLoc_albumName));
expected2.coverLocation = trackAlbum % ".jpg";
EXPECT_EQ(expected2, res2);

Expand All @@ -300,7 +300,7 @@ TEST_F(CoverArtUtilTest, searchImage) {
res2 = CoverArtUtils::selectCoverArtForTrack(trackBaseName, trackAlbum,
prefCovers);

expected2.hash = CoverArtUtils::calculateHash(QImage(cLoc_filename));
expected2.hash = CoverImageUtils::calculateHash(QImage(cLoc_filename));
expected2.coverLocation = trackBaseName % ".jpg";
EXPECT_EQ(expected2, res2);

Expand Down
2 changes: 1 addition & 1 deletion src/widget/wcoverartmenu.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,7 @@ void WCoverArtMenu::slotChange() {
coverInfo.source = CoverInfo::USER_SELECTED;
coverInfo.coverLocation = selectedCoverPath;
// TODO() here we may introduce a duplicate hash code
coverInfo.hash = CoverArtUtils::calculateHash(image);
coverInfo.hash = CoverImageUtils::calculateHash(image);
qDebug() << "WCoverArtMenu::slotChange emit" << coverInfo;
emit coverInfoSelected(coverInfo);
}
Expand Down

0 comments on commit 618cb62

Please sign in to comment.