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.

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

Ensure that tracks with an invalid bpm are re-analyzed #2776

Merged
merged 2 commits into from
May 16, 2020
Merged
Changes from all commits
Commits
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
75 changes: 42 additions & 33 deletions src/analyzer/analyzerbeats.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -149,41 +149,50 @@ bool AnalyzerBeats::shouldAnalyze(TrackPointer tio) const {
// If the track already has a Beats object then we need to decide whether to
// analyze this track or not.
mixxx::BeatsPointer pBeats = tio->getBeats();
if (pBeats) {
QString version = pBeats->getVersion();
QString subVersion = pBeats->getSubVersion();
if (!pBeats) {
return true;
}
if (!mixxx::Bpm::isValidValue(pBeats->getBpm())) {
Copy link
Contributor

Choose a reason for hiding this comment

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

This is a logical change. Bpm::isValidValue checks if the BPM is < 0. The old code checks if the BPM == 0. Do we want <= 0?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

No, that's correct. It checks for value > kValueMin = 0.0, i.e. re-analyze if bpm <= 0.

// 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
// to keep their zero bpm tracks could lock them to prevent
// this re-analysis (see the check above).
qDebug() << "Re-analyzing track with invalid BPM despite preference settings.";
return true;
}
if (pBeats->findNextBeat(0) <= 0.0) {
qDebug() << "First beat is 0 for grid so analyzing track to find first beat.";
return true;
}

QHash<QString, QString> extraVersionInfo = getExtraVersionInfo(
pluginID,
m_bPreferencesFastAnalysis);
QString newVersion = BeatFactory::getPreferredVersion(
m_bPreferencesOffsetCorrection);
QString newSubVersion = BeatFactory::getPreferredSubVersion(
m_bPreferencesFixedTempo,
m_bPreferencesOffsetCorrection,
iMinBpm,
iMaxBpm,
extraVersionInfo);

if (version == newVersion && subVersion == newSubVersion) {
// If the version and settings have not changed then if the world is
// sane, re-analyzing will do nothing.
return false;
}
if (!m_bPreferencesReanalyzeOldBpm) {
return false;
}
if (pBeats->getBpm() == 0.0) {
qDebug() << "BPM is 0 for track so re-analyzing despite preference settings.";
} else if (pBeats->findNextBeat(0) <= 0.0) {
qDebug() << "First beat is 0 for grid so analyzing track to find first beat.";
} else {
qDebug() << "Beat calculation skips analyzing because the track has"
<< "a BPM computed by a previous Mixxx version and user"
<< "preferences indicate we should not change it.";
return false;
}
// Version check
QString version = pBeats->getVersion();
QString subVersion = pBeats->getSubVersion();
QHash<QString, QString> extraVersionInfo = getExtraVersionInfo(
pluginID,
m_bPreferencesFastAnalysis);
QString newVersion = BeatFactory::getPreferredVersion(
m_bPreferencesOffsetCorrection);
QString newSubVersion = BeatFactory::getPreferredSubVersion(
m_bPreferencesFixedTempo,
m_bPreferencesOffsetCorrection,
iMinBpm,
iMaxBpm,
extraVersionInfo);
if (version == newVersion && subVersion == newSubVersion) {
// If the version and settings have not changed then if the world is
// sane, re-analyzing will do nothing.
return false;
}
// Beat grid exists but version and settings differ
if (!m_bPreferencesReanalyzeOldBpm) {
qDebug() << "Beat calculation skips analyzing because the track has"
<< "a BPM computed by a previous Mixxx version and user"
<< "preferences indicate we should not change it.";
return false;
}

return true;
}

Expand Down