diff --git a/src/engine/controls/loopingcontrol.cpp b/src/engine/controls/loopingcontrol.cpp index c83b618ebda..28c00bfe43c 100644 --- a/src/engine/controls/loopingcontrol.cpp +++ b/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]; diff --git a/src/engine/enginebuffer.cpp b/src/engine/enginebuffer.cpp index 8f832236038..d4ecf108260 100644 --- a/src/engine/enginebuffer.cpp +++ b/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) { diff --git a/src/engine/enginebuffer.h b/src/engine/enginebuffer.h index c04a03e0032..0d9d8868f65 100644 --- a/src/engine/enginebuffer.h +++ b/src/engine/enginebuffer.h @@ -6,6 +6,7 @@ #include #include +#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 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 m_pChannelToCloneFrom; // Is true if the previous buffer was silent due to pausing diff --git a/src/test/cuecontrol_test.cpp b/src/test/cuecontrol_test.cpp index 2b8c1f6eb04..903fe75dc22 100644 --- a/src/test/cuecontrol_test.cpp +++ b/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(); } diff --git a/src/test/enginebuffertest.cpp b/src/test/enginebuffertest.cpp index bce375dac5c..d0eb53f966c 100644 --- a/src/test/enginebuffertest.cpp +++ b/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); diff --git a/src/test/hotcuecontrol_test.cpp b/src/test/hotcuecontrol_test.cpp index c45c951d528..d92b3e966b5 100644 --- a/src/test/hotcuecontrol_test.cpp +++ b/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(); } diff --git a/src/test/looping_control_test.cpp b/src/test/looping_control_test.cpp index 7f0dcb1fde8..57947e7209e 100644 --- a/src/test/looping_control_test.cpp +++ b/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();