Skip to content

Commit

Permalink
BeatMap: Reimplement findNthBeat without fuzzy behavior
Browse files Browse the repository at this point in the history
  • Loading branch information
@Holzhaus
Holzhaus committed Jul 11, 2021
1 parent 0979e8e commit 6929e13
Show file tree
Hide file tree
Showing 2 changed files with 38 additions and 152 deletions.
91 changes: 0 additions & 91 deletions src/test/beatmaptest.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -156,97 +156,6 @@ TEST_F(BeatMapTest, TestNthBeatWhenOnBeat) {
EXPECT_EQ(position, pMap->findPrevBeat(position));
}

TEST_F(BeatMapTest, TestNthBeatWhenOnBeat_BeforeEpsilon) {
constexpr mixxx::Bpm bpm(60.0);
m_pTrack->trySetBpm(bpm.value());
mixxx::audio::FrameDiff_t beatLengthFrames = getBeatLengthFrames(bpm);
const auto startOffsetFrames = mixxx::audio::FramePos(7);
const int numBeats = 100;
// Note beats must be in frames, not samples.
QVector<mixxx::audio::FramePos> beats =
createBeatVector(startOffsetFrames, numBeats, beatLengthFrames);
auto pMap = BeatMap::makeBeatMap(m_pTrack->getSampleRate(), QString(), beats);

// Pretend we're just before the 20th beat;
const int curBeat = 20;
const mixxx::audio::FramePos kClosestBeat = startOffsetFrames + curBeat * beatLengthFrames;
const mixxx::audio::FramePos position = kClosestBeat - beatLengthFrames * 0.005;

// The spec dictates that a value of 0 is always invalid and returns -1
EXPECT_FALSE(pMap->findNthBeat(position, 0).isValid());

// findNthBeat should return exactly the current beat if we ask for 1 or
// -1. For all other values, it should return n times the beat length.
for (int i = 1; i < curBeat; ++i) {
EXPECT_DOUBLE_EQ((kClosestBeat + beatLengthFrames * (i - 1)).value(),
pMap->findNthBeat(position, i).value());
EXPECT_DOUBLE_EQ((kClosestBeat + beatLengthFrames * (-i + 1)).value(),
pMap->findNthBeat(position, -i).value());
}

// Also test prev/next beat calculation
mixxx::audio::FramePos prevBeat, nextBeat;
pMap->findPrevNextBeats(position, &prevBeat, &nextBeat, true);
EXPECT_EQ(kClosestBeat, prevBeat);
EXPECT_EQ(kClosestBeat + beatLengthFrames, nextBeat);

// Also test prev/next beat calculation without snapping tolerance
pMap->findPrevNextBeats(position, &prevBeat, &nextBeat, false);
EXPECT_EQ(kClosestBeat - beatLengthFrames, prevBeat);
EXPECT_EQ(kClosestBeat, nextBeat);

// Both previous and next beat should return the closest beat.
EXPECT_EQ(kClosestBeat, pMap->findNextBeat(position));
EXPECT_EQ(kClosestBeat, pMap->findPrevBeat(position));

}

TEST_F(BeatMapTest, TestNthBeatWhenOnBeat_AfterEpsilon) {
constexpr mixxx::Bpm bpm(60.0);
m_pTrack->trySetBpm(bpm.value());
mixxx::audio::FrameDiff_t beatLengthFrames = getBeatLengthFrames(bpm);
const auto startOffsetFrames = mixxx::audio::FramePos(7);
const int numBeats = 100;
// Note beats must be in frames, not samples.
QVector<mixxx::audio::FramePos> beats =
createBeatVector(startOffsetFrames, numBeats, beatLengthFrames);
auto pMap = BeatMap::makeBeatMap(m_pTrack->getSampleRate(), QString(), beats);

// Pretend we're just after the 20th beat;
const int curBeat = 20;
const mixxx::audio::FramePos kClosestBeat = startOffsetFrames + curBeat * beatLengthFrames;
const mixxx::audio::FramePos position = kClosestBeat + beatLengthFrames * 0.005;

// The spec dictates that a value of 0 is always invalid and returns -1
EXPECT_FALSE(pMap->findNthBeat(position, 0).isValid());

EXPECT_EQ(kClosestBeat, pMap->findClosestBeat(position));

// findNthBeat should return exactly the current beat if we ask for 1 or
// -1. For all other values, it should return n times the beat length.
for (int i = 1; i < curBeat; ++i) {
EXPECT_DOUBLE_EQ((kClosestBeat + beatLengthFrames * (i - 1)).value(),
pMap->findNthBeat(position, i).value());
EXPECT_DOUBLE_EQ((kClosestBeat + beatLengthFrames * (-i + 1)).value(),
pMap->findNthBeat(position, -i).value());
}

// Also test prev/next beat calculation.
mixxx::audio::FramePos prevBeat, nextBeat;
pMap->findPrevNextBeats(position, &prevBeat, &nextBeat, true);
EXPECT_EQ(kClosestBeat, prevBeat);
EXPECT_EQ(kClosestBeat + beatLengthFrames, nextBeat);

// Also test prev/next beat calculation without snapping tolerance
pMap->findPrevNextBeats(position, &prevBeat, &nextBeat, false);
EXPECT_EQ(kClosestBeat, prevBeat);
EXPECT_EQ(kClosestBeat + beatLengthFrames, nextBeat);

// Both previous and next beat should return the closest beat.
EXPECT_EQ(kClosestBeat, pMap->findNextBeat(position));
EXPECT_EQ(kClosestBeat, pMap->findPrevBeat(position));
}

TEST_F(BeatMapTest, TestNthBeatWhenNotOnBeat) {
constexpr mixxx::Bpm bpm(60.0);
m_pTrack->trySetBpm(bpm.value());
Expand Down
99 changes: 38 additions & 61 deletions src/track/beatmap.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -315,80 +315,57 @@ audio::FramePos BeatMap::findNthBeat(audio::FramePos position, int n) const {
return audio::kInvalidFramePos;
}

Beat beat = beatFromFramePos(position.toNearestFrameBoundary());

// it points at the first occurrence of beat or the next largest beat
BeatList::const_iterator it =
std::lower_bound(m_beats.constBegin(), m_beats.constEnd(), beat, beatLessThan);

// If the position is within 1/10th of a second of the next or previous
// beat, pretend we are on that beat.
const double kFrameEpsilon = 0.1 * m_sampleRate;
int numBeatsLeft = std::abs(n);

// Back-up by one.
if (it != m_beats.begin()) {
--it;
}
const auto searchFromPosition = (n > 0) ? position.toUpperFrameBoundary()
: position.toLowerFrameBoundary();
const Beat searchFromBeat = beatFromFramePos(searchFromPosition);
auto it = std::lower_bound(m_beats.cbegin(), m_beats.cend(), searchFromBeat, beatLessThan);

// Scan forward to find whether we are on a beat.
BeatList::const_iterator on_beat = m_beats.constEnd();
BeatList::const_iterator previous_beat = m_beats.constEnd();
BeatList::const_iterator next_beat = m_beats.constEnd();
for (; it != m_beats.end(); ++it) {
qint32 delta = it->frame_position() - beat.frame_position();
if (n > 0) {
// Search in forward direction
numBeatsLeft--;

// We are "on" this beat.
if (abs(delta) < kFrameEpsilon) {
on_beat = it;
break;
while (it != m_beats.cend() && numBeatsLeft > 0) {
if (it->enabled()) {
numBeatsLeft--;
}
it++;
}
} else {
// Search in backward direction
numBeatsLeft--;

if (delta < 0) {
// If we are not on the beat and delta < 0 then this beat comes
// before our current position.
previous_beat = it;
} else {
// If we are past the beat and we aren't on it then this beat comes
// after our current position.
next_beat = it;
// Stop because we have everything we need now.
break;
if (it == m_beats.cend()) {
return audio::kInvalidFramePos;
}
}

// If we are within epsilon samples of a beat then the immediately next and
// previous beats are the beat we are on.
if (on_beat != m_beats.end()) {
next_beat = on_beat;
previous_beat = on_beat;
}

if (n > 0) {
for (; next_beat != m_beats.end(); ++next_beat) {
if (!next_beat->enabled()) {
continue;
}
if (n == 1) {
return mixxx::audio::FramePos(next_beat->frame_position());
// We may be one beat behind the searchFromBeat. In this case, we advance
// the reverse iterator by one beat.
if (it->frame_position() > searchFromBeat.frame_position()) {
if (it == m_beats.cbegin()) {
return audio::kInvalidFramePos;
}
--n;
it--;
}
} else if (n < 0 && previous_beat != m_beats.end()) {
for (; true; --previous_beat) {
if (previous_beat->enabled()) {
if (n == -1) {
return mixxx::audio::FramePos(previous_beat->frame_position());
}
++n;
}

// Don't step before the start of the list.
if (previous_beat == m_beats.begin()) {
break;
while (it != m_beats.cbegin() && numBeatsLeft > 0) {
if (it->enabled()) {
numBeatsLeft--;
}
it--;
}
}
return audio::kInvalidFramePos;

if (numBeatsLeft > 0 || it == m_beats.cend()) {
return audio::kInvalidFramePos;
}

const auto foundBeatPosition = mixxx::audio::FramePos(it->frame_position());

DEBUG_ASSERT(foundBeatPosition >= searchFromPosition || n < 0);
DEBUG_ASSERT(foundBeatPosition <= searchFromPosition || n > 0);
return foundBeatPosition;
}

bool BeatMap::findPrevNextBeats(audio::FramePos position,
Expand Down

0 comments on commit 6929e13

Please sign in to comment.