From 883cea184f8ac9f7260d614a6bef6ccbd6a36b94 Mon Sep 17 00:00:00 2001 From: Jan Holthuis Date: Fri, 15 Oct 2021 12:22:08 +0200 Subject: [PATCH 1/4] Beats: Refactor getBpm() into getBpmInRange(startPos, endPos) This lays some groundwork for a future, sparse representation of beats in a track. At the moment, we have two kinds of beats implementations: 1. The `BeatMap` representation is simply a list of every single beat position in a track. Hence, changing tempos are supported, but memory intensive to work with. 2. The `BeatGrid` representation is actually sparse (only stores the offset and bpm instead of actual beat positions), but only supports a single constant tempo. Both are not sufficient. A future implementation should be both sparse and support multiple tempos per track. To make this possible, we need to know the track duration for calculating the average BPM of a track. There are two ways to handle this: We could either store the track duration in a beats object, or we change the `getBpm()` method signature to require passing the position range of the track that we want to know the BPM for. This commit implements the latter solution, because it's fully backwards-compatible and - in contrast to the former - can be implemented *without* modifying the protobuf format of the `BeatGrid` class (which currently doesn't store the track end position). Related Zulip discussion: https://mixxx.zulipchat.com/#narrow/stream/109171-development/topic/beat.20grid.20revamp.20re.3A.20pr.20.232961/near/251702242 --- src/analyzer/analyzerbeats.cpp | 13 +++++- src/engine/controls/bpmcontrol.cpp | 13 ++++-- src/engine/sync/synccontrol.cpp | 2 +- src/library/dao/trackdao.cpp | 5 ++- src/library/dlgtrackinfo.cpp | 21 +++++++--- src/test/beatgridtest.cpp | 35 ++++++++++++---- src/test/beatmaptest.cpp | 29 +++++++++---- src/track/beatgrid.cpp | 12 +++++- src/track/beatgrid.h | 3 +- src/track/beatmap.cpp | 67 +++++++++++++++--------------- src/track/beatmap.h | 10 ++--- src/track/beats.h | 7 ++-- src/track/track.cpp | 21 +++++++--- 13 files changed, 157 insertions(+), 81 deletions(-) diff --git a/src/analyzer/analyzerbeats.cpp b/src/analyzer/analyzerbeats.cpp index 57c5cff8744..b662b33c37f 100644 --- a/src/analyzer/analyzerbeats.cpp +++ b/src/analyzer/analyzerbeats.cpp @@ -145,7 +145,10 @@ bool AnalyzerBeats::shouldAnalyze(TrackPointer pTrack) const { if (!pBeats) { return true; } - if (!pBeats->getBpm().isValid()) { + if (!pBeats->getBpmInRange(mixxx::audio::kStartFramePos, + mixxx::audio::FramePos{ + pTrack->getDuration() * pBeats->getSampleRate()}) + .isValid()) { // Tracks with an invalid bpm <= 0 should be re-analyzed, // independent of the preference settings. We expect that // all tracks have a bpm > 0 when analyzed. Users that want @@ -230,7 +233,13 @@ void AnalyzerBeats::storeResults(TrackPointer pTrack) { m_bPreferencesFixedTempo, m_sampleRate); qDebug() << "AnalyzerBeats plugin detected" << beats.size() - << "beats. Average BPM:" << (pBeats ? pBeats->getBpm() : mixxx::Bpm()); + << "beats. Average BPM:" + << (pBeats ? pBeats->getBpmInRange( + mixxx::audio::kStartFramePos, + mixxx::audio::FramePos{ + pTrack->getDuration() * + pBeats->getSampleRate()}) + : mixxx::Bpm()); } else { mixxx::Bpm bpm = m_pPlugin->getBpm(); qDebug() << "AnalyzerBeats plugin detected constant BPM: " << bpm; diff --git a/src/engine/controls/bpmcontrol.cpp b/src/engine/controls/bpmcontrol.cpp index bc3383ae10a..80f0a544872 100644 --- a/src/engine/controls/bpmcontrol.cpp +++ b/src/engine/controls/bpmcontrol.cpp @@ -157,7 +157,8 @@ void BpmControl::adjustBeatsBpm(double deltaBpm) { return; } - mixxx::Bpm bpm = pBeats->getBpm(); + const mixxx::Bpm bpm = pBeats->getBpmInRange( + mixxx::audio::kStartFramePos, frameInfo().trackEndPosition); // FIXME: calling bpm.value() without checking bpm.isValid() const auto centerBpm = mixxx::Bpm(math_max(kBpmAdjustMin, bpm.value() + deltaBpm)); mixxx::Bpm adjustedBpm = BeatUtils::roundBpmWithinRange( @@ -956,7 +957,10 @@ void BpmControl::trackLoaded(TrackPointer pNewTrack) { void BpmControl::trackBeatsUpdated(mixxx::BeatsPointer pBeats) { if (kLogger.traceEnabled()) { kLogger.trace() << getGroup() << "BpmControl::trackBeatsUpdated" - << (pBeats ? pBeats->getBpm() : mixxx::Bpm()); + << (pBeats ? pBeats->getBpmInRange( + mixxx::audio::kStartFramePos, + frameInfo().trackEndPosition) + : mixxx::Bpm()); } m_pBeats = pBeats; updateLocalBpm(); @@ -1009,10 +1013,11 @@ mixxx::Bpm BpmControl::updateLocalBpm() { mixxx::Bpm prevLocalBpm = mixxx::Bpm(m_pLocalBpm->get()); mixxx::Bpm localBpm; const mixxx::BeatsPointer pBeats = m_pBeats; + const FrameInfo info = frameInfo(); if (pBeats) { - localBpm = pBeats->getBpmAroundPosition(frameInfo().currentPosition, kLocalBpmSpan); + localBpm = pBeats->getBpmAroundPosition(info.currentPosition, kLocalBpmSpan); if (!localBpm.isValid()) { - localBpm = pBeats->getBpm(); + localBpm = pBeats->getBpmInRange(mixxx::audio::kStartFramePos, info.trackEndPosition); } } if (localBpm != prevLocalBpm) { diff --git a/src/engine/sync/synccontrol.cpp b/src/engine/sync/synccontrol.cpp index ec0bd4100ee..d42d31cfaaa 100644 --- a/src/engine/sync/synccontrol.cpp +++ b/src/engine/sync/synccontrol.cpp @@ -622,7 +622,7 @@ void SyncControl::reportPlayerSpeed(double speed, bool scratching) { mixxx::Bpm SyncControl::fileBpm() const { mixxx::BeatsPointer pBeats = m_pBeats; if (pBeats) { - return pBeats->getBpm(); + return pBeats->getBpmInRange(mixxx::audio::kStartFramePos, frameInfo().trackEndPosition); } return {}; } diff --git a/src/library/dao/trackdao.cpp b/src/library/dao/trackdao.cpp index f10914bfcc4..09e9c51d1bd 100644 --- a/src/library/dao/trackdao.cpp +++ b/src/library/dao/trackdao.cpp @@ -594,7 +594,10 @@ void bindTrackLibraryValues( beatsBlob = pBeats->toByteArray(); beatsVersion = pBeats->getVersion(); beatsSubVersion = pBeats->getSubVersion(); - bpm = pBeats->getBpm(); + const auto trackEndPosition = mixxx::audio::FramePos{ + trackMetadata.getStreamInfo().getDuration().toDoubleSeconds() * + pBeats->getSampleRate()}; + bpm = pBeats->getBpmInRange(mixxx::audio::kStartFramePos, trackEndPosition); } const double bpmValue = bpm.isValid() ? bpm.value() : mixxx::Bpm::kValueUndefined; pTrackLibraryQuery->bindValue(":bpm", bpmValue); diff --git a/src/library/dlgtrackinfo.cpp b/src/library/dlgtrackinfo.cpp index c33d3e7b366..c94cc33c0e6 100644 --- a/src/library/dlgtrackinfo.cpp +++ b/src/library/dlgtrackinfo.cpp @@ -367,9 +367,15 @@ void DlgTrackInfo::updateTrackMetadataFields() { } void DlgTrackInfo::updateSpinBpmFromBeats() { - const auto bpmValue = m_pBeatsClone - ? m_pBeatsClone->getBpm().valueOr(mixxx::Bpm::kValueUndefined) - : mixxx::Bpm::kValueUndefined; + auto bpmValue = mixxx::Bpm::kValueUndefined; + if (m_pLoadedTrack && m_pBeatsClone) { + const auto trackEndPosition = mixxx::audio::FramePos{ + m_pLoadedTrack->getDuration() * m_pBeatsClone->getSampleRate()}; + bpmValue = m_pBeatsClone + ->getBpmInRange(mixxx::audio::kStartFramePos, + trackEndPosition) + .valueOr(mixxx::Bpm::kValueUndefined); + } spinBpm->setValue(bpmValue); } @@ -607,9 +613,12 @@ void DlgTrackInfo::slotSpinBpmValueChanged(double value) { bpm); } - if (m_pBeatsClone) { - const mixxx::Bpm oldValue = m_pBeatsClone->getBpm(); - if (oldValue == bpm) { + if (m_pLoadedTrack && m_pBeatsClone) { + const auto trackEndPosition = mixxx::audio::FramePos{ + m_pLoadedTrack->getDuration() * m_pBeatsClone->getSampleRate()}; + const mixxx::Bpm oldBpm = m_pBeatsClone->getBpmInRange( + mixxx::audio::kStartFramePos, trackEndPosition); + if (oldBpm == bpm) { return; } m_pBeatsClone = m_pBeatsClone->trySetBpm(bpm).value_or(m_pBeatsClone); diff --git a/src/test/beatgridtest.cpp b/src/test/beatgridtest.cpp index 2b28a5bb23a..b57736576d3 100644 --- a/src/test/beatgridtest.cpp +++ b/src/test/beatgridtest.cpp @@ -32,25 +32,40 @@ TEST(BeatGridTest, Scale) { auto pGrid = Beats::fromConstTempo(pTrack->getSampleRate(), mixxx::audio::kStartFramePos, mixxx::Bpm(bpm)); + const auto trackEndPosition = audio::FramePos{pTrack->getDuration() * pGrid->getSampleRate()}; - EXPECT_DOUBLE_EQ(bpm.value(), pGrid->getBpm().value()); + EXPECT_DOUBLE_EQ(bpm.value(), + pGrid->getBpmInRange(audio::kStartFramePos, trackEndPosition) + .value()); pGrid = *pGrid->tryScale(Beats::BpmScale::Double); - EXPECT_DOUBLE_EQ(2 * bpm.value(), pGrid->getBpm().value()); + EXPECT_DOUBLE_EQ(2 * bpm.value(), + pGrid->getBpmInRange(audio::kStartFramePos, trackEndPosition) + .value()); pGrid = *pGrid->tryScale(Beats::BpmScale::Halve); - EXPECT_DOUBLE_EQ(bpm.value(), pGrid->getBpm().value()); + EXPECT_DOUBLE_EQ(bpm.value(), + pGrid->getBpmInRange(audio::kStartFramePos, trackEndPosition) + .value()); pGrid = *pGrid->tryScale(Beats::BpmScale::TwoThirds); - EXPECT_DOUBLE_EQ(bpm.value() * 2 / 3, pGrid->getBpm().value()); + EXPECT_DOUBLE_EQ(bpm.value() * 2 / 3, + pGrid->getBpmInRange(audio::kStartFramePos, trackEndPosition) + .value()); pGrid = *pGrid->tryScale(Beats::BpmScale::ThreeHalves); - EXPECT_DOUBLE_EQ(bpm.value(), pGrid->getBpm().value()); + EXPECT_DOUBLE_EQ(bpm.value(), + pGrid->getBpmInRange(audio::kStartFramePos, trackEndPosition) + .value()); pGrid = *pGrid->tryScale(Beats::BpmScale::ThreeFourths); - EXPECT_DOUBLE_EQ(bpm.value() * 3 / 4, pGrid->getBpm().value()); + EXPECT_DOUBLE_EQ(bpm.value() * 3 / 4, + pGrid->getBpmInRange(audio::kStartFramePos, trackEndPosition) + .value()); pGrid = *pGrid->tryScale(Beats::BpmScale::FourThirds); - EXPECT_DOUBLE_EQ(bpm.value(), pGrid->getBpm().value()); + EXPECT_DOUBLE_EQ(bpm.value(), + pGrid->getBpmInRange(audio::kStartFramePos, trackEndPosition) + .value()); } TEST(BeatGridTest, TestNthBeatWhenOnBeat) { @@ -149,7 +164,11 @@ TEST(BeatGridTest, FromMetadata) { EXPECT_DOUBLE_EQ(pTrack->getBpm(), bpm.value()); auto pBeats = pTrack->getBeats(); - EXPECT_DOUBLE_EQ(pBeats->getBpm().value(), bpm.value()); + const auto trackEndPosition = audio::FramePos{pTrack->getDuration() * pBeats->getSampleRate()}; + EXPECT_DOUBLE_EQ( + pBeats->getBpmInRange(audio::kStartFramePos, trackEndPosition) + .value(), + bpm.value()); // Invalid bpm resets the bpm ASSERT_TRUE(pTrack->trySetBpm(-60.1)); diff --git a/src/test/beatmaptest.cpp b/src/test/beatmaptest.cpp index 84c316a29be..9b8c2936d2c 100644 --- a/src/test/beatmaptest.cpp +++ b/src/test/beatmaptest.cpp @@ -61,25 +61,40 @@ TEST_F(BeatMapTest, Scale) { QVector beats = createBeatVector(startOffsetFrames, numBeats, beatLengthFrames); auto pMap = Beats::fromBeatPositions(m_pTrack->getSampleRate(), beats); + const auto trackEndPosition = audio::FramePos{m_pTrack->getDuration() * pMap->getSampleRate()}; - EXPECT_DOUBLE_EQ(bpm.value(), pMap->getBpm().value()); + EXPECT_DOUBLE_EQ(bpm.value(), + pMap->getBpmInRange(audio::kStartFramePos, trackEndPosition) + .value()); pMap = *pMap->tryScale(Beats::BpmScale::Double); - EXPECT_DOUBLE_EQ(2 * bpm.value(), pMap->getBpm().value()); + EXPECT_DOUBLE_EQ(2 * bpm.value(), + pMap->getBpmInRange(audio::kStartFramePos, trackEndPosition) + .value()); pMap = *pMap->tryScale(Beats::BpmScale::Halve); - EXPECT_DOUBLE_EQ(bpm.value(), pMap->getBpm().value()); + EXPECT_DOUBLE_EQ(bpm.value(), + pMap->getBpmInRange(audio::kStartFramePos, trackEndPosition) + .value()); pMap = *pMap->tryScale(Beats::BpmScale::TwoThirds); - EXPECT_DOUBLE_EQ(bpm.value() * 2 / 3, pMap->getBpm().value()); + EXPECT_DOUBLE_EQ(bpm.value() * 2 / 3, + pMap->getBpmInRange(audio::kStartFramePos, trackEndPosition) + .value()); pMap = *pMap->tryScale(Beats::BpmScale::ThreeHalves); - EXPECT_DOUBLE_EQ(bpm.value(), pMap->getBpm().value()); + EXPECT_DOUBLE_EQ(bpm.value(), + pMap->getBpmInRange(audio::kStartFramePos, trackEndPosition) + .value()); pMap = *pMap->tryScale(Beats::BpmScale::ThreeFourths); - EXPECT_DOUBLE_EQ(bpm.value() * 3 / 4, pMap->getBpm().value()); + EXPECT_DOUBLE_EQ(bpm.value() * 3 / 4, + pMap->getBpmInRange(audio::kStartFramePos, trackEndPosition) + .value()); pMap = *pMap->tryScale(Beats::BpmScale::FourThirds); - EXPECT_DOUBLE_EQ(bpm.value(), pMap->getBpm().value()); + EXPECT_DOUBLE_EQ(bpm.value(), + pMap->getBpmInRange(audio::kStartFramePos, trackEndPosition) + .value()); } TEST_F(BeatMapTest, TestNthBeat) { diff --git a/src/track/beatgrid.cpp b/src/track/beatgrid.cpp index ef70be7b80a..24685321a9c 100644 --- a/src/track/beatgrid.cpp +++ b/src/track/beatgrid.cpp @@ -247,7 +247,11 @@ bool BeatGrid::hasBeatInRange(audio::FramePos startPosition, audio::FramePos end return false; } -mixxx::Bpm BeatGrid::getBpm() const { +mixxx::Bpm BeatGrid::getBpmInRange( + audio::FramePos startPosition, audio::FramePos endPosition) const { + Q_UNUSED(startPosition); + Q_UNUSED(endPosition); + if (!isValid()) { return {}; } @@ -258,7 +262,11 @@ mixxx::Bpm BeatGrid::getBpm() const { mixxx::Bpm BeatGrid::getBpmAroundPosition(audio::FramePos position, int n) const { Q_UNUSED(position); Q_UNUSED(n); - return getBpm(); + + if (!isValid()) { + return {}; + } + return bpm(); } std::optional BeatGrid::tryTranslate(audio::FrameDiff_t offset) const { diff --git a/src/track/beatgrid.h b/src/track/beatgrid.h index 455287f49ac..1a9e0f96df0 100644 --- a/src/track/beatgrid.h +++ b/src/track/beatgrid.h @@ -51,7 +51,8 @@ class BeatGrid final : public Beats { std::unique_ptr findBeats(audio::FramePos startPosition, audio::FramePos endPosition) const override; bool hasBeatInRange(audio::FramePos startPosition, audio::FramePos endPosition) const override; - mixxx::Bpm getBpm() const override; + mixxx::Bpm getBpmInRange(audio::FramePos startPosition, + audio::FramePos endPosition) const override; mixxx::Bpm getBpmAroundPosition(audio::FramePos position, int n) const override; audio::SampleRate getSampleRate() const override { diff --git a/src/track/beatmap.cpp b/src/track/beatmap.cpp index 026a55d4dfb..9f115da0a68 100644 --- a/src/track/beatmap.cpp +++ b/src/track/beatmap.cpp @@ -10,6 +10,7 @@ #include #include #include +#include #include "track/beatutils.h" #include "track/track.h" @@ -128,22 +129,6 @@ void scaleFourth(BeatList* pBeats) { } } -mixxx::Bpm calculateNominalBpm(const BeatList& beats, mixxx::audio::SampleRate sampleRate) { - QVector beatvect; - beatvect.reserve(beats.size()); - for (const auto& beat : beats) { - if (beat.enabled()) { - beatvect.append(mixxx::audio::FramePos(beat.frame_position())); - } - } - - if (beatvect.size() < 2) { - return {}; - } - - return BeatUtils::calculateBpm(beatvect, mixxx::audio::SampleRate(sampleRate)); -} - } // namespace namespace mixxx { @@ -181,31 +166,27 @@ BeatMap::BeatMap( MakeSharedTag, audio::SampleRate sampleRate, const QString& subVersion, - BeatList beats, - mixxx::Bpm nominalBpm) + BeatList beats) : m_subVersion(subVersion), m_sampleRate(sampleRate), - m_nominalBpm(nominalBpm), m_beats(std::move(beats)) { } BeatMap::BeatMap( MakeSharedTag, const BeatMap& other, - BeatList beats, - mixxx::Bpm nominalBpm) + BeatList beats) : BeatMap( MakeSharedTag{}, other.m_sampleRate, other.m_subVersion, - std::move(beats), - nominalBpm) { + std::move(beats)) { } BeatMap::BeatMap( MakeSharedTag, const BeatMap& other) - : BeatMap(MakeSharedTag{}, other, other.m_beats, other.m_nominalBpm) { + : BeatMap(MakeSharedTag{}, other, other.m_beats) { } // static @@ -213,7 +194,6 @@ BeatsPointer BeatMap::fromByteArray( audio::SampleRate sampleRate, const QString& subVersion, const QByteArray& byteArray) { - auto nominalBpm = mixxx::Bpm(); BeatList beatList; track::io::BeatMap map; @@ -222,12 +202,11 @@ BeatsPointer BeatMap::fromByteArray( const Beat& beat = map.beat(i); beatList.append(beat); } - nominalBpm = calculateNominalBpm(beatList, sampleRate); } else { qDebug() << "ERROR: Could not parse BeatMap from QByteArray of size" << byteArray.size(); } - return std::make_shared(MakeSharedTag{}, sampleRate, subVersion, beatList, nominalBpm); + return std::make_shared(MakeSharedTag{}, sampleRate, subVersion, beatList); } // static @@ -258,8 +237,7 @@ BeatsPointer BeatMap::makeBeatMap( beatList.append(beat); previousBeatPos = beatPos; } - const auto nominalBpm = calculateNominalBpm(beatList, sampleRate); - return std::make_shared(MakeSharedTag{}, sampleRate, subVersion, beatList, nominalBpm); + return std::make_shared(MakeSharedTag{}, sampleRate, subVersion, beatList); } QByteArray BeatMap::toByteArray() const { @@ -478,11 +456,33 @@ bool BeatMap::hasBeatInRange(audio::FramePos startPosition, audio::FramePos endP return false; } -mixxx::Bpm BeatMap::getBpm() const { +mixxx::Bpm BeatMap::getBpmInRange( + audio::FramePos startPosition, audio::FramePos endPosition) const { if (!isValid()) { return {}; } - return m_nominalBpm; + + const Beat startBeat = beatFromFramePos(startPosition.toUpperFrameBoundary()); + const Beat endBeat = beatFromFramePos(endPosition.toLowerFrameBoundary()); + + BeatList::const_iterator start = std::lower_bound( + m_beats.constBegin(), m_beats.constEnd(), startBeat, beatLessThan); + const BeatList::const_iterator end = std::upper_bound( + m_beats.constBegin(), m_beats.constEnd(), endBeat, beatLessThan); + + const int numBeats = std::distance(start, end); + if (numBeats < 2) { + return {}; + } + + QVector beats; + beats.reserve(numBeats); + std::transform(start, end, std::back_inserter(beats), [](const Beat& beat) -> audio::FramePos { + return audio::FramePos(beat.frame_position()); + }); + DEBUG_ASSERT(beats.size() == numBeats); + + return BeatUtils::calculateBpm(beats, m_sampleRate); } // Note: Also called from the engine thread @@ -552,7 +552,7 @@ std::optional BeatMap::tryTranslate(audio::FrameDiff_t offset) con } } - return std::make_shared(MakeSharedTag{}, *this, beats, m_nominalBpm); + return std::make_shared(MakeSharedTag{}, *this, beats); } std::optional BeatMap::tryScale(BpmScale scale) const { @@ -599,8 +599,7 @@ std::optional BeatMap::tryScale(BpmScale scale) const { return std::nullopt; } - mixxx::Bpm bpm = calculateNominalBpm(beats, m_sampleRate); - return std::make_shared(MakeSharedTag{}, *this, beats, bpm); + return std::make_shared(MakeSharedTag{}, *this, beats); } std::optional BeatMap::trySetBpm(mixxx::Bpm bpm) const { diff --git a/src/track/beatmap.h b/src/track/beatmap.h index 354211228c1..0b25ad956c7 100644 --- a/src/track/beatmap.h +++ b/src/track/beatmap.h @@ -53,7 +53,8 @@ class BeatMap final : public Beats { audio::FramePos endPosition) const override; bool hasBeatInRange(audio::FramePos startPosition, audio::FramePos endPosition) const override; - mixxx::Bpm getBpm() const override; + mixxx::Bpm getBpmInRange(audio::FramePos startPosition, + audio::FramePos endPosition) const override; mixxx::Bpm getBpmAroundPosition(audio::FramePos position, int n) const override; audio::SampleRate getSampleRate() const override { @@ -76,14 +77,12 @@ class BeatMap final : public Beats { MakeSharedTag, audio::SampleRate sampleRate, const QString& subVersion, - BeatList beats, - mixxx::Bpm nominalBpm); + BeatList beats); // Constructor to update the beat map BeatMap( MakeSharedTag, const BeatMap& other, - BeatList beats, - mixxx::Bpm nominalBpm); + BeatList beats); BeatMap( MakeSharedTag, const BeatMap& other); @@ -93,7 +92,6 @@ class BeatMap final : public Beats { const QString m_subVersion; const audio::SampleRate m_sampleRate; - const mixxx::Bpm m_nominalBpm; const BeatList m_beats; }; diff --git a/src/track/beats.h b/src/track/beats.h index 14774eb1f8a..28843705335 100644 --- a/src/track/beats.h +++ b/src/track/beats.h @@ -150,9 +150,10 @@ class Beats : private std::enable_shared_from_this { virtual bool hasBeatInRange(audio::FramePos startPosition, audio::FramePos endPosition) const = 0; - /// Return the average BPM over the entire track if the BPM is valid, - /// otherwise returns an invalid bpm value. - virtual mixxx::Bpm getBpm() const = 0; + /// Return the average BPM between `startPosition` and `endPosition` if the BPM is valid, + /// otherwise returns an invalid BPM value. + virtual mixxx::Bpm getBpmInRange(audio::FramePos startPosition, + audio::FramePos endPosition) const = 0; /// Return the average BPM over the range of n*2 beats centered around /// frame position `position`. For example, n=4 results in an averaging of 8 beats. diff --git a/src/track/track.cpp b/src/track/track.cpp index e455da98553..70ec47191ec 100644 --- a/src/track/track.cpp +++ b/src/track/track.cpp @@ -49,8 +49,13 @@ inline bool compareAndSet(T* pField, T&& value) { } inline mixxx::Bpm getBeatsPointerBpm( - const mixxx::BeatsPointer& pBeats) { - return pBeats ? mixxx::Bpm{pBeats->getBpm()} : mixxx::Bpm{}; + const mixxx::BeatsPointer& pBeats, double durationSecs) { + if (!pBeats) { + return mixxx::Bpm{}; + } + + const auto trackEndPosition = mixxx::audio::FramePos{durationSecs * pBeats->getSampleRate()}; + return pBeats->getBpmInRange(mixxx::audio::kStartFramePos, trackEndPosition); } } // anonymous namespace @@ -166,7 +171,10 @@ void Track::replaceMetadataFromSource( // Need to set BPM after sample rate since beat grid creation depends on // knowing the sample rate. Bug #1020438. auto beatsAndBpmModified = false; - if (importedBpm.isValid() && (!m_pBeats || !m_pBeats->getBpm().isValid())) { + if (importedBpm.isValid() && + (!m_pBeats || + !getBeatsPointerBpm(m_pBeats, getDuration()) + .isValid())) { // Only use the imported BPM if the current beat grid is either // missing or not valid! The BPM value in the metadata might be // imprecise (normalized or rounded), e.g. ID3v2 only supports @@ -338,7 +346,8 @@ void Track::adjustReplayGainFromPregain(double gain) { mixxx::Bpm Track::getBpmWhileLocked() const { // BPM values must be synchronized at all times! - DEBUG_ASSERT(m_record.getMetadata().getTrackInfo().getBpm() == getBeatsPointerBpm(m_pBeats)); + DEBUG_ASSERT(m_record.getMetadata().getTrackInfo().getBpm() == + getBeatsPointerBpm(m_pBeats, getDuration())); return m_record.getMetadata().getTrackInfo().getBpm(); } @@ -357,7 +366,7 @@ bool Track::trySetBpmWhileLocked(mixxx::Bpm bpm) { cuePosition, bpm); return trySetBeatsWhileLocked(std::move(pBeats)); - } else if (m_pBeats->getBpm() != bpm) { + } else if (getBeatsPointerBpm(m_pBeats, getDuration()) != bpm) { // Continue with the regular cases const auto newBeats = m_pBeats->trySetBpm(bpm); if (newBeats) { @@ -401,7 +410,7 @@ bool Track::setBeatsWhileLocked(mixxx::BeatsPointer pBeats) { return false; } m_pBeats = std::move(pBeats); - m_record.refMetadata().refTrackInfo().setBpm(getBeatsPointerBpm(m_pBeats)); + m_record.refMetadata().refTrackInfo().setBpm(getBeatsPointerBpm(m_pBeats, getDuration())); return true; } From 300c160b465888a135331c7a0f31557e0b093ce6 Mon Sep 17 00:00:00 2001 From: Jan Holthuis Date: Wed, 20 Oct 2021 20:55:49 +0200 Subject: [PATCH 2/4] AnalyzerBeats: Fix wrong log message about detected BPM --- src/analyzer/analyzerbeats.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/analyzer/analyzerbeats.cpp b/src/analyzer/analyzerbeats.cpp index b662b33c37f..f3c0bd6846c 100644 --- a/src/analyzer/analyzerbeats.cpp +++ b/src/analyzer/analyzerbeats.cpp @@ -233,7 +233,7 @@ void AnalyzerBeats::storeResults(TrackPointer pTrack) { m_bPreferencesFixedTempo, m_sampleRate); qDebug() << "AnalyzerBeats plugin detected" << beats.size() - << "beats. Average BPM:" + << "beats. Predominant BPM:" << (pBeats ? pBeats->getBpmInRange( mixxx::audio::kStartFramePos, mixxx::audio::FramePos{ From 9638b4fb0a1423f087b498de08f88303cd858c37 Mon Sep 17 00:00:00 2001 From: Jan Holthuis Date: Wed, 20 Oct 2021 20:57:23 +0200 Subject: [PATCH 3/4] Beats: Fix wrong comment stating that getBpmInRage returns average BPM --- src/track/beats.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/track/beats.h b/src/track/beats.h index 28843705335..c6e9e1a9844 100644 --- a/src/track/beats.h +++ b/src/track/beats.h @@ -150,8 +150,8 @@ class Beats : private std::enable_shared_from_this { virtual bool hasBeatInRange(audio::FramePos startPosition, audio::FramePos endPosition) const = 0; - /// Return the average BPM between `startPosition` and `endPosition` if the BPM is valid, - /// otherwise returns an invalid BPM value. + /// Return the predominant BPM value between `startPosition` and `endPosition` + /// if the BPM is valid, otherwise returns an invalid BPM value. virtual mixxx::Bpm getBpmInRange(audio::FramePos startPosition, audio::FramePos endPosition) const = 0; From 8decc52558e8c8966e06093744a8c8d01d3951c8 Mon Sep 17 00:00:00 2001 From: Jan Holthuis Date: Wed, 20 Oct 2021 20:58:06 +0200 Subject: [PATCH 4/4] Beats: Clarify that getBpmAroundPosition returns the arithmetic average --- src/track/beats.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/track/beats.h b/src/track/beats.h index c6e9e1a9844..6f04f045921 100644 --- a/src/track/beats.h +++ b/src/track/beats.h @@ -155,7 +155,7 @@ class Beats : private std::enable_shared_from_this { virtual mixxx::Bpm getBpmInRange(audio::FramePos startPosition, audio::FramePos endPosition) const = 0; - /// Return the average BPM over the range of n*2 beats centered around + /// Return the arithmetic average BPM over the range of n*2 beats centered around /// frame position `position`. For example, n=4 results in an averaging of 8 beats. /// The returned Bpm value may be invalid. virtual mixxx::Bpm getBpmAroundPosition(audio::FramePos position, int n) const = 0;