Cue Frame Refactoring

src/analyzer/analyzersilence.cpp

@@ -15,7 +15,7 @@ bool shouldAnalyze(TrackPointer pTrack) {
     CuePointer pOutroCue = pTrack->findCueByType(mixxx::CueType::Outro);
     CuePointer pAudibleSound = pTrack->findCueByType(mixxx::CueType::AudibleSound);
 
-    if (!pIntroCue || !pOutroCue || !pAudibleSound || pAudibleSound->getLength() <= 0) {
+    if (!pIntroCue || !pOutroCue || !pAudibleSound || pAudibleSound->getLengthFrames() <= 0) {
         return true;
     }
     return false;

src/audio/frame.h

@@ -22,6 +22,7 @@ class FramePos final {
     typedef double value_t;
     static constexpr value_t kStartValue = 0;
     static constexpr value_t kInvalidValue = std::numeric_limits<FramePos::value_t>::quiet_NaN();
+    static constexpr double kLegacyInvalidEnginePosition = -1.0;
 
     constexpr FramePos()
             : m_framePosition(kInvalidValue) {
@@ -31,14 +32,50 @@ class FramePos final {
             : m_framePosition(framePosition) {
     }
 
+    /// Return a `FramePos` from a given engine sample position. To catch
+    /// "invalid" positions (e.g. when parsing values from control objects),
+    /// use `FramePos::fromEngineSamplePosMaybeInvalid` instead.
     static constexpr FramePos fromEngineSamplePos(double engineSamplePos) {
         return FramePos(engineSamplePos / mixxx::kEngineChannelCount);
     }
 
+    /// Return an engine sample position. The `FramePos` is expected to be
+    /// valid. If invalid positions are possible (e.g. for control object
+    /// values), use `FramePos::toEngineSamplePosMaybeInvalid` instead.
     double toEngineSamplePos() const {
+        DEBUG_ASSERT(isValid());
         return value() * mixxx::kEngineChannelCount;
     }
 
+    /// Return a `FramePos` from a given engine sample position. Sample
+    /// positions that equal `kLegacyInvalidEnginePosition` are considered
+    /// invalid and result in an invalid `FramePos` instead.
+    ///
+    /// In general, using this method should be avoided and is only necessary
+    /// for compatiblity with our control objects and legacy parts of the code
+    /// base. Using a different code path based on the output of `isValid()` is
+    /// preferable.
+    static constexpr FramePos fromEngineSamplePosMaybeInvalid(double engineSamplePos) {
+        if (engineSamplePos == kLegacyInvalidEnginePosition) {
+            return {};
+        }
+        return fromEngineSamplePos(engineSamplePos);
+    }
+
+    /// Return an engine sample position. If the `FramePos` is invalid,
+    /// `kLegacyInvalidEnginePosition` is returned instad.
+    ///
+    /// In general, using this method should be avoided and is only necessary
+    /// for compatiblity with our control objects and legacy parts of the code
+    /// base. Using a different code path based on the output of `isValid()` is
+    /// preferable.
+    double toEngineSamplePosMaybeInvalid() const {
+        if (!isValid()) {
+            return kLegacyInvalidEnginePosition;
+        }
+        return toEngineSamplePos();
+    }
+
     /// Return true if the frame position is valid. Any finite value is
     /// considered valid, i.e. any value except NaN and negative/positive
     /// infinity.

src/engine/controls/cuecontrol.cpp

@@ -484,7 +484,7 @@ void CueControl::trackLoaded(TrackPointer pNewTrack) {
                 pNewTrack->findCueByType(mixxx::CueType::AudibleSound);
         double audibleSoundPosition = Cue::kNoPosition;
         if (pAudibleSound) {
-            audibleSoundPosition = pAudibleSound->getPosition();
+            audibleSoundPosition = pAudibleSound->getPosition().toEngineSamplePosMaybeInvalid();
         }
         if (audibleSoundPosition != Cue::kNoPosition) {
             seekOnLoad(audibleSoundPosition);
@@ -581,8 +581,8 @@ void CueControl::loadCuesFromTrack() {
             } else {
                 // If the old hotcue is the same, then we only need to update
                 Cue::StartAndEndPositions pos = pCue->getStartAndEndPosition();
-                pControl->setPosition(pos.startPosition);
-                pControl->setEndPosition(pos.endPosition);
+                pControl->setPosition(pos.startPosition.toEngineSamplePosMaybeInvalid());
+                pControl->setEndPosition(pos.endPosition.toEngineSamplePosMaybeInvalid());
                 pControl->setColor(pCue->getColor());
                 pControl->setType(pCue->getType());
             }
@@ -604,8 +604,8 @@ void CueControl::loadCuesFromTrack() {
     }
 
     if (pIntroCue) {
-        double startPosition = pIntroCue->getPosition();
-        double endPosition = pIntroCue->getEndPosition();
+        double startPosition = pIntroCue->getPosition().toEngineSamplePosMaybeInvalid();
+        double endPosition = pIntroCue->getEndPosition().toEngineSamplePosMaybeInvalid();
 
         m_pIntroStartPosition->set(quantizeCuePoint(startPosition));
         m_pIntroStartEnabled->forceSet(
@@ -621,8 +621,8 @@ void CueControl::loadCuesFromTrack() {
     }
 
     if (pOutroCue) {
-        double startPosition = pOutroCue->getPosition();
-        double endPosition = pOutroCue->getEndPosition();
+        double startPosition = pOutroCue->getPosition().toEngineSamplePosMaybeInvalid();
+        double endPosition = pOutroCue->getEndPosition().toEngineSamplePosMaybeInvalid();
 
         m_pOutroStartPosition->set(quantizeCuePoint(startPosition));
         m_pOutroStartEnabled->forceSet(
@@ -642,7 +642,7 @@ void CueControl::loadCuesFromTrack() {
     // The mixxx::CueType::MainCue from getCuePoints() has the priority
     CuePosition mainCuePoint;
     if (pMainCue) {
-        double position = pMainCue->getPosition();
+        double position = pMainCue->getPosition().toEngineSamplePosMaybeInvalid();
         mainCuePoint.setPosition(position);
         // adjust the track cue accordingly
         m_pLoadedTrack->setCuePoint(mainCuePoint);
@@ -899,7 +899,7 @@ void CueControl::hotcueGotoAndLoop(HotcueControl* pControl, double value) {
         return;
     }
 
-    double startPosition = pCue->getPosition();
+    double startPosition = pCue->getPosition().toEngineSamplePosMaybeInvalid();
     if (startPosition == Cue::kNoPosition) {
         return;
     }
@@ -933,10 +933,10 @@ void CueControl::hotcueCueLoop(HotcueControl* pControl, double value) {
 
     CuePointer pCue = pControl->getCue();
 
-    if (!pCue || pCue->getPosition() == Cue::kNoPosition) {
+    if (!pCue || !pCue->getPosition().isValid()) {
         hotcueSet(pControl, value, HotcueSetMode::Cue);
         pCue = pControl->getCue();
-        VERIFY_OR_DEBUG_ASSERT(pCue && pCue->getPosition() != Cue::kNoPosition) {
+        VERIFY_OR_DEBUG_ASSERT(pCue && pCue->getPosition().isValid()) {
             return;
         }
     }
@@ -951,15 +951,17 @@ void CueControl::hotcueCueLoop(HotcueControl* pControl, double value) {
         } else {
             bool loopActive = pControl->getStatus() == HotcueControl::Status::Active;
             Cue::StartAndEndPositions pos = pCue->getStartAndEndPosition();
-            setLoop(pos.startPosition, pos.endPosition, !loopActive);
+            setLoop(pos.startPosition.toEngineSamplePosMaybeInvalid(),
+                    pos.endPosition.toEngineSamplePosMaybeInvalid(),
+                    !loopActive);
         }
     } break;
     case mixxx::CueType::HotCue: {
         // The hotcue_X_cueloop CO was invoked for a hotcue. In that case,
         // create a beatloop starting at the hotcue position. This is useful for
         // mapping the CUE LOOP mode labeled on some controllers.
         setCurrentSavedLoopControlAndActivate(nullptr);
-        double startPosition = pCue->getPosition();
+        double startPosition = pCue->getPosition().toEngineSamplePosMaybeInvalid();
         bool loopActive = m_pLoopEnabled->toBool() &&
                 (startPosition == m_pLoopStartPosition->get());
         setBeatLoop(startPosition, !loopActive);
@@ -978,7 +980,7 @@ void CueControl::hotcueActivate(HotcueControl* pControl, double value, HotcueSet
     CuePointer pCue = pControl->getCue();
     if (value > 0) {
         // pressed
-        if (pCue && pCue->getPosition() != Cue::kNoPosition &&
+        if (pCue && pCue->getPosition().isValid() &&
                 pCue->getType() != mixxx::CueType::Invalid) {
             if (m_pPlay->toBool() && m_currentlyPreviewingIndex == Cue::kNoHotCue) {
                 // playing by Play button
@@ -993,7 +995,10 @@ void CueControl::hotcueActivate(HotcueControl* pControl, double value, HotcueSet
                         bool loopActive = pControl->getStatus() ==
                                 HotcueControl::Status::Active;
                         Cue::StartAndEndPositions pos = pCue->getStartAndEndPosition();
-                        setLoop(pos.startPosition, pos.endPosition, !loopActive);
+                        setLoop(pos.startPosition
+                                        .toEngineSamplePosMaybeInvalid(),
+                                pos.endPosition.toEngineSamplePosMaybeInvalid(),
+                                !loopActive);
                     }
                     break;
                 default:
@@ -1096,7 +1101,9 @@ void CueControl::hotcuePositionChanged(
         if (newPosition == Cue::kNoPosition) {
             detachCue(pControl);
         } else if (newPosition > 0 && newPosition < m_pTrackSamples->get()) {
-            if (pCue->getType() == mixxx::CueType::Loop && newPosition >= pCue->getEndPosition()) {
+            if (pCue->getType() == mixxx::CueType::Loop &&
+                    newPosition >= pCue->getEndPosition()
+                                           .toEngineSamplePosMaybeInvalid()) {
                 return;
             }
             pCue->setStartPosition(newPosition);
@@ -1115,7 +1122,7 @@ void CueControl::hotcueEndPositionChanged(
             pCue->setType(mixxx::CueType::HotCue);
             pCue->setEndPosition(Cue::kNoPosition);
         } else {
-            if (newEndPosition > pCue->getPosition()) {
+            if (newEndPosition > pCue->getPosition().toEngineSamplePosMaybeInvalid()) {
                 pCue->setEndPosition(newEndPosition);
             }
         }
@@ -2092,12 +2099,14 @@ void CueControl::setCurrentSavedLoopControlAndActivate(HotcueControl* pControl)
 
     VERIFY_OR_DEBUG_ASSERT(
             type == mixxx::CueType::Loop &&
-            pos.endPosition != Cue::kNoPosition) {
+            pos.endPosition.isValid()) {
         return;
     }
 
     // Set new control as active
-    setLoop(pos.startPosition, pos.endPosition, true);
+    setLoop(pos.startPosition.toEngineSamplePosMaybeInvalid(),
+            pos.endPosition.toEngineSamplePosMaybeInvalid(),
+            true);
     pControl->setStatus(HotcueControl::Status::Active);
     m_pCurrentSavedLoopControl.storeRelease(pControl);
 }
@@ -2113,14 +2122,14 @@ void CueControl::slotLoopEnabledChanged(bool enabled) {
     }
 
     DEBUG_ASSERT(pSavedLoopControl->getStatus() != HotcueControl::Status::Empty);
-    DEBUG_ASSERT(
-            pSavedLoopControl->getCue() &&
+    DEBUG_ASSERT(pSavedLoopControl->getCue() &&
             pSavedLoopControl->getCue()->getPosition() ==
-                    m_pLoopStartPosition->get());
-    DEBUG_ASSERT(
-            pSavedLoopControl->getCue() &&
+                    mixxx::audio::FramePos::fromEngineSamplePosMaybeInvalid(
+                            m_pLoopStartPosition->get()));
+    DEBUG_ASSERT(pSavedLoopControl->getCue() &&
             pSavedLoopControl->getCue()->getEndPosition() ==
-                    m_pLoopEndPosition->get());
+                    mixxx::audio::FramePos::fromEngineSamplePosMaybeInvalid(
+                            m_pLoopEndPosition->get()));
 
     if (enabled) {
         pSavedLoopControl->setStatus(HotcueControl::Status::Active);
@@ -2421,8 +2430,8 @@ double HotcueControl::getEndPosition() const {
 void HotcueControl::setCue(const CuePointer& pCue) {
     DEBUG_ASSERT(!m_pCue);
     Cue::StartAndEndPositions pos = pCue->getStartAndEndPosition();
-    setPosition(pos.startPosition);
-    setEndPosition(pos.endPosition);
+    setPosition(pos.startPosition.toEngineSamplePosMaybeInvalid());
+    setEndPosition(pos.endPosition.toEngineSamplePosMaybeInvalid());
     setColor(pCue->getColor());
     setStatus((pCue->getType() == mixxx::CueType::Invalid)
                     ? HotcueControl::Status::Empty

src/engine/controls/cuecontrol.h

@@ -112,7 +112,7 @@ class HotcueControl : public QObject {
     /// for the case the cue is deleted during preview.
     void cachePreviewingStartState() {
         if (m_pCue) {
-            m_previewingPosition.setValue(m_pCue->getPosition());
+            m_previewingPosition.setValue(m_pCue->getPosition().toEngineSamplePosMaybeInvalid());
             m_previewingType.setValue(m_pCue->getType());
         } else {
             m_previewingType.setValue(mixxx::CueType::Invalid);

src/engine/controls/enginecontrol.h

@@ -5,6 +5,7 @@
 #include <QList>
 #include <QObject>
 
+#include "audio/frame.h"
 #include "control/controlvalue.h"
 #include "engine/cachingreader/cachingreader.h"
 #include "engine/effects/groupfeaturestate.h"
@@ -17,6 +18,9 @@ class EngineMaster;
 class EngineBuffer;
 
 const int kNoTrigger = -1;
+static_assert(
+        mixxx::audio::FramePos::kLegacyInvalidEnginePosition == kNoTrigger,
+        "Invalid engine position value mismatch");
 
 /**
  * EngineControl is an abstract base class for objects which implement

src/engine/controls/vinylcontrolcontrol.cpp

@@ -127,7 +127,7 @@ void VinylControlControl::slotControlVinylSeek(double fractionalPos) {
                 continue;
             }
 
-            double cue_position = pCue->getPosition();
+            double cue_position = pCue->getPosition().toEngineSamplePosMaybeInvalid();
             // pick cues closest to new_playpos
             if ((nearest_playpos == -1) ||
                 (fabs(new_playpos - cue_position) < shortest_distance)) {

src/library/autodj/autodjprocessor.cpp

@@ -1104,7 +1104,7 @@ double AutoDJProcessor::getFirstSoundSecond(DeckAttributes* pDeck) {
 
     CuePointer pFromTrackAudibleSound = pTrack->findCueByType(mixxx::CueType::AudibleSound);
     if (pFromTrackAudibleSound) {
-        double firstSound = pFromTrackAudibleSound->getPosition();
+        double firstSound = pFromTrackAudibleSound->getPosition().toEngineSamplePosMaybeInvalid();
         if (firstSound > 0.0) {
             return samplePositionToSeconds(firstSound, pDeck);
         }
@@ -1121,8 +1121,9 @@ double AutoDJProcessor::getLastSoundSecond(DeckAttributes* pDeck) {
     CuePointer pFromTrackAudibleSound = pTrack->findCueByType(mixxx::CueType::AudibleSound);
     if (pFromTrackAudibleSound) {
         Cue::StartAndEndPositions pos = pFromTrackAudibleSound->getStartAndEndPosition();
-        if (pos.endPosition > 0 && (pos.endPosition - pos.startPosition) > 0) {
-            return samplePositionToSeconds(pos.endPosition, pDeck);
+        if (pos.endPosition > mixxx::audio::kStartFramePos &&
+                (pos.endPosition - pos.startPosition) > 0) {
+            return samplePositionToSeconds(pos.endPosition.toEngineSamplePosMaybeInvalid(), pDeck);
         }
     }
     return getEndSecond(pDeck);

src/library/dao/cuedao.cpp

@@ -4,6 +4,7 @@
 #include <QtDebug>
 #include <QtSql>
 
+#include "engine/engine.h"
 #include "library/queryutil.h"
 #include "track/track.h"
 #include "util/assert.h"
@@ -40,8 +41,10 @@ CuePointer cueFromRow(const QSqlRecord& row) {
     const auto id = DbId(row.value(row.indexOf("id")));
     TrackId trackId(row.value(row.indexOf("track_id")));
     int type = row.value(row.indexOf("type")).toInt();
-    int position = row.value(row.indexOf("position")).toInt();
-    int length = row.value(row.indexOf("length")).toInt();
+    const auto position =
+            mixxx::audio::FramePos::fromEngineSamplePosMaybeInvalid(
+                    row.value(row.indexOf("position")).toInt());
+    int lengthFrames = row.value(row.indexOf("length")).toInt() / mixxx::kEngineChannelCount;
     int hotcue = row.value(row.indexOf("hotcue")).toInt();
     QString label = labelFromQVariant(row.value(row.indexOf("label")));
     mixxx::RgbColor::optional_t color = mixxx::RgbColor::fromQVariant(row.value(row.indexOf("color")));
@@ -51,7 +54,7 @@ CuePointer cueFromRow(const QSqlRecord& row) {
     CuePointer pCue(new Cue(id,
             static_cast<mixxx::CueType>(type),
             position,
-            length,
+            lengthFrames,
             hotcue,
             label,
             *color));
@@ -165,8 +168,8 @@ bool CueDAO::saveCue(TrackId trackId, Cue* cue) const {
     // Bind values and execute query
     query.bindValue(":track_id", trackId.toVariant());
     query.bindValue(":type", static_cast<int>(cue->getType()));
-    query.bindValue(":position", cue->getPosition());
-    query.bindValue(":length", cue->getLength());
+    query.bindValue(":position", cue->getPosition().toEngineSamplePosMaybeInvalid());
+    query.bindValue(":length", cue->getLengthFrames() * mixxx::kEngineChannelCount);
     query.bindValue(":hotcue", cue->getHotCue());
     query.bindValue(":label", labelToQVariant(cue->getLabel()));
     query.bindValue(":color", mixxx::RgbColor::toQVariant(cue->getColor()));

src/library/dao/trackdao.cpp

@@ -536,7 +536,8 @@ void bindTrackLibraryValues(
     pTrackLibraryQuery->bindValue(":comment", trackInfo.getComment());
     pTrackLibraryQuery->bindValue(":url", track.getUrl());
     pTrackLibraryQuery->bindValue(":rating", track.getRating());
-    pTrackLibraryQuery->bindValue(":cuepoint", track.getCuePoint().getPosition());
+    pTrackLibraryQuery->bindValue(":cuepoint",
+            track.getMainCuePosition().toEngineSamplePosMaybeInvalid());
     pTrackLibraryQuery->bindValue(":bpm_lock", track.getBpmLocked() ? 1 : 0);
     pTrackLibraryQuery->bindValue(":replaygain", trackInfo.getReplayGain().getRatio());
     pTrackLibraryQuery->bindValue(":replaygain_peak", trackInfo.getReplayGain().getPeak());
@@ -1145,7 +1146,8 @@ bool setTrackRating(const QSqlRecord& record, const int column,
 
 bool setTrackCuePoint(const QSqlRecord& record, const int column,
                       TrackPointer pTrack) {
-    pTrack->setCuePoint(CuePosition(record.value(column).toDouble()));
+    pTrack->setMainCuePosition(mixxx::audio::FramePos::fromEngineSamplePosMaybeInvalid(
+            record.value(column).toDouble()));
     return false;
 }
 

src/library/export/engineprimeexportjob.cpp

@@ -248,7 +248,7 @@ void exportMetadata(djinterop::database* pDatabase,
 
         djinterop::hot_cue hotCue{};
         hotCue.label = label.toStdString();
-        hotCue.sample_offset = playPosToSampleOffset(pCue->getPosition());
+        hotCue.sample_offset = playPosToSampleOffset(pCue->getPosition().toEngineSamplePos());
 
         auto color = mixxx::RgbColor::toQColor(pCue->getColor());
         hotCue.color = djinterop::pad_color{

src/mixer/basetrackplayer.cpp

@@ -265,8 +265,8 @@ void BaseTrackPlayerImpl::loadTrack(TrackPointer pTrack) {
         }
 
         if (pLoopCue) {
-            double loopStart = pLoopCue->getPosition();
-            double loopEnd = loopStart + pLoopCue->getLength();
+            double loopStart = pLoopCue->getPosition().toEngineSamplePosMaybeInvalid();
+            double loopEnd = loopStart + (pLoopCue->getLengthFrames() * mixxx::kEngineChannelCount);
             if (loopStart != kNoTrigger && loopEnd != kNoTrigger && loopStart <= loopEnd) {
                 m_pLoopInPoint->set(loopStart);
                 m_pLoopOutPoint->set(loopEnd);