Skip to content

Commit

Permalink
Merge remote-tracking branch 'upstream/2.5' into 2.5
Browse files Browse the repository at this point in the history
  • Loading branch information
Swiftb0y committed Aug 15, 2024
2 parents 3640f90 + f21fa88 commit fe043ac
Show file tree
Hide file tree
Showing 11 changed files with 291 additions and 120 deletions.
1 change: 1 addition & 0 deletions CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -2165,6 +2165,7 @@ add_executable(mixxx-test
src/test/trackdao_test.cpp
src/test/trackexport_test.cpp
src/test/trackmetadata_test.cpp
src/test/trackmetadataexport_test.cpp
src/test/tracknumberstest.cpp
src/test/trackreftest.cpp
src/test/trackupdate_test.cpp
Expand Down
14 changes: 8 additions & 6 deletions src/library/dao/trackdao.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -621,7 +621,7 @@ void bindTrackLibraryValues(
QString keysVersion = keys.getVersion();
QString keysSubVersion = keys.getSubVersion();
mixxx::track::io::key::ChromaticKey key = keys.getGlobalKey();
QString keyText = KeyUtils::formatGlobalKey(keys);
QString keyText = keys.getGlobalKeyText();
pTrackLibraryQuery->bindValue(":keys", keysBlob);
pTrackLibraryQuery->bindValue(":keys_version", keysVersion);
pTrackLibraryQuery->bindValue(":keys_sub_version", keysSubVersion);
Expand Down Expand Up @@ -1251,16 +1251,18 @@ void setTrackKey(const QSqlRecord& record, const int column, Track* pTrack) {
QString keysVersion = record.value(column + 1).toString();
QString keysSubVersion = record.value(column + 2).toString();
QByteArray keysBlob = record.value(column + 3).toByteArray();
Keys keys = KeyFactory::loadKeysFromByteArray(
keysVersion, keysSubVersion, &keysBlob);

if (!keysVersion.isEmpty()) {
pTrack->setKeys(keys);
pTrack->setKeys(KeyFactory::loadKeysFromByteArray(
keysVersion,
keysSubVersion,
&keysBlob));
} else if (!keyText.isEmpty()) {
// Typically this happens if we are upgrading from an older (<1.12.0)
// version of Mixxx that didn't support Keys. We treat all legacy data
// as user-generated because that way it will be treated sensitively.
pTrack->setKeyText(keyText, mixxx::track::io::key::USER);
pTrack->setKeys(KeyFactory::makeBasicKeysKeepText(
keyText,
mixxx::track::io::key::USER));
}
}

Expand Down
3 changes: 3 additions & 0 deletions src/sources/soundsourceproxy.h
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
#pragma once

#include <gtest/gtest_prod.h>

#include <QMimeType>

#include "sources/soundsourceproviderregistry.h"
Expand Down Expand Up @@ -201,6 +203,7 @@ class SoundSourceProxy {
static QHash<QMimeType, QString> s_fileTypeByMimeType;

friend class TrackCollectionManager;
FRIEND_TEST(TrackMetadataExportTest, keepWithespaceKey);
static ExportTrackMetadataResult exportTrackMetadataBeforeSaving(
Track* pTrack,
const SyncTrackMetadataParams& syncParams);
Expand Down
138 changes: 138 additions & 0 deletions src/test/trackmetadataexport_test.cpp
Original file line number Diff line number Diff line change
@@ -0,0 +1,138 @@
#include <gtest/gtest.h>

#include <QTemporaryDir>

#include "test/mixxxtest.h"
#include "test/soundsourceproviderregistration.h"
#include "track/globaltrackcache.h"
#include "track/track.h"

namespace {

const QString kEmptyFile = QStringLiteral("empty.mp3");

void deleteTrack(Track* pTrack) {
// Delete track objects directly in unit tests with
// no main event loop
delete pTrack;
};

class GlobalTrackCacheHelper : public GlobalTrackCacheSaver {
public:
void saveEvictedTrack(Track* pTrack) noexcept override {
ASSERT_FALSE(pTrack == nullptr);
}
GlobalTrackCacheHelper() {
GlobalTrackCache::createInstance(this, deleteTrack);
}
~GlobalTrackCacheHelper() override {
GlobalTrackCache::destroyInstance();
}
};

} // namespace

class TrackMetadataExportTest : public MixxxTest, private SoundSourceProviderRegistration {
public:
TrackMetadataExportTest()
: m_testDataDir(QDir::current().absoluteFilePath(
"src/test/id3-test-data")) {
}

protected:
const QDir m_testDataDir;
QTemporaryDir m_exportTempDir;
GlobalTrackCacheHelper m_globalTrackCacheHelper;
};

TEST_F(TrackMetadataExportTest, keepWithespaceKey) {
const QString kWhiteSpacesKey = QStringLiteral(" A#m ");
const QString kNormalizedDisplayKey = QString::fromUtf8("B♭m");
constexpr std::string_view kId3Key = "Bbm";

// Generate a file name for exporting metadata
const QString exportTrackPath = m_exportTempDir.filePath(kEmptyFile);
mixxxtest::copyFile(m_testDataDir.absoluteFilePath(kEmptyFile), exportTrackPath);
TrackPointer pTrack = Track::newTemporary(exportTrackPath);

mixxx::TrackMetadata writeTrackMetadata;
writeTrackMetadata.refTrackInfo().setKeyText(kWhiteSpacesKey);

// the internal value is still unchanged
EXPECT_EQ(writeTrackMetadata.getTrackInfo().getKeyText().toStdString(),
kWhiteSpacesKey.toStdString());

// This saves the metadata object literally, but normalizes
// the global key value as StandardID3v2
pTrack->replaceMetadataFromSource(
std::move(writeTrackMetadata),
QDateTime::currentDateTimeUtc());

// getKeytext returns the normalized version suitable for GUI presentation
EXPECT_EQ(pTrack->getKeyText().toStdString(), kNormalizedDisplayKey.toStdString());

// the internal value is still unchanged
EXPECT_EQ(pTrack->getRecord()
.getMetadata()
.getTrackInfo()
.getKeyText()
.toStdString(),
kWhiteSpacesKey.toStdString());

pTrack->markForMetadataExport();
SyncTrackMetadataParams params;
ExportTrackMetadataResult result =
SoundSourceProxy::exportTrackMetadataBeforeSaving(
pTrack.get(), params);
EXPECT_EQ(result, ExportTrackMetadataResult::Succeeded);

// recreate the Track
mixxx::TrackMetadata readTrackMetadata;
SoundSourceProxy::importTrackMetadataAndCoverImageFromFile(
mixxx::FileAccess(mixxx::FileInfo(exportTrackPath)),
&readTrackMetadata,
nullptr,
false);

// the internal value is still unchanged
EXPECT_EQ(readTrackMetadata.getTrackInfo().getKeyText().toStdString(),
kWhiteSpacesKey.toStdString());

pTrack->replaceMetadataFromSource(
readTrackMetadata,
QDateTime::currentDateTimeUtc());

// getKeytext returns the normalized version suitable for GUI presentation
EXPECT_EQ(pTrack->getKeyText().toStdString(), kNormalizedDisplayKey.toStdString());

// the internal value is still unchanged
EXPECT_EQ(pTrack->getRecord()
.getMetadata()
.getTrackInfo()
.getKeyText()
.toStdString(),
kWhiteSpacesKey.toStdString());

// Reject edits which results to the same key
pTrack->setKeyText(kNormalizedDisplayKey);
EXPECT_EQ(pTrack->getRecord()
.getMetadata()
.getTrackInfo()
.getKeyText()
.toStdString(),
kWhiteSpacesKey.toStdString());

// Allow to remove key with an empty string
pTrack->setKeyText("");
EXPECT_EQ(pTrack->getRecord()
.getMetadata()
.getTrackInfo()
.getKeyText()
.toStdString(),
QString().toStdString());

// normalize user edits
pTrack->setKeyText(kWhiteSpacesKey);
// the internal value is now at the preferred ID3v2 format
EXPECT_EQ(pTrack->getKeys().getGlobalKeyText().toStdString(), kId3Key);
}
3 changes: 2 additions & 1 deletion src/track/keyfactory.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,8 @@ Keys KeyFactory::makeBasicKeys(
mixxx::track::io::key::Source source) {
KeyMap key_map;
key_map.set_global_key(global_key);
QString global_key_text = KeyUtils::keyToString(global_key, KeyUtils::KeyNotation::Custom);
QString global_key_text = KeyUtils::keyToString(
global_key, KeyUtils::KeyNotation::ID3v2);
key_map.set_global_key_text(global_key_text.toStdString());
key_map.set_source(source);
return Keys(key_map);
Expand Down
9 changes: 4 additions & 5 deletions src/track/keys.h
Original file line number Diff line number Diff line change
Expand Up @@ -29,12 +29,11 @@ class Keys final {
const QString& getSubVersion() const;
void setSubVersion(const QString& subVersion);

////////////////////////////////////////////////////////////////////////////
// Key calculations
////////////////////////////////////////////////////////////////////////////

// Return the average key over the entire track if the key is valid.
// Return the average key over the entire track if analyzed by Mixxx
// or the Key found in the track metadata
mixxx::track::io::key::ChromaticKey getGlobalKey() const;

// Return key text form the track metadata literally (not normalized)
QString getGlobalKeyText() const;

private:
Expand Down
131 changes: 106 additions & 25 deletions src/track/keyutils.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -38,31 +38,66 @@ const QString s_sharpSymbol = QString::fromUtf8("♯");
//static const QString s_flatSymbol = QString::fromUtf8("♭");

const QString s_traditionalKeyNames[] = {
QString::fromUtf8("INVALID"),
QString::fromUtf8("C"),
QString::fromUtf8("D♭"),
QString::fromUtf8("D"),
QString::fromUtf8("E♭"),
QString::fromUtf8("E"),
QString::fromUtf8("F"),
QString::fromUtf8("F♯/G♭"),
QString::fromUtf8("G"),
QString::fromUtf8("A♭"),
QString::fromUtf8("A"),
QString::fromUtf8("B♭"),
QString::fromUtf8("B"),
QString::fromUtf8("Cm"),
QString::fromUtf8("C♯m"),
QString::fromUtf8("Dm"),
QString::fromUtf8("D♯m/E♭m"),
QString::fromUtf8("Em"),
QString::fromUtf8("Fm"),
QString::fromUtf8("F♯m"),
QString::fromUtf8("Gm"),
QString::fromUtf8("G♯m"),
QString::fromUtf8("Am"),
QString::fromUtf8("B♭m"),
QString::fromUtf8("Bm")};
QStringLiteral(u"INVALID"),
QStringLiteral(u"C"),
QStringLiteral(u"D♭"),
QStringLiteral(u"D"),
QStringLiteral(u"E♭"),
QStringLiteral(u"E"),
QStringLiteral(u"F"),
QStringLiteral(u"F♯/G♭"),
QStringLiteral(u"G"),
QStringLiteral(u"A♭"),
QStringLiteral(u"A"),
QStringLiteral(u"B♭"),
QStringLiteral(u"B"),
QStringLiteral(u"Cm"),
QStringLiteral(u"C♯m"),
QStringLiteral(u"Dm"),
QStringLiteral(u"D♯m/E♭m"),
QStringLiteral(u"Em"),
QStringLiteral(u"Fm"),
QStringLiteral(u"F♯m"),
QStringLiteral(u"Gm"),
QStringLiteral(u"G♯m"),
QStringLiteral(u"Am"),
QStringLiteral(u"B♭m"),
QStringLiteral(u"Bm")};

// ID3v2.3.0 specification (https://id3.org/id3v2.3.0):
// The 'Initial key' frame contains the musical key in which the sound starts.
// It is represented as a string with a maximum length of three characters.
// The ground keys are represented with "A","B","C","D","E", "F" and "G" and halfkeys
// represented with "b" and "#". Minor is represented as "m". Example "Cbm".
// Off key is represented with an "o" only.
const std::array s_IDv3KeyNames = {
// these are QStringLiterals because they're used as QStrings below, even though they
// only contain ASCII characters
QStringLiteral("o"),
QStringLiteral("C"),
QStringLiteral("Db"),
QStringLiteral("D"),
QStringLiteral("Eb"),
QStringLiteral("E"),
QStringLiteral("F"),
QStringLiteral("F#"),
QStringLiteral("G"),
QStringLiteral("Ab"),
QStringLiteral("A"),
QStringLiteral("Bb"),
QStringLiteral("B"),
QStringLiteral("Cm"),
QStringLiteral("C#m"),
QStringLiteral("Dm"),
QStringLiteral("Ebm"),
QStringLiteral("Em"),
QStringLiteral("Fm"),
QStringLiteral("F#m"),
QStringLiteral("Gm"),
QStringLiteral("G#m"),
QStringLiteral("Am"),
QStringLiteral("Bbm"),
QStringLiteral("Bm")};

// Maps an OpenKey number to its major and minor key.
constexpr ChromaticKey s_openKeyToKeys[][2] = {
Expand Down Expand Up @@ -271,6 +306,50 @@ void KeyUtils::setNotation(const QMap<ChromaticKey, QString>& notation) {
}
}

// static
int KeyUtils::keyToOpenKeyNumber(mixxx::track::io::key::ChromaticKey key) {
switch (key) {
case mixxx::track::io::key::C_MAJOR:
case mixxx::track::io::key::A_MINOR:
return 1;
case mixxx::track::io::key::G_MAJOR:
case mixxx::track::io::key::E_MINOR:
return 2;
case mixxx::track::io::key::D_MAJOR:
case mixxx::track::io::key::B_MINOR:
return 3;
case mixxx::track::io::key::A_MAJOR:
case mixxx::track::io::key::F_SHARP_MINOR:
return 4;
case mixxx::track::io::key::E_MAJOR:
case mixxx::track::io::key::C_SHARP_MINOR:
return 5;
case mixxx::track::io::key::B_MAJOR:
case mixxx::track::io::key::G_SHARP_MINOR:
return 6;
case mixxx::track::io::key::F_SHARP_MAJOR:
case mixxx::track::io::key::E_FLAT_MINOR:
return 7;
case mixxx::track::io::key::D_FLAT_MAJOR:
case mixxx::track::io::key::B_FLAT_MINOR:
return 8;
case mixxx::track::io::key::A_FLAT_MAJOR:
case mixxx::track::io::key::F_MINOR:
return 9;
case mixxx::track::io::key::E_FLAT_MAJOR:
case mixxx::track::io::key::C_MINOR:
return 10;
case mixxx::track::io::key::B_FLAT_MAJOR:
case mixxx::track::io::key::G_MINOR:
return 11;
case mixxx::track::io::key::F_MAJOR:
case mixxx::track::io::key::D_MINOR:
return 12;
default:
return 0;
}
}

// static
QString KeyUtils::keyToString(ChromaticKey key,
KeyNotation notation) {
Expand Down Expand Up @@ -309,6 +388,8 @@ QString KeyUtils::keyToString(ChromaticKey key,
return QString::number(number) + (major ? "B" : "A") + " (" + trad + ")";
} else if (notation == KeyNotation::Traditional) {
return s_traditionalKeyNames[static_cast<int>(key)];
} else if (notation == KeyNotation::ID3v2) {
return s_IDv3KeyNames[static_cast<int>(key)];
}
return keyDebugName(key);
}
Expand Down
Loading

0 comments on commit fe043ac

Please sign in to comment.