Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Beats: Refactor getBpm() into getBpmInRange(startPos, endPos) #4361

Merged
merged 4 commits into from
Oct 20, 2021
Merged

Beats: Refactor getBpm() into getBpmInRange(startPos, endPos) #4361

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
883cea1
Beats: Refactor getBpm() into getBpmInRange(startPos, endPos)
Holzhaus Oct 15, 2021
300c160
AnalyzerBeats: Fix wrong log message about detected BPM
Holzhaus Oct 20, 2021
9638b4f
Beats: Fix wrong comment stating that getBpmInRage returns average BPM
Holzhaus Oct 20, 2021
8decc52
Beats: Clarify that getBpmAroundPosition returns the arithmetic average
Holzhaus Oct 20, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 11 additions & 2 deletions src/analyzer/analyzerbeats.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -145,7 +145,10 @@ bool AnalyzerBeats::shouldAnalyze(TrackPointer pTrack) const {
if (!pBeats) {
return true;
}
if (!pBeats->getBpm().isValid()) {
if (!pBeats->getBpmInRange(mixxx::audio::kStartFramePos,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is now a very expensive check for a valid BPM. Maybe caching the nominal bpm is not such a bad idea.
What are the alternatives?

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
Expand Down Expand Up @@ -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. Predominant 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;
Expand Down
13 changes: 9 additions & 4 deletions src/engine/controls/bpmcontrol.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand Down Expand Up @@ -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();
Expand Down Expand Up @@ -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) {
Expand Down
2 changes: 1 addition & 1 deletion src/engine/sync/synccontrol.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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 {};
}
Expand Down
5 changes: 4 additions & 1 deletion src/library/dao/trackdao.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
21 changes: 15 additions & 6 deletions src/library/dlgtrackinfo.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}

Expand Down Expand Up @@ -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);
Expand Down
35 changes: 27 additions & 8 deletions src/test/beatgridtest.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down Expand Up @@ -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));
Expand Down
29 changes: 22 additions & 7 deletions src/test/beatmaptest.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -61,25 +61,40 @@ TEST_F(BeatMapTest, Scale) {
QVector<mixxx::audio::FramePos> 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) {
Expand Down
12 changes: 10 additions & 2 deletions src/track/beatgrid.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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 {};
}
Expand All @@ -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<BeatsPointer> BeatGrid::tryTranslate(audio::FrameDiff_t offset) const {
Expand Down
3 changes: 2 additions & 1 deletion src/track/beatgrid.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,8 @@ class BeatGrid final : public Beats {
std::unique_ptr<BeatIterator> 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 {
Expand Down
Loading