diff --git a/src/test/beatmaptest.cpp b/src/test/beatmaptest.cpp index 96e2e1940ab9..e053c8748663 100644 --- a/src/test/beatmaptest.cpp +++ b/src/test/beatmaptest.cpp @@ -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 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 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()); diff --git a/src/track/beatmap.cpp b/src/track/beatmap.cpp index 36125c834590..d3418bd3d526 100644 --- a/src/track/beatmap.cpp +++ b/src/track/beatmap.cpp @@ -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,