Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Engine Prime export uses libdjinterop snapshot API #3776

Merged
merged 1 commit into from
Apr 11, 2021
Merged

Engine Prime export uses libdjinterop snapshot API #3776

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions CMakeLists.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1724,8 +1724,8 @@ if(ENGINEPRIME)
set(DJINTEROP_LIBRARY "lib/${CMAKE_STATIC_LIBRARY_PREFIX}djinterop${CMAKE_STATIC_LIBRARY_SUFFIX}")
file(MAKE_DIRECTORY "${CMAKE_CURRENT_BINARY_DIR}/download")
ExternalProject_Add(libdjinterop
URL "https://github.com/xsco/libdjinterop/archive/0.14.6.tar.gz"
URL_HASH SHA256=db2f57f6c06c801d1785280ede0f032d7280bedd72f2a30bc263a272e3269587
URL "https://github.com/xsco/libdjinterop/archive/0.15.1.tar.gz"
Copy link
Contributor

@uklotzde uklotzde Apr 11, 2021 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Out of scope: Did you consider switching to GIT_REPOSITORY + GIT_TAG? I Just used this variant recently in a project and it seems to be more concise.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This did originally use GIT_REPOSITORY etc. but I changed it in a (sadly misguided) attempt to get the Ubuntu build working. I agree with you that GIT_REPOSITORY is more clear vs. a download link on GitHub. It's also easier during development to temporarily point to a branch (e.g. when I'm testing out a new version of libdjinterop that hasn't been merged yet).

I'll make a note to change this back at some point!

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

#3779

I wasn't aware of the Debian/Ubuntu build dependencies. The Fedora build system requires you to manually upload and hash sources in advance.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There is a more full discussion in #3620

In short, there was no need to change from GIT_REPOSITORY to plain old http downloads. The Ubuntu build should not be downloading sources of dependencies from GitHub, and should instead be using packages. To that end, libdjinterop now has its own PPA, and I either Mixxx's PPA can use the same packages, or make a dependency on the libdjinterop PPA.

This reminds me to talk to Daniel about agreeing the approach, which I'll do on Zulip now.

URL_HASH SHA256=87b3e6c726c208333d55b7e7e3af0a7230c9ad9edb3ca0ca81feffe17b3fc008
DOWNLOAD_DIR "${CMAKE_CURRENT_BINARY_DIR}/download/libdjinterop"
INSTALL_DIR ${DJINTEROP_INSTALL_DIR}
CMAKE_ARGS
Expand Down
83 changes: 54 additions & 29 deletions src/library/export/engineprimeexportjob.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
#include <QStringList>
#include <QtGlobal>
#include <array>
#include <chrono>
#include <cstdint>
#include <djinterop/djinterop.hpp>
#include <memory>
Expand Down Expand Up @@ -105,14 +106,14 @@ QString exportFile(const QSharedPointer<EnginePrimeExportRequest> pRequest,
return pRequest->engineLibraryDbDir.relativeFilePath(dstPath);
}

djinterop::track getTrackByRelativePath(
std::optional<djinterop::track> getTrackByRelativePath(
djinterop::database* pDatabase, const QString& relativePath) {
const auto trackCandidates = pDatabase->tracks_by_relative_path(relativePath.toStdString());
switch (trackCandidates.size()) {
case 0:
return pDatabase->create_track(relativePath.toStdString());
return std::nullopt;
case 1:
return trackCandidates.front();
return std::make_optional(trackCandidates.front());
default:
qWarning() << "More than one external track with the same relative path.";
return trackCandidates.front();
Expand All @@ -124,51 +125,58 @@ void exportMetadata(djinterop::database* pDatabase,
TrackPointer pTrack,
const Waveform* pWaveform,
const QString& relativePath) {
// Create or load the track in the database, using the relative path to
// the music file. We will record the mapping from Mixxx track id to
// exported track id as well.
// Attempt to load the track in the database, using the relative path to
// the music file. If it exists already, take a snapshot of the track and
// update it. If it does not exist, we'll create a new snapshot.
auto externalTrack = getTrackByRelativePath(pDatabase, relativePath);
pMixxxToEnginePrimeTrackIdMap->insert(pTrack->getId(), externalTrack.id());
auto snapshot = externalTrack
? externalTrack->snapshot()
: djinterop::track_snapshot{};
snapshot.relative_path = relativePath.toStdString();

// Note that the Engine Prime format has the scope for recording meta-data
// about whether track was imported from an external database. However,
// that meta-data only extends as far as other Engine Prime databases,
// which Mixxx is not. So we do not set any import information on the
// exported track.
externalTrack.set_track_number(pTrack->getTrackNumber().toInt());
externalTrack.set_bpm(pTrack->getBpm());
externalTrack.set_year(pTrack->getYear().toInt());
externalTrack.set_title(pTrack->getTitle().toStdString());
externalTrack.set_artist(pTrack->getArtist().toStdString());
externalTrack.set_album(pTrack->getAlbum().toStdString());
externalTrack.set_genre(pTrack->getGenre().toStdString());
externalTrack.set_comment(pTrack->getComment().toStdString());
externalTrack.set_composer(pTrack->getComposer().toStdString());
externalTrack.set_key(toDjinteropKey(pTrack->getKey()));
snapshot.track_number = pTrack->getTrackNumber().toInt();
snapshot.duration = std::chrono::milliseconds{
static_cast<int64_t>(1000 * pTrack->getDuration())};
snapshot.bpm = pTrack->getBpm();
snapshot.year = pTrack->getYear().toInt();
snapshot.title = pTrack->getTitle().toStdString();
snapshot.artist = pTrack->getArtist().toStdString();
snapshot.album = pTrack->getAlbum().toStdString();
snapshot.genre = pTrack->getGenre().toStdString();
snapshot.comment = pTrack->getComment().toStdString();
snapshot.composer = pTrack->getComposer().toStdString();
snapshot.key = toDjinteropKey(pTrack->getKey());
int64_t lastModifiedMillisSinceEpoch =
pTrack->getFileInfo().fileLastModified().toMSecsSinceEpoch();
std::chrono::system_clock::time_point lastModifiedAt{
std::chrono::milliseconds{lastModifiedMillisSinceEpoch}};
externalTrack.set_last_modified_at(lastModifiedAt);
externalTrack.set_bitrate(pTrack->getBitrate());
snapshot.last_modified_at = lastModifiedAt;
snapshot.bitrate = pTrack->getBitrate();
snapshot.rating = pTrack->getRating() * 20; // note rating is in range 0-100

// Frames used interchangeably with "samples" here.
const auto sampleCount = static_cast<int64_t>(pTrack->getDuration() * pTrack->getSampleRate());
externalTrack.set_sampling({static_cast<double>(pTrack->getSampleRate()), sampleCount});
snapshot.sampling = djinterop::sampling_info{
static_cast<double>(pTrack->getSampleRate()), sampleCount};

// Set track loudness.
// Note that the djinterop API method for setting loudness may be revised
// in future, as more is discovered about the exact meaning of the loudness
// field in the Engine Library format. Make the assumption for now that
// ReplayGain ratio is an appropriate value to set, which has been validated
// by basic experimental testing.
externalTrack.set_average_loudness(pTrack->getReplayGain().getRatio());
snapshot.average_loudness = pTrack->getReplayGain().getRatio();

// Set main cue-point.
double cuePlayPos = pTrack->getCuePoint().getPosition();
double cueSampleOffset = playPosToSampleOffset(cuePlayPos);
externalTrack.set_default_main_cue(cueSampleOffset);
externalTrack.set_adjusted_main_cue(cueSampleOffset);
snapshot.default_main_cue = cueSampleOffset;
snapshot.adjusted_main_cue = cueSampleOffset;

// Fill in beat grid. For now, assume a constant average BPM across
// the whole track. Note that points in the track are specified as
Expand Down Expand Up @@ -197,8 +205,8 @@ void exportMetadata(djinterop::database* pDatabase,
{0, playPosToSampleOffset(firstBarAlignedBeatPlayPos)},
{numBeats, playPosToSampleOffset(lastBeatPlayPos)}};
beatgrid = el::normalize_beatgrid(std::move(beatgrid), sampleCount);
externalTrack.set_default_beatgrid(beatgrid);
externalTrack.set_adjusted_beatgrid(beatgrid);
snapshot.default_beatgrid = beatgrid;
snapshot.adjusted_beatgrid = beatgrid;
} else {
qWarning() << "Non-positive number of beats in beat data of track" << pTrack->getId()
<< "(" << pTrack->getFileInfo().fileName() << ")";
Expand All @@ -208,7 +216,10 @@ void exportMetadata(djinterop::database* pDatabase,
<< "(" << pTrack->getFileInfo().fileName() << ")";
}

// Note that any existing hot cues on the track are kept in place, if Mixxx
// does not have a hot cue at that location.
const auto cues = pTrack->getCuePoints();
snapshot.hot_cues.fill(djinterop::stdx::nullopt);
for (const CuePointer& pCue : cues) {
// We are only interested in hot cues.
if (pCue->getType() != CueType::HotCue) {
Expand All @@ -232,21 +243,23 @@ void exportMetadata(djinterop::database* pDatabase,
hotCue.label = label.toStdString();
hotCue.sample_offset = playPosToSampleOffset(pCue->getPosition());
hotCue.color = el::standard_pad_colors::pads[hotCueIndex];
externalTrack.set_hot_cue_at(hotCueIndex, hotCue);
snapshot.hot_cues[hotCueIndex] = hotCue;
}

// Note that Mixxx does not support pre-calculated stored loops, but it will
// remember the position of a single ad-hoc loop between track loads.
// However, since this single ad-hoc loop is likely to be different in use
// from a set of stored loops (and is easily overwritten), we do not export
// it to the external database here.
externalTrack.set_loops({});
//
// Note also that the loops on any existing track are not modified here.

// Write waveform.
// Note that writing a single waveform will automatically calculate an
// overview waveform too.
if (pWaveform) {
int64_t samplesPerEntry = externalTrack.required_waveform_samples_per_entry();
int64_t samplesPerEntry =
el::required_waveform_samples_per_entry(pTrack->getSampleRate());
int64_t externalWaveformSize = (sampleCount + samplesPerEntry - 1) / samplesPerEntry;
std::vector<djinterop::waveform_entry> externalWaveform;
externalWaveform.reserve(externalWaveformSize);
Expand All @@ -256,11 +269,23 @@ void exportMetadata(djinterop::database* pDatabase,
{pWaveform->getMid(j), kDefaultWaveformOpacity},
{pWaveform->getHigh(j), kDefaultWaveformOpacity}});
}
externalTrack.set_waveform(std::move(externalWaveform));
snapshot.waveform = std::move(externalWaveform);
} else {
qInfo() << "No waveform data found for track" << pTrack->getId()
<< "(" << pTrack->getFileInfo().fileName() << ")";
}

int externalTrackId;
if (externalTrack) {
externalTrack->update(snapshot);
externalTrackId = externalTrack->id();
} else {
auto newTrack = pDatabase->create_track(snapshot);
externalTrackId = newTrack.id();
}

// Record the mapping from Mixxx track id to exported track id.
pMixxxToEnginePrimeTrackIdMap->insert(pTrack->getId(), externalTrackId);
}

void exportTrack(
Expand Down