Skip to content

Commit

Permalink
Merge pull request #3885 from Holzhaus/serato-beatgrid-import-fix
Browse files Browse the repository at this point in the history 
SeratoBeatsImporter: Fix import/export of non-const beatgrids
  • Loading branch information
@uklotzde
uklotzde authored May 24, 2021
2 parents 9a105f2 + 3c23fcc commit 3fd7adc
Show file tree
Hide file tree
Showing 4 changed files with 218 additions and 98 deletions.
147 changes: 130 additions & 17 deletions src/test/seratobeatgridtest.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -8,10 +8,18 @@
#include "track/beatgrid.h"
#include "track/beatmap.h"
#include "track/serato/beatgrid.h"
#include "track/serato/beatsimporter.h"
#include "util/memory.h"

namespace {

// Maximum allowed frame position inaccuracy after reimport
constexpr double kEpsilon = 0.1;

} // namespace

namespace mixxx {

class SeratoBeatGridTest : public testing::Test {
protected:
bool parseBeatGridData(const QByteArray& inputValue,
Expand Down Expand Up @@ -122,56 +130,161 @@ TEST_F(SeratoBeatGridTest, SerializeBeatgrid) {

TEST_F(SeratoBeatGridTest, SerializeBeatMap) {
// Create a non-const beatmap
constexpr double bpm = 100.0;
constexpr double timingOffsetMillis = -10;
constexpr int bpm = 120;
const auto sampleRate = mixxx::audio::SampleRate(44100);
const auto signalInfo = mixxx::audio::SignalInfo(mixxx::audio::ChannelCount(2), sampleRate);
const auto duration = mixxx::Duration::fromSeconds<int>(300);
const double framesPerMinute = signalInfo.getSampleRate() * 60;
const double framesPerBeat = framesPerMinute / bpm;
const double initialFrameOffset = framesPerBeat / 2;

QVector<double> beatPositionsFrames;
double beatPositionFrames = 0;
// Add 2 minutes of beats at 100 bpm to the beatgrid
for (int i = 0; i < 2 * bpm; i++) {
double beatPositionFrames = initialFrameOffset;

constexpr int kNumBeats120BPM = 4;
qInfo() << "Step 1: Add" << kNumBeats120BPM << "beats at 100 bpm to the beatgrid";
for (int i = 0; i < kNumBeats120BPM; i++) {
beatPositionsFrames.append(beatPositionFrames);
beatPositionFrames += framesPerBeat;
}
ASSERT_EQ(beatPositionsFrames.size(), kNumBeats120BPM);

// Check the const beatmap
{
const auto pBeats = mixxx::BeatMap::makeBeatMap(
sampleRate, QString("Test"), beatPositionsFrames);
// At the 1 minute mark the BPM should be 100
EXPECT_EQ(pBeats->getBpmAroundPosition(framesPerMinute, 1), bpm);
// Check that the first section's BPM is 100
EXPECT_EQ(pBeats->getBpmAroundPosition(
signalInfo.frames2samples(
static_cast<SINT>(initialFrameOffset +
framesPerBeat * kNumBeats120BPM / 2)),
1),
bpm);

mixxx::SeratoBeatGrid seratoBeatGrid;
seratoBeatGrid.setBeats(pBeats, signalInfo, duration, 0);
seratoBeatGrid.setBeats(pBeats, signalInfo, duration, timingOffsetMillis);
EXPECT_EQ(seratoBeatGrid.nonTerminalMarkers().size(), 0);
EXPECT_NE(seratoBeatGrid.terminalMarker(), nullptr);
EXPECT_FLOAT_EQ(seratoBeatGrid.terminalMarker()->bpm(), static_cast<float>(bpm));

// Check if the beats can be re-imported losslessly
mixxx::SeratoBeatsImporter beatsImporter(
seratoBeatGrid.nonTerminalMarkers(),
seratoBeatGrid.terminalMarker());
QVector<double> importedBeatPositionsFrames =
beatsImporter.importBeatsAndApplyTimingOffset(timingOffsetMillis, signalInfo);
ASSERT_EQ(beatPositionsFrames.size(), importedBeatPositionsFrames.size());
for (int i = 0; i < beatPositionsFrames.size(); i++) {
EXPECT_NEAR(beatPositionsFrames[i], importedBeatPositionsFrames[i], kEpsilon);
}
}

// Now add 3 minutes of beats at 50 bpm to the beatgrid
for (int i = 0; i < 3 * bpm / 2; i++) {
constexpr int kNumBeats60BPM = 4;
qInfo() << "Step 2: Add" << kNumBeats60BPM << "beats at 50 bpm to the beatgrid";
for (int i = 0; i < kNumBeats60BPM; i++) {
beatPositionsFrames.append(beatPositionFrames);
beatPositionFrames += framesPerBeat * 2;
}
ASSERT_EQ(beatPositionsFrames.size(), kNumBeats120BPM + kNumBeats60BPM);

// Check the non-const beatmap
{
const auto pBeats = mixxx::BeatMap::makeBeatMap(
sampleRate, QString("Test"), beatPositionsFrames);
// At the 1 minute mark the BPM should be 100
EXPECT_EQ(pBeats->getBpmAroundPosition(framesPerMinute, 1), bpm);
// At the 4 minute mark the BPM should be 50
EXPECT_EQ(pBeats->getBpmAroundPosition(framesPerMinute * 4, 1), bpm / 2);
// Check that the first section'd BPM is 100
EXPECT_EQ(pBeats->getBpmAroundPosition(
signalInfo.frames2samples(
static_cast<SINT>(initialFrameOffset +
framesPerBeat * kNumBeats120BPM / 2)),
1),
bpm);
// Check that the second section'd BPM is 50
EXPECT_EQ(pBeats->getBpmAroundPosition(
signalInfo.frames2samples(
static_cast<SINT>(initialFrameOffset +
framesPerBeat * kNumBeats120BPM +
framesPerBeat * kNumBeats60BPM / 2)),
1),
bpm / 2);

mixxx::SeratoBeatGrid seratoBeatGrid;
seratoBeatGrid.setBeats(pBeats, signalInfo, duration, 0);
EXPECT_EQ(seratoBeatGrid.nonTerminalMarkers().size(), 2);
seratoBeatGrid.setBeats(pBeats, signalInfo, duration, timingOffsetMillis);
ASSERT_EQ(seratoBeatGrid.nonTerminalMarkers().size(), 2);
ASSERT_EQ(seratoBeatGrid.nonTerminalMarkers()[0]->beatsTillNextMarker(), kNumBeats120BPM);
ASSERT_EQ(seratoBeatGrid.nonTerminalMarkers()[1]->beatsTillNextMarker(),
kNumBeats60BPM - 1);
EXPECT_NE(seratoBeatGrid.terminalMarker(), nullptr);
EXPECT_FLOAT_EQ(seratoBeatGrid.terminalMarker()->bpm(), static_cast<float>(bpm / 2));

// Check if the beats can be re-imported losslessly
mixxx::SeratoBeatsImporter beatsImporter(
seratoBeatGrid.nonTerminalMarkers(),
seratoBeatGrid.terminalMarker());
QVector<double> importedBeatPositionsFrames =
beatsImporter.importBeatsAndApplyTimingOffset(timingOffsetMillis, signalInfo);
ASSERT_EQ(beatPositionsFrames.size(), importedBeatPositionsFrames.size());
for (int i = 0; i < beatPositionsFrames.size(); i++) {
EXPECT_NEAR(beatPositionsFrames[i], importedBeatPositionsFrames[i], kEpsilon);
}
}

qInfo() << "Step 3: Add" << kNumBeats120BPM << "beats at 100 bpm to the beatgrid";
for (int i = 0; i < kNumBeats120BPM; i++) {
beatPositionsFrames.append(beatPositionFrames);
beatPositionFrames += framesPerBeat;
}
ASSERT_EQ(beatPositionsFrames.size(), 2 * kNumBeats120BPM + kNumBeats60BPM);

// Add the last beat
beatPositionsFrames.append(beatPositionFrames);

{
const auto pBeats = mixxx::BeatMap::makeBeatMap(
sampleRate, QString("Test"), beatPositionsFrames);
// Check that the first section's BPM is 100
EXPECT_EQ(pBeats->getBpmAroundPosition(
signalInfo.frames2samples(
static_cast<SINT>(initialFrameOffset +
framesPerBeat * kNumBeats120BPM / 2)),
1),
bpm);
// Check that the second section's BPM is 50
EXPECT_EQ(pBeats->getBpmAroundPosition(
signalInfo.frames2samples(
static_cast<SINT>(initialFrameOffset +
framesPerBeat * kNumBeats120BPM +
framesPerBeat * kNumBeats60BPM / 2)),
1),
bpm / 2);
// Check that the third section's BPM is 100
EXPECT_EQ(pBeats->getBpmAroundPosition(
signalInfo.frames2samples(static_cast<SINT>(
initialFrameOffset +
framesPerBeat * kNumBeats120BPM * 1.5 +
framesPerBeat * kNumBeats60BPM)),
1),
bpm / 2);

mixxx::SeratoBeatGrid seratoBeatGrid;
seratoBeatGrid.setBeats(pBeats, signalInfo, duration, timingOffsetMillis);
ASSERT_EQ(seratoBeatGrid.nonTerminalMarkers().size(), 3);
ASSERT_EQ(seratoBeatGrid.nonTerminalMarkers()[0]->beatsTillNextMarker(), kNumBeats120BPM);
ASSERT_EQ(seratoBeatGrid.nonTerminalMarkers()[1]->beatsTillNextMarker(), kNumBeats60BPM);
ASSERT_EQ(seratoBeatGrid.nonTerminalMarkers()[2]->beatsTillNextMarker(), kNumBeats60BPM);
EXPECT_NE(seratoBeatGrid.terminalMarker(), nullptr);
EXPECT_FLOAT_EQ(seratoBeatGrid.terminalMarker()->bpm(), static_cast<float>(bpm));

// Check if the beats can be re-imported losslessly
mixxx::SeratoBeatsImporter beatsImporter(
seratoBeatGrid.nonTerminalMarkers(),
seratoBeatGrid.terminalMarker());
QVector<double> importedBeatPositionsFrames =
beatsImporter.importBeatsAndApplyTimingOffset(timingOffsetMillis, signalInfo);
ASSERT_EQ(beatPositionsFrames.size(), importedBeatPositionsFrames.size());
for (int i = 0; i < beatPositionsFrames.size(); i++) {
EXPECT_NEAR(beatPositionsFrames[i], importedBeatPositionsFrames[i], kEpsilon);
}
}
}

} // namespace
} // namespace mixxx
93 changes: 33 additions & 60 deletions src/track/serato/beatgrid.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -443,73 +443,57 @@ void SeratoBeatGrid::setBeats(BeatsPointer pBeats,
auto pBeatsIterator = pBeats->findBeats(0, trackDurationSamples);

// This might be null if the track doesn't contain any beats
if (!pBeatsIterator) {
if (!pBeatsIterator || !pBeatsIterator->hasNext()) {
qWarning() << "Serato Beatgrid: No beats available, exporting empty beatgrid!";
setTerminalMarker(nullptr);
setNonTerminalMarkers({});
return;
}

const double beatgridFrameOffset = signalInfo.samples2framesFractional(pBeatsIterator->next());
double currentBeatPositionFrames = 0;
double previousBeatPositionFrames = 0;
double previousDeltaFrames = -1;
int beatsSinceLastMarker = 0;
QList<SeratoBeatGridNonTerminalMarkerPointer> nonTerminalMarkers;
{
const double positionSecs = signalInfo.frames2secsFractional(
currentBeatPositionFrames + beatgridFrameOffset);
nonTerminalMarkers.append(
std::make_shared<SeratoBeatGridNonTerminalMarker>(
positionSecs - timingOffsetSecs, 0));
}
while (pBeatsIterator->hasNext()) {
previousBeatPositionFrames = currentBeatPositionFrames;
currentBeatPositionFrames =
signalInfo.samples2framesFractional(
pBeatsIterator->next());
signalInfo.samples2framesFractional(pBeatsIterator->next()) -
beatgridFrameOffset;

// Calculate the delta between the current beat and the previous beat.
// If the distance is the same as the distance between the previous
// beat and the beat before that, we can just increment
// `beatsSinceLastMarker`. If not, we need to add a new marker.
const double currentDeltaFrames = currentBeatPositionFrames - previousBeatPositionFrames;
if (previousDeltaFrames < 0) {
previousDeltaFrames = currentDeltaFrames;
}

const double differenceBetweenCurrentAndPreviousDelta =
abs(currentDeltaFrames - previousDeltaFrames);
if (differenceBetweenCurrentAndPreviousDelta > kEpsilon) {
if (differenceBetweenCurrentAndPreviousDelta >= kEpsilon) {
const double positionSecs = signalInfo.frames2secsFractional(
previousBeatPositionFrames + beatgridFrameOffset);
nonTerminalMarkers.append(
std::make_shared<SeratoBeatGridNonTerminalMarker>(
positionSecs - timingOffsetSecs, 1));
} else {
// We are adding a new beat marker, therefore we need to update the
// `beatsSinceLastMarker` variable of the last marker we added.
if (!nonTerminalMarkers.isEmpty()) {
DEBUG_ASSERT(beatsSinceLastMarker > 0);
const auto pNonTerminalMarker =
nonTerminalMarkers.at(nonTerminalMarkers.size() - 1);
DEBUG_ASSERT(pNonTerminalMarker);
pNonTerminalMarker->setBeatsTillNextMarker(beatsSinceLastMarker);

// After adding the first marker, the the sample delta won't
// match, because it compares the distance between beats 1 and
// two with the distance between the start of the track and the
// first beat. This special case makes sure that we don't add
// an unnecessary additional marker that has
// beatsSinceLastMarker set to 1.
if (nonTerminalMarkers.size() == 1 && beatsSinceLastMarker == 1) {
previousDeltaFrames = currentDeltaFrames;
beatsSinceLastMarker++;
continue;
}
}
// Don't create a SeratoBeatGridNonTerminalMarker entry for the
// last beat, this needs to be a terminal marker entry.
if (pBeatsIterator->hasNext()) {
const double positionSecs =
signalInfo.frames2secsFractional(
currentBeatPositionFrames) -
timingOffsetSecs;
nonTerminalMarkers.append(
std::make_shared<SeratoBeatGridNonTerminalMarker>(positionSecs, 0));
}
beatsSinceLastMarker = 0;
const auto pNonTerminalMarker = nonTerminalMarkers.last();
pNonTerminalMarker->setBeatsTillNextMarker(
pNonTerminalMarker->beatsTillNextMarker() + 1);
}
beatsSinceLastMarker++;
previousDeltaFrames = currentDeltaFrames;
}

// The track has no beats. This isn't possible because the `pBeatsIterator`
// check above should have caught this case already.
VERIFY_OR_DEBUG_ASSERT(currentBeatPositionFrames != -1) {
return;
previousDeltaFrames = currentDeltaFrames;
}

// If the beatgrid is constant, i.e. if there is only a single non-terminal
Expand All @@ -520,28 +504,17 @@ void SeratoBeatGrid::setBeats(BeatsPointer pBeats,
nonTerminalMarkers.clear();
}

// Update the `beatsSinceLastMarker` of the last non-terminal marker we inserted.
if (!nonTerminalMarkers.isEmpty()) {
DEBUG_ASSERT(beatsSinceLastMarker > 0);
const auto pNonTerminalMarker = nonTerminalMarkers.at(nonTerminalMarkers.size() - 1);
DEBUG_ASSERT(pNonTerminalMarker);
// We need to subtract 1 from `beatsSinceLastMarker`, because at the end of
// the last iteration the counter is incremented even though we didn't move
// a beat forwards.
pNonTerminalMarker->setBeatsTillNextMarker(beatsSinceLastMarker - 1);
}

// Finally, create the terminal marker.
const double positionSecs =
signalInfo.frames2secsFractional(
currentBeatPositionFrames) -
timingOffsetSecs;
const double currentBeatPositionFramesWithOffset =
currentBeatPositionFrames + beatgridFrameOffset;
const double positionSecs = signalInfo.frames2secsFractional(
currentBeatPositionFramesWithOffset);
const double bpm = pBeats->getBpmAroundPosition(
signalInfo.frames2samples(
static_cast<SINT>(currentBeatPositionFrames)),
signalInfo.getChannelCount() * currentBeatPositionFramesWithOffset,
1);

setTerminalMarker(std::make_shared<SeratoBeatGridTerminalMarker>(positionSecs, bpm));
setTerminalMarker(std::make_shared<SeratoBeatGridTerminalMarker>(
positionSecs - timingOffsetSecs, bpm));
setNonTerminalMarkers(nonTerminalMarkers);
}

Expand Down
Loading

0 comments on commit 3fd7adc

Please sign in to comment.