EngineBuffer: Use mixxx::audio::FramePos for some methods

src/engine/controls/loopingcontrol.cpp

@@ -1149,10 +1149,10 @@ void LoopingControl::slotBeatLoop(double beats, bool keepStartPoint, bool enable
     }
 
     // if a seek was queued in the engine buffer move the current sample to its position
-    double p_seekPosition = 0;
-    if (getEngineBuffer()->getQueuedSeekPosition(&p_seekPosition)) {
+    const mixxx::audio::FramePos seekPosition = getEngineBuffer()->queuedSeekPosition();
+    if (seekPosition.isValid()) {
         // seek position is already quantized if quantization is enabled
-        m_currentSample.setValue(p_seekPosition);
+        m_currentSample.setValue(seekPosition.toEngineSamplePos());
     }
 
     double maxBeatSize = s_dBeatSizes[sizeof(s_dBeatSizes)/sizeof(s_dBeatSizes[0]) - 1];

src/engine/enginebuffer.cpp

@@ -383,7 +383,7 @@ void EngineBuffer::setEngineMaster(EngineMaster* pEngineMaster) {
     }
 }
 
-void EngineBuffer::queueNewPlaypos(double newpos, enum SeekRequest seekType) {
+void EngineBuffer::queueNewPlaypos(mixxx::audio::FramePos position, enum SeekRequest seekType) {
     // All seeks need to be done in the Engine thread so queue it up.
     // Write the position before the seek type, to reduce a possible race
     // condition effect
@@ -392,7 +392,7 @@ void EngineBuffer::queueNewPlaypos(double newpos, enum SeekRequest seekType) {
         // use SEEK_STANDARD for that
         seekType = SEEK_STANDARD;
     }
-    m_queuedSeek.setValue({newpos, seekType});
+    m_queuedSeek.setValue({position, seekType});
 }
 
 void EngineBuffer::requestSyncPhase() {
@@ -457,7 +457,9 @@ void EngineBuffer::readToCrossfadeBuffer(const int iBufferSize) {
 }
 
 void EngineBuffer::seekCloneBuffer(EngineBuffer* pOtherBuffer) {
-    doSeekPlayPos(pOtherBuffer->getExactPlayPos(), SEEK_EXACT);
+    const auto position = mixxx::audio::FramePos::fromEngineSamplePos(
+            pOtherBuffer->getExactPlayPos());
+    doSeekPlayPos(position, SEEK_EXACT);
 }
 
 // WARNING: This method is not thread safe and must not be called from outside
@@ -584,7 +586,7 @@ void EngineBuffer::ejectTrack() {
     m_pause.lock();
 
     m_visualPlayPos->set(0.0, 0.0, 0.0, 0.0, 0.0);
-    doSeekPlayPos(0.0, SEEK_EXACT);
+    doSeekPlayPos(mixxx::audio::kStartFramePos, SEEK_EXACT);
 
     m_pCurrentTrack.reset();
     m_pTrackSamples->set(0);
@@ -654,12 +656,16 @@ void EngineBuffer::slotControlSeek(double fractionalPos) {
 
 // WARNING: This method runs from SyncWorker and Engine Worker
 void EngineBuffer::slotControlSeekAbs(double playPosition) {
-    doSeekPlayPos(playPosition, SEEK_STANDARD);
+    // TODO: Check if we can assert a valid play position here
+    const auto position = mixxx::audio::FramePos::fromEngineSamplePos(playPosition);
+    doSeekPlayPos(position, SEEK_STANDARD);
 }
 
 // WARNING: This method runs from SyncWorker and Engine Worker
 void EngineBuffer::slotControlSeekExact(double playPosition) {
-    doSeekPlayPos(playPosition, SEEK_EXACT);
+    // TODO: Check if we can assert a valid play position here
+    const auto position = mixxx::audio::FramePos::fromEngineSamplePos(playPosition);
+    doSeekPlayPos(position, SEEK_EXACT);
 }
 
 double EngineBuffer::fractionalPlayposFromAbsolute(double absolutePlaypos) {
@@ -676,11 +682,18 @@ void EngineBuffer::doSeekFractional(double fractionalPos, enum SeekRequest seekT
     VERIFY_OR_DEBUG_ASSERT(!util_isnan(fractionalPos)) {
         return;
     }
-    double newSamplePosition = fractionalPos * m_pTrackSamples->get();
-    doSeekPlayPos(newSamplePosition, seekType);
+
+    // FIXME: Use maybe invalid here
+    const auto trackEndPosition =
+            mixxx::audio::FramePos::fromEngineSamplePos(m_pTrackSamples->get());
+    VERIFY_OR_DEBUG_ASSERT(trackEndPosition.isValid()) {
+        return;
+    }
+    const auto seekPosition = trackEndPosition * fractionalPos;
+    doSeekPlayPos(seekPosition, seekType);
 }
 
-void EngineBuffer::doSeekPlayPos(double new_playpos, enum SeekRequest seekType) {
+void EngineBuffer::doSeekPlayPos(mixxx::audio::FramePos position, enum SeekRequest seekType) {
 #ifdef __VINYLCONTROL__
     // Notify the vinyl control that a seek has taken place in case it is in
     // absolute mode and needs be switched to relative.
@@ -689,7 +702,7 @@ void EngineBuffer::doSeekPlayPos(double new_playpos, enum SeekRequest seekType)
     }
 #endif
 
-    queueNewPlaypos(new_playpos, seekType);
+    queueNewPlaypos(position, seekType);
 }
 
 bool EngineBuffer::updateIndicatorsAndModifyPlay(bool newPlay, bool oldPlay) {
@@ -1224,12 +1237,7 @@ void EngineBuffer::processSeek(bool paused) {
     const QueuedSeek queuedSeek = m_queuedSeek.getValue();
 
     SeekRequests seekType = queuedSeek.seekType;
-    double position = queuedSeek.position;
-
-    // Don't allow the playposition to go past the end.
-    if (position > m_trackSamplesOld) {
-        position = m_trackSamplesOld;
-    }
+    mixxx::audio::FramePos framePosition = queuedSeek.position;
 
     // Add SEEK_PHASE bit, if any
     if (m_iSeekPhaseQueued.fetchAndStoreRelease(0)) {
@@ -1241,7 +1249,7 @@ void EngineBuffer::processSeek(bool paused) {
             return;
         case SEEK_PHASE:
             // only adjust phase
-            position = m_filepos_play;
+            framePosition = mixxx::audio::FramePos::fromEngineSamplePos(m_filepos_play);
             break;
         case SEEK_STANDARD:
             if (m_pQuantize->toBool()) {
@@ -1260,6 +1268,17 @@ void EngineBuffer::processSeek(bool paused) {
             return;
     }
 
+    VERIFY_OR_DEBUG_ASSERT(framePosition.isValid()) {
+        return;
+    }
+
+    auto position = framePosition.toEngineSamplePos();
+
+    // Don't allow the playposition to go past the end.
+    if (position > m_trackSamplesOld) {
+        position = m_trackSamplesOld;
+    }
+
     if (!paused && (seekType & SEEK_PHASE)) {
         if (kLogger.traceEnabled()) {
             kLogger.trace() << "EngineBuffer::processSeek" << getGroup() << "Seeking phase";
@@ -1315,10 +1334,13 @@ void EngineBuffer::postProcess(const int iBufferSize) {
     updateIndicators(m_speed_old, iBufferSize);
 }
 
-bool EngineBuffer::getQueuedSeekPosition(double* pSeekPosition) const {
+mixxx::audio::FramePos EngineBuffer::queuedSeekPosition() const {
     const QueuedSeek queuedSeek = m_queuedSeek.getValue();
-    *pSeekPosition = queuedSeek.position;
-    return (queuedSeek.seekType != SEEK_NONE);
+    if (queuedSeek.seekType == SEEK_NONE) {
+        return {};
+    }
+
+    return queuedSeek.position;
 }
 
 void EngineBuffer::updateIndicators(double speed, int iBufferSize) {

src/engine/enginebuffer.h

@@ -6,6 +6,7 @@
 #include <QMutex>
 #include <cfloat>
 
+#include "audio/frame.h"
 #include "control/controlvalue.h"
 #include "engine/cachingreader/cachingreader.h"
 #include "engine/engineobject.h"
@@ -110,7 +111,7 @@ class EngineBuffer : public EngineObject {
     void setEngineMaster(EngineMaster*);
 
     // Queues a new seek position. Use SEEK_EXACT or SEEK_STANDARD as seekType
-    void queueNewPlaypos(double newpos, enum SeekRequest seekType);
+    void queueNewPlaypos(mixxx::audio::FramePos newpos, enum SeekRequest seekType);
     void requestSyncPhase();
     void requestEnableSync(bool enabled);
     void requestSyncMode(SyncMode mode);
@@ -121,9 +122,9 @@ class EngineBuffer : public EngineObject {
     void processSlip(int iBufferSize);
     void postProcess(const int iBufferSize);
 
-    /// Return true iff a seek is currently queued but not yet processed
-    /// If no seek was queued, the seek position is set to -1
-    bool getQueuedSeekPosition(double* pSeekPosition) const;
+    /// Returns the seek position iff a seek is currently queued but not yet
+    /// processed. If no seek was queued, and invalid frame position is returned.
+    mixxx::audio::FramePos queuedSeekPosition() const;
 
     bool isTrackLoaded() const;
     TrackPointer getLoadedTrack() const;
@@ -195,7 +196,7 @@ class EngineBuffer : public EngineObject {
 
   private:
     struct QueuedSeek {
-        double position;
+        mixxx::audio::FramePos position;
         enum SeekRequest seekType;
     };
 
@@ -215,7 +216,7 @@ class EngineBuffer : public EngineObject {
     double fractionalPlayposFromAbsolute(double absolutePlaypos);
 
     void doSeekFractional(double fractionalPos, enum SeekRequest seekType);
-    void doSeekPlayPos(double playpos, enum SeekRequest seekType);
+    void doSeekPlayPos(mixxx::audio::FramePos position, enum SeekRequest seekType);
 
     // Read one buffer from the current scaler into the crossfade buffer.  Used
     // for transitioning from one scaler to another, or reseeking a scaler
@@ -391,8 +392,9 @@ class EngineBuffer : public EngineObject {
     QAtomicInt m_iEnableSyncQueued;
     QAtomicInt m_iSyncModeQueued;
     ControlValueAtomic<QueuedSeek> m_queuedSeek;
-    static constexpr QueuedSeek kNoQueuedSeek = {
-            -1.0, SEEK_NONE}; // value used if no seek is queued
+
+    /// Indicates that no seek is queued
+    static constexpr QueuedSeek kNoQueuedSeek = {mixxx::audio::kInvalidFramePos, SEEK_NONE};
     QAtomicPointer<EngineChannel> m_pChannelToCloneFrom;
 
     // Is true if the previous buffer was silent due to pausing

src/test/cuecontrol_test.cpp

@@ -55,8 +55,9 @@ class CueControlTest : public BaseSignalPathTest {
         return m_pChannel1->getEngineBuffer()->m_pCueControl->getSampleOfTrack().current;
     }
 
-    void setCurrentSample(double sample) {
-        m_pChannel1->getEngineBuffer()->queueNewPlaypos(sample, EngineBuffer::SEEK_STANDARD);
+    void setCurrentSample(double samplePosition) {
+        const auto position = mixxx::audio::FramePos::fromEngineSamplePos(samplePosition);
+        m_pChannel1->getEngineBuffer()->queueNewPlaypos(position, EngineBuffer::SEEK_STANDARD);
         ProcessBuffer();
     }
 

src/test/enginebuffertest.cpp

@@ -241,8 +241,8 @@ TEST_F(EngineBufferE2ETest, ScratchTest) {
     // to the other.
     ControlObject::set(ConfigKey(m_sGroup1, "scratch2_enable"), 1.0);
     ControlObject::set(ConfigKey(m_sGroup1, "scratch2"), 1.1);
-    m_pChannel1->getEngineBuffer()->queueNewPlaypos(450,
-                                                    EngineBuffer::SEEK_EXACT);
+    m_pChannel1->getEngineBuffer()->queueNewPlaypos(
+            mixxx::audio::FramePos(225), EngineBuffer::SEEK_EXACT);
     ProcessBuffer();
     ControlObject::set(ConfigKey(m_sGroup1, "scratch2"), -1.1);
     ProcessBuffer();
@@ -354,8 +354,8 @@ TEST_F(EngineBufferE2ETest, SeekTest) {
     ControlObject::set(ConfigKey(m_sGroup1, "rate"), 0.0);
     ControlObject::set(ConfigKey(m_sGroup1, "play"), 1.0);
     ProcessBuffer();
-    m_pChannel1->getEngineBuffer()->queueNewPlaypos(1000,
-                                                    EngineBuffer::SEEK_EXACT);
+    m_pChannel1->getEngineBuffer()->queueNewPlaypos(
+            mixxx::audio::FramePos(500), EngineBuffer::SEEK_EXACT);
     ProcessBuffer();
     assertBufferMatchesReference(m_pEngineMaster->masterBuffer(),
                                  kProcessBufferSize, "SeekTest");
@@ -406,7 +406,7 @@ TEST_F(EngineBufferE2ETest, CueGotoAndPlayTest) {
     ControlObject::set(ConfigKey(m_sGroup1, "quantize"), 1.0);
     ControlObject::set(ConfigKey(m_sGroup1, "cue_point"), 0.0);
     m_pChannel1->getEngineBuffer()->queueNewPlaypos(
-            1000, EngineBuffer::SEEK_EXACT);
+            mixxx::audio::FramePos(500), EngineBuffer::SEEK_EXACT);
     ProcessBuffer();
     ControlObject::set(ConfigKey(m_sGroup1, "cue_gotoandplay"), 1.0);
     ProcessBuffer();
@@ -430,7 +430,7 @@ TEST_F(EngineBufferE2ETest, CueGotoAndPlayDenon) {
     // enable Denon mode Bug #1504934
     ControlObject::set(ConfigKey(m_sGroup1, "cue_mode"), 2.0); // CUE_MODE_DENON
     m_pChannel1->getEngineBuffer()->queueNewPlaypos(
-            1000, EngineBuffer::SEEK_EXACT);
+            mixxx::audio::FramePos(500), EngineBuffer::SEEK_EXACT);
     ProcessBuffer();
     double cueBefore = ControlObject::get(ConfigKey(m_sGroup1, "cue_point"));
     ControlObject::set(ConfigKey(m_sGroup1, "cue_gotoandplay"), 1.0);

src/test/hotcuecontrol_test.cpp

@@ -82,8 +82,9 @@ class HotcueControlTest : public BaseSignalPathTest {
         return m_pChannel1->getEngineBuffer()->m_pCueControl->getSampleOfTrack().current;
     }
 
-    void setCurrentSamplePosition(double sample) {
-        m_pChannel1->getEngineBuffer()->queueNewPlaypos(sample, EngineBuffer::SEEK_STANDARD);
+    void setCurrentSamplePosition(double samplePosition) {
+        const auto position = mixxx::audio::FramePos::fromEngineSamplePos(samplePosition);
+        m_pChannel1->getEngineBuffer()->queueNewPlaypos(position, EngineBuffer::SEEK_STANDARD);
         ProcessBuffer();
     }
 

src/test/looping_control_test.cpp

@@ -69,8 +69,9 @@ class LoopingControlTest : public MockedEngineBackendTest {
         return m_pLoopEnabled->get() > 0.0;
     }
 
-    void seekToSampleAndProcess(double new_pos) {
-        m_pChannel1->getEngineBuffer()->queueNewPlaypos(new_pos, EngineBuffer::SEEK_STANDARD);
+    void seekToSampleAndProcess(double samplePosition) {
+        const auto position = mixxx::audio::FramePos::fromEngineSamplePos(samplePosition);
+        m_pChannel1->getEngineBuffer()->queueNewPlaypos(position, EngineBuffer::SEEK_STANDARD);
         ProcessBuffer();
     }
 
@@ -536,8 +537,7 @@ TEST_F(LoopingControlTest, LoopMoveTest) {
     EXPECT_EQ(44110, m_pChannel1->getEngineBuffer()->m_pLoopingControl->getSampleOfTrack().current);
 
     // Move backward so that the current position is outside the new location of the loop
-    m_pChannel1->getEngineBuffer()->queueNewPlaypos(44300, EngineBuffer::SEEK_STANDARD);
-    ProcessBuffer();
+    seekToSampleAndProcess(44300);
     m_pButtonBeatMoveBackward->set(1.0);
     m_pButtonBeatMoveBackward->set(0.0);
     ProcessBuffer();
@@ -565,8 +565,7 @@ TEST_F(LoopingControlTest, LoopMoveTest) {
             kLoopPositionMaxAbsError);
 
     // Move backward so that the current position is outside the new location of the loop
-    m_pChannel1->getEngineBuffer()->queueNewPlaypos(500, EngineBuffer::SEEK_STANDARD);
-    ProcessBuffer();
+    seekToSampleAndProcess(500);
     m_pButtonBeatMoveBackward->set(1.0);
     m_pButtonBeatMoveBackward->set(0.0);
     ProcessBuffer();