diff --git a/src/audio/frame.h b/src/audio/frame.h index b1c619f0f35..c967eb84a7f 100644 --- a/src/audio/frame.h +++ b/src/audio/frame.h @@ -109,6 +109,12 @@ class FramePos final { return FramePos(std::floor(value())); } + /// Return position rounded to the next upper full frame position, without + /// the fractional part. + [[nodiscard]] FramePos toUpperFrameBoundary() const { + return FramePos(std::ceil(value())); + } + /// Return position rounded to the nearest full frame position, without /// the fractional part. [[nodiscard]] FramePos toNearestFrameBoundary() const { diff --git a/src/test/beatmaptest.cpp b/src/test/beatmaptest.cpp index 6cbe84dbb50..96e2e1940ab 100644 --- a/src/test/beatmaptest.cpp +++ b/src/test/beatmaptest.cpp @@ -10,6 +10,15 @@ using namespace mixxx; namespace { +int countRemainingBeats(std::unique_ptr pIterator) { + int numBeatsFound = 0; + while (pIterator->hasNext()) { + pIterator->next(); + numBeatsFound++; + } + return numBeatsFound; +} + class BeatMapTest : public testing::Test { protected: BeatMapTest() @@ -326,4 +335,83 @@ TEST_F(BeatMapTest, TestBpmAround) { .value()); } +TEST_F(BeatMapTest, FindBeatsWithFractionalPos) { + constexpr mixxx::Bpm bpm(60.0); + constexpr int numBeats = 120; + const mixxx::audio::FrameDiff_t beatLengthFrames = getBeatLengthFrames(bpm); + ASSERT_EQ(beatLengthFrames, std::round(beatLengthFrames)); + + mixxx::audio::FramePos beatPos = mixxx::audio::kStartFramePos; + const mixxx::audio::FramePos lastBeatPos = beatPos + beatLengthFrames * (numBeats - 1); + QVector beats; + for (; beatPos <= lastBeatPos; beatPos += beatLengthFrames) { + beats.append(beatPos); + } + const auto pMap = BeatMap::makeBeatMap(m_pTrack->getSampleRate(), QString(), beats); + + // All beats are in range + auto it = pMap->findBeats(mixxx::audio::kStartFramePos, lastBeatPos); + int numBeatsFound = countRemainingBeats(std::move(it)); + EXPECT_EQ(numBeats, numBeatsFound); + + // Only half the beats are in range + const auto halfBeatsPosition = mixxx::audio::kStartFramePos + + beatLengthFrames * ((numBeats / 2) - 1); + it = pMap->findBeats(mixxx::audio::kStartFramePos, halfBeatsPosition); + numBeatsFound = countRemainingBeats(std::move(it)); + EXPECT_EQ(numBeats / 2, numBeatsFound); + + // First beat is not in range + it = pMap->findBeats(mixxx::audio::kStartFramePos + 0.5, lastBeatPos + 0.5); + numBeatsFound = countRemainingBeats(std::move(it)); + EXPECT_EQ(numBeats - 1, numBeatsFound); + + // Last beat is not in range + it = pMap->findBeats(mixxx::audio::kStartFramePos - 0.5, lastBeatPos - 0.5); + numBeatsFound = countRemainingBeats(std::move(it)); + EXPECT_EQ(numBeats - 1, numBeatsFound); + + // All beats are in range + it = pMap->findBeats(mixxx::audio::kStartFramePos - 0.5, lastBeatPos + 0.5); + numBeatsFound = countRemainingBeats(std::move(it)); + EXPECT_EQ(numBeats, numBeatsFound); + + // First and last beats in range + it = pMap->findBeats(mixxx::audio::kStartFramePos + 0.5, lastBeatPos - 0.5); + numBeatsFound = countRemainingBeats(std::move(it)); + EXPECT_EQ(numBeats - 2, numBeatsFound); +} + +TEST_F(BeatMapTest, HasBeatInRangeWithFractionalPos) { + constexpr mixxx::Bpm bpm(60.0); + constexpr int numBeats = 120; + const mixxx::audio::FrameDiff_t beatLengthFrames = getBeatLengthFrames(bpm); + ASSERT_EQ(beatLengthFrames, std::round(beatLengthFrames)); + + mixxx::audio::FramePos beatPos = mixxx::audio::kStartFramePos; + const mixxx::audio::FramePos lastBeatPos = beatPos + beatLengthFrames * (numBeats - 1); + QVector beats; + for (; beatPos <= lastBeatPos; beatPos += beatLengthFrames) { + beats.append(beatPos); + } + const auto pMap = BeatMap::makeBeatMap(m_pTrack->getSampleRate(), QString(), beats); + + const mixxx::audio::FrameDiff_t halfBeatLengthFrames = beatLengthFrames / 2; + EXPECT_TRUE(pMap->hasBeatInRange(mixxx::audio::kStartFramePos, + mixxx::audio::kStartFramePos + halfBeatLengthFrames)); + EXPECT_TRUE(pMap->hasBeatInRange(mixxx::audio::kStartFramePos - 0.2, + mixxx::audio::kStartFramePos + halfBeatLengthFrames)); + // FIXME: The next comparison is broken due to fuzzy matching in BeatMap::findNthBeat() + //EXPECT_FALSE(pMap->hasBeatInRange(mixxx::audio::kStartFramePos + 0.2, mixxx::audio::kStartFramePos + halfBeatLengthFrames)); + EXPECT_TRUE(pMap->hasBeatInRange( + mixxx::audio::kStartFramePos - halfBeatLengthFrames, + mixxx::audio::kStartFramePos)); + EXPECT_FALSE(pMap->hasBeatInRange( + mixxx::audio::kStartFramePos - halfBeatLengthFrames, + mixxx::audio::kStartFramePos - 0.2)); + EXPECT_TRUE(pMap->hasBeatInRange( + mixxx::audio::kStartFramePos - halfBeatLengthFrames, + mixxx::audio::kStartFramePos + 0.2)); +} + } // namespace diff --git a/src/test/frametest.cpp b/src/test/frametest.cpp index 4fd28be1c8f..6df0eb8332a 100644 --- a/src/test/frametest.cpp +++ b/src/test/frametest.cpp @@ -78,3 +78,31 @@ TEST_F(FrameTest, LowerFrameBoundary) { EXPECT_EQ(mixxx::audio::FramePos(-101), mixxx::audio::FramePos(-100.1).toLowerFrameBoundary()); EXPECT_EQ(mixxx::audio::FramePos(-101), mixxx::audio::FramePos(-100.9).toLowerFrameBoundary()); } + +TEST_F(FrameTest, UpperFrameBoundary) { + EXPECT_EQ(mixxx::audio::FramePos(0), mixxx::audio::FramePos(0).toUpperFrameBoundary()); + EXPECT_EQ(mixxx::audio::FramePos(1), mixxx::audio::FramePos(0.5).toUpperFrameBoundary()); + EXPECT_EQ(mixxx::audio::FramePos(1), mixxx::audio::FramePos(0.9).toUpperFrameBoundary()); + EXPECT_EQ(mixxx::audio::FramePos(100), mixxx::audio::FramePos(100).toUpperFrameBoundary()); + EXPECT_EQ(mixxx::audio::FramePos(101), mixxx::audio::FramePos(100.1).toUpperFrameBoundary()); + EXPECT_EQ(mixxx::audio::FramePos(101), mixxx::audio::FramePos(100.9).toUpperFrameBoundary()); + EXPECT_EQ(mixxx::audio::FramePos(-99), mixxx::audio::FramePos(-99.9).toUpperFrameBoundary()); + EXPECT_EQ(mixxx::audio::FramePos(-99), mixxx::audio::FramePos(-99.1).toUpperFrameBoundary()); + EXPECT_EQ(mixxx::audio::FramePos(-100), mixxx::audio::FramePos(-100.1).toUpperFrameBoundary()); + EXPECT_EQ(mixxx::audio::FramePos(-100), mixxx::audio::FramePos(-100.9).toUpperFrameBoundary()); +} + +TEST_F(FrameTest, NearestFrameBoundary) { + EXPECT_EQ(mixxx::audio::FramePos(0), mixxx::audio::FramePos(0).toNearestFrameBoundary()); + EXPECT_EQ(mixxx::audio::FramePos(1), mixxx::audio::FramePos(0.5).toNearestFrameBoundary()); + EXPECT_EQ(mixxx::audio::FramePos(1), mixxx::audio::FramePos(0.9).toNearestFrameBoundary()); + EXPECT_EQ(mixxx::audio::FramePos(100), mixxx::audio::FramePos(100).toNearestFrameBoundary()); + EXPECT_EQ(mixxx::audio::FramePos(100), mixxx::audio::FramePos(100.1).toNearestFrameBoundary()); + EXPECT_EQ(mixxx::audio::FramePos(101), mixxx::audio::FramePos(100.9).toNearestFrameBoundary()); + EXPECT_EQ(mixxx::audio::FramePos(-100), mixxx::audio::FramePos(-99.9).toNearestFrameBoundary()); + EXPECT_EQ(mixxx::audio::FramePos(-99), mixxx::audio::FramePos(-99.1).toNearestFrameBoundary()); + EXPECT_EQ(mixxx::audio::FramePos(-100), + mixxx::audio::FramePos(-100.1).toNearestFrameBoundary()); + EXPECT_EQ(mixxx::audio::FramePos(-101), + mixxx::audio::FramePos(-100.9).toNearestFrameBoundary()); +} diff --git a/src/track/beatmap.cpp b/src/track/beatmap.cpp index cf743f74f91..36125c83459 100644 --- a/src/track/beatmap.cpp +++ b/src/track/beatmap.cpp @@ -32,7 +32,7 @@ inline Beat beatFromFramePos(mixxx::audio::FramePos beatPosition) { return beat; } -bool BeatLessThan(const Beat& beat1, const Beat& beat2) { +inline bool beatLessThan(const Beat& beat1, const Beat& beat2) { return beat1.frame_position() < beat2.frame_position(); } @@ -315,11 +315,11 @@ audio::FramePos BeatMap::findNthBeat(audio::FramePos position, int n) const { return audio::kInvalidFramePos; } - Beat beat = beatFromFramePos(position); + 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); + 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. @@ -401,11 +401,11 @@ bool BeatMap::findPrevNextBeats(audio::FramePos position, return false; } - Beat beat = beatFromFramePos(position); + 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); + 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. @@ -481,15 +481,19 @@ std::unique_ptr BeatMap::findBeats( return std::unique_ptr(); } - Beat startBeat = beatFromFramePos(startPosition); - Beat endBeat = beatFromFramePos(endPosition); + // Beats can only appear a full frame positions. If the start position is + // fractional, it needs to be rounded up to avoid finding a beat that + // appears *before* the requested start position. For the end position, the + // opposite applies, i.e. we need to round down to avoid finding a beat + // *after* the requested end position. + Beat startBeat = beatFromFramePos(startPosition.toUpperFrameBoundary()); + Beat endBeat = beatFromFramePos(endPosition.toLowerFrameBoundary()); BeatList::const_iterator curBeat = - std::lower_bound(m_beats.constBegin(), m_beats.constEnd(), - startBeat, BeatLessThan); + std::lower_bound(m_beats.constBegin(), m_beats.constEnd(), startBeat, beatLessThan); BeatList::const_iterator lastBeat = - std::upper_bound(m_beats.constBegin(), m_beats.constEnd(), endBeat, BeatLessThan); + std::upper_bound(m_beats.constBegin(), m_beats.constEnd(), endBeat, beatLessThan); if (curBeat >= lastBeat) { return std::unique_ptr(); @@ -503,8 +507,14 @@ bool BeatMap::hasBeatInRange(audio::FramePos startPosition, audio::FramePos endP startPosition > endPosition) { return false; } - audio::FramePos beatPosition = findNextBeat(startPosition); - if (beatPosition <= endPosition) { + audio::FramePos beatPosition = findNextBeat(startPosition.toUpperFrameBoundary()); + + // FIXME: The following assertion should always hold true, but it doesn't, + // because the position matching in findNthBeat() is fuzzy. This should be + // resolved, and moved to the calling code, to only use fuzzy matches when + // actually desired. + // DEBUG_ASSERT(!beatPosition.isValid() || beatPosition >= startPosition.toUpperFrameBoundary()); + if (beatPosition.isValid() && beatPosition <= endPosition.toLowerFrameBoundary()) { return true; } return false;