Add FIXMEs for use of bpm.value() without checking

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);

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;

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:

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,41 +522,47 @@ 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());
 }
 
 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());
 }
 
 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());
 }
 
 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());
 }
 
 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());
 }
 
 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());
 }
 

src/library/export/engineprimeexportjob.cpp

@@ -110,6 +110,55 @@ std::optional<djinterop::track> getTrackByRelativePath(
     }
 }
 
+bool tryGetBeatgrid(BeatsPointer pBeats,
+        mixxx::audio::FramePos cuePlayPos,
+        int64_t frameCount,
+        std::vector<djinterop::beatgrid_marker>* 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<djinterop::beatgrid_marker> 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<TrackId, int64_t>* 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<djinterop::beatgrid_marker> beatgrid{
-                    {0, firstBarAlignedBeatPlayPos.value()},
-                    {numBeats, lastBeatPlayPos.value()}};
-            beatgrid = el::normalize_beatgrid(std::move(beatgrid), frameCount);
+        std::vector<djinterop::beatgrid_marker> 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);

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<mixxx::audio::FramePos>& 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));

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<SeratoBeatGridTerminalMarker>(
             positionSecs - timingOffsetSecs, static_cast<float>(bpm.value())));
     setNonTerminalMarkers(nonTerminalMarkers);