diff --git a/src/engine/controls/bpmcontrol.cpp b/src/engine/controls/bpmcontrol.cpp index 5ffb7eb7166c..89037428ba19 100644 --- a/src/engine/controls/bpmcontrol.cpp +++ b/src/engine/controls/bpmcontrol.cpp @@ -174,6 +174,7 @@ void BpmControl::adjustBeatsBpm(double deltaBpm) { const mixxx::BeatsPointer pBeats = pTrack->getBeats(); if (pBeats && (pBeats->getCapabilities() & mixxx::Beats::BEATSCAP_SETBPM)) { mixxx::Bpm bpm = pBeats->getBpm(); + // FIXME: calling bpm.value() without checking bpm.isValid() const auto centerBpm = mixxx::Bpm(math_max(kBpmAdjustMin, bpm.value() + deltaBpm)); mixxx::Bpm adjustedBpm = BeatUtils::roundBpmWithinRange( centerBpm - kBpmAdjustStep / 2, centerBpm, centerBpm + kBpmAdjustStep / 2); diff --git a/src/engine/sync/synccontrol.cpp b/src/engine/sync/synccontrol.cpp index 9c498e36dd07..5e766381c7c6 100644 --- a/src/engine/sync/synccontrol.cpp +++ b/src/engine/sync/synccontrol.cpp @@ -511,6 +511,7 @@ void SyncControl::reportPlayerSpeed(double speed, bool scratching) { double SyncControl::fileBpm() const { mixxx::BeatsPointer pBeats = m_pBeats; if (pBeats) { + // FIXME: calling bpm.value() without checking bpm.isValid() return pBeats->getBpm().value(); } return mixxx::Bpm::kValueUndefined; diff --git a/src/library/basetracktablemodel.cpp b/src/library/basetracktablemodel.cpp index 81dc6d49b8ea..8e11f0e4fd29 100644 --- a/src/library/basetracktablemodel.cpp +++ b/src/library/basetracktablemodel.cpp @@ -782,6 +782,7 @@ QVariant BaseTrackTableModel::roleValue( case ColumnCache::COLUMN_LIBRARYTABLE_BPM: { bool ok; const auto bpmValue = rawValue.toDouble(&ok); + // FIXME: calling bpm.value() without checking bpm.isValid() return ok ? bpmValue : mixxx::Bpm().value(); } case ColumnCache::COLUMN_LIBRARYTABLE_TIMESPLAYED: diff --git a/src/library/dlgtrackinfo.cpp b/src/library/dlgtrackinfo.cpp index 1df9aa179e0e..6b011c73511e 100644 --- a/src/library/dlgtrackinfo.cpp +++ b/src/library/dlgtrackinfo.cpp @@ -369,6 +369,7 @@ void DlgTrackInfo::updateTrackMetadataFields() { void DlgTrackInfo::reloadTrackBeats(const Track& track) { m_pBeatsClone = track.getBeats(); if (m_pBeatsClone) { + // FIXME: calling bpm.value() without checking bpm.isValid() spinBpm->setValue(m_pBeatsClone->getBpm().value()); } else { spinBpm->setValue(0.0); @@ -521,6 +522,7 @@ void DlgTrackInfo::slotBpmDouble() { m_pBeatsClone = m_pBeatsClone->scale(mixxx::Beats::BpmScale::Double); // read back the actual value mixxx::Bpm newValue = m_pBeatsClone->getBpm(); + // FIXME: calling bpm.value() without checking bpm.isValid() spinBpm->setValue(newValue.value()); } @@ -528,6 +530,7 @@ void DlgTrackInfo::slotBpmHalve() { m_pBeatsClone = m_pBeatsClone->scale(mixxx::Beats::BpmScale::Halve); // read back the actual value const mixxx::Bpm newValue = m_pBeatsClone->getBpm(); + // FIXME: calling bpm.value() without checking bpm.isValid() spinBpm->setValue(newValue.value()); } @@ -535,6 +538,7 @@ void DlgTrackInfo::slotBpmTwoThirds() { m_pBeatsClone = m_pBeatsClone->scale(mixxx::Beats::BpmScale::TwoThirds); // read back the actual value const mixxx::Bpm newValue = m_pBeatsClone->getBpm(); + // FIXME: calling bpm.value() without checking bpm.isValid() spinBpm->setValue(newValue.value()); } @@ -542,6 +546,7 @@ void DlgTrackInfo::slotBpmThreeFourth() { m_pBeatsClone = m_pBeatsClone->scale(mixxx::Beats::BpmScale::ThreeFourths); // read back the actual value const mixxx::Bpm newValue = m_pBeatsClone->getBpm(); + // FIXME: calling bpm.value() without checking bpm.isValid() spinBpm->setValue(newValue.value()); } @@ -549,6 +554,7 @@ void DlgTrackInfo::slotBpmFourThirds() { m_pBeatsClone = m_pBeatsClone->scale(mixxx::Beats::BpmScale::FourThirds); // read back the actual value const mixxx::Bpm newValue = m_pBeatsClone->getBpm(); + // FIXME: calling bpm.value() without checking bpm.isValid() spinBpm->setValue(newValue.value()); } @@ -556,6 +562,7 @@ void DlgTrackInfo::slotBpmThreeHalves() { m_pBeatsClone = m_pBeatsClone->scale(mixxx::Beats::BpmScale::ThreeHalves); // read back the actual value const mixxx::Bpm newValue = m_pBeatsClone->getBpm(); + // FIXME: calling bpm.value() without checking bpm.isValid() spinBpm->setValue(newValue.value()); } diff --git a/src/library/export/engineprimeexportjob.cpp b/src/library/export/engineprimeexportjob.cpp index eab0acbc07da..60cb970dc189 100644 --- a/src/library/export/engineprimeexportjob.cpp +++ b/src/library/export/engineprimeexportjob.cpp @@ -110,6 +110,55 @@ std::optional getTrackByRelativePath( } } +bool tryGetBeatgrid(BeatsPointer pBeats, + mixxx::audio::FramePos cuePlayPos, + int64_t frameCount, + std::vector* pBeatgrid) { + if (!cuePlayPos.isValid()) { + return false; + } + + // For now, assume a constant average BPM across the whole track. + // Note that Mixxx does not (currently) store any information about + // which beat of a bar a given beat represents. As such, in order to + // make sure we have the right phrasing, assume that the main cue point + // starts at the beginning of a bar, then move backwards towards the + // beginning of the track in 4-beat decrements to find the first beat + // in the track that also aligns with the start of a bar. + const auto firstBeatPlayPos = pBeats->firstBeat(); + const auto cueBeatPlayPos = pBeats->findClosestBeat(cuePlayPos); + if (!firstBeatPlayPos.isValid() || !cueBeatPlayPos.isValid()) { + return false; + } + + int numBeatsToCue = pBeats->numBeatsInRange(firstBeatPlayPos, cueBeatPlayPos); + const auto firstBarAlignedBeatPlayPos = pBeats->findNBeatsFromPosition( + cueBeatPlayPos, numBeatsToCue & ~0x3); + if (!firstBarAlignedBeatPlayPos.isValid()) { + return false; + } + + // We will treat the first bar-aligned beat as beat zero. Find the + // number of pBeats from there until the end of the track in order to + // correctly assign an index for the last beat. + const auto lastBeatPlayPos = pBeats->findPrevBeat(mixxx::audio::kStartFramePos + frameCount); + if (!lastBeatPlayPos.isValid()) { + return false; + } + + int numBeats = pBeats->numBeatsInRange(firstBarAlignedBeatPlayPos, lastBeatPlayPos); + if (numBeats <= 0) { + return false; + } + + std::vector beatgrid{ + {0, firstBarAlignedBeatPlayPos.value()}, + {numBeats, lastBeatPlayPos.value()}}; + beatgrid = el::normalize_beatgrid(std::move(beatgrid), frameCount); + pBeatgrid->assign(std::begin(beatgrid), std::end(beatgrid)); + return true; +} + void exportMetadata(djinterop::database* pDatabase, QHash* pMixxxToEnginePrimeTrackIdMap, TrackPointer pTrack, @@ -174,38 +223,17 @@ void exportMetadata(djinterop::database* pDatabase, snapshot.default_main_cue = cuePlayPosValue; snapshot.adjusted_main_cue = cuePlayPosValue; - // Fill in beat grid. For now, assume a constant average BPM across - // the whole track. Note that points in the track are specified as - // "play positions", which are twice the sample offset. + // Fill in beat grid. BeatsPointer beats = pTrack->getBeats(); if (beats != nullptr) { - // Note that Mixxx does not (currently) store any information about - // which beat of a bar a given beat represents. As such, in order to - // make sure we have the right phrasing, assume that the main cue point - // starts at the beginning of a bar, then move backwards towards the - // beginning of the track in 4-beat decrements to find the first beat - // in the track that also aligns with the start of a bar. - const auto firstBeatPlayPos = beats->firstBeat(); - const auto cueBeatPlayPos = beats->findClosestBeat(cuePlayPos); - int numBeatsToCue = beats->numBeatsInRange(firstBeatPlayPos, cueBeatPlayPos); - const auto firstBarAlignedBeatPlayPos = beats->findNBeatsFromPosition( - cueBeatPlayPos, numBeatsToCue & ~0x3); - - // We will treat the first bar-aligned beat as beat zero. Find the - // number of beats from there until the end of the track in order to - // correctly assign an index for the last beat. - const auto lastBeatPlayPos = beats->findPrevBeat(mixxx::audio::kStartFramePos + frameCount); - int numBeats = beats->numBeatsInRange(firstBarAlignedBeatPlayPos, lastBeatPlayPos); - if (numBeats > 0) { - std::vector beatgrid{ - {0, firstBarAlignedBeatPlayPos.value()}, - {numBeats, lastBeatPlayPos.value()}}; - beatgrid = el::normalize_beatgrid(std::move(beatgrid), frameCount); + std::vector beatgrid; + if (tryGetBeatgrid(beats, cuePlayPos, frameCount, &beatgrid)) { snapshot.default_beatgrid = beatgrid; snapshot.adjusted_beatgrid = beatgrid; } else { - qWarning() << "Non-positive number of beats in beat data of track" << pTrack->getId() - << "(" << pTrack->getFileInfo().fileName() << ")"; + qWarning() << "Beats data exists but is invalid for track" + << pTrack->getId() << "(" + << pTrack->getFileInfo().fileName() << ")"; } } else { qInfo() << "No beats data found for track" << pTrack->getId() @@ -230,6 +258,12 @@ void exportMetadata(djinterop::database* pDatabase, continue; } + if (!pCue->getPosition().isValid()) { + qWarning() << "Hot cue" << hotCueIndex << "exists but is invalid for track" + << pTrack->getId() << "(" << pTrack->getFileInfo().fileName() << ")"; + continue; + } + QString label = pCue->getLabel(); if (label == "") { label = QString("Cue %1").arg(hotCueIndex + 1); diff --git a/src/track/beatutils.cpp b/src/track/beatutils.cpp index bae1e12a4d43..b98cfa67fa0c 100644 --- a/src/track/beatutils.cpp +++ b/src/track/beatutils.cpp @@ -325,6 +325,7 @@ mixxx::Bpm BeatUtils::makeConstBpm( mixxx::Bpm BeatUtils::roundBpmWithinRange( mixxx::Bpm minBpm, mixxx::Bpm centerBpm, mixxx::Bpm maxBpm) { // First try to snap to a full integer BPM + // FIXME: calling bpm.value() without checking bpm.isValid() auto snapBpm = mixxx::Bpm(round(centerBpm.value())); if (snapBpm > minBpm && snapBpm < maxBpm) { // Success @@ -387,6 +388,7 @@ mixxx::audio::FramePos BeatUtils::adjustPhase( mixxx::Bpm bpm, mixxx::audio::SampleRate sampleRate, const QVector& beats) { + // FIXME: calling bpm.value() without checking bpm.isValid() const double beatLength = 60 * sampleRate / bpm.value(); const mixxx::audio::FramePos startOffset = mixxx::audio::FramePos(fmod(firstBeat.value(), beatLength)); diff --git a/src/track/serato/beatgrid.cpp b/src/track/serato/beatgrid.cpp index a5161ef856cb..ab250c66f193 100644 --- a/src/track/serato/beatgrid.cpp +++ b/src/track/serato/beatgrid.cpp @@ -509,6 +509,7 @@ void SeratoBeatGrid::setBeats(BeatsPointer pBeats, currentBeatPositionFramesWithOffset.value()); const mixxx::Bpm bpm = pBeats->getBpmAroundPosition(currentBeatPositionFramesWithOffset, 1); + // FIXME: calling bpm.value() without checking bpm.isValid() setTerminalMarker(std::make_shared( positionSecs - timingOffsetSecs, static_cast(bpm.value()))); setNonTerminalMarkers(nonTerminalMarkers);