From 21c8816be5acd0f0444017f51003848255177740 Mon Sep 17 00:00:00 2001 From: xeruf <27jf@pm.me> Date: Tue, 16 Nov 2021 21:05:51 +0100 Subject: [PATCH] Macros: Migrate to FramePos --- src/engine/controls/enginecontrol.h | 22 +++++------ src/engine/controls/macrocontrol.cpp | 51 ++++++++++++------------ src/engine/controls/macrocontrol.h | 10 +++-- src/engine/enginebuffer.cpp | 20 +++++----- src/engine/enginebuffer.h | 12 +++--- src/test/macros/macro_test.cpp | 2 +- src/test/macros/macro_test.h | 8 ++-- src/test/macros/macrocontrol_test.cpp | 52 ++++++++++++------------- src/test/macros/macrodao_test.cpp | 2 +- src/test/macros/macroplayback_test.cpp | 10 ++--- src/test/macros/macrorecording_test.cpp | 34 ++++++++-------- src/track/beats.h | 6 +-- src/track/macro.cpp | 10 ++--- src/track/macro.h | 4 +- src/track/macroaction.cpp | 6 +-- src/track/macroaction.h | 29 ++++++-------- src/track/track.cpp | 8 ++-- 17 files changed, 138 insertions(+), 148 deletions(-) diff --git a/src/engine/controls/enginecontrol.h b/src/engine/controls/enginecontrol.h index 8d2a4431b9c2..0f0abda4a3e2 100644 --- a/src/engine/controls/enginecontrol.h +++ b/src/engine/controls/enginecontrol.h @@ -41,17 +41,17 @@ class EngineControl : public QObject { UserSettingsPointer pConfig); ~EngineControl() override; - // Called by EngineBuffer::process every latency period. See the above - // comments for information about guarantees that hold during this call. An - // EngineControl can perform any upkeep operations that are necessary during - // this call. + /// Called by EngineBuffer::process every latency period. + /// See the above comments for information about guarantees that hold during this call. + /// An EngineControl can perform any upkeep operations necessary here. + /// @param dRate current playback rate in audio frames per second virtual void process(const double dRate, mixxx::audio::FramePos currentPosition, const int iBufferSize); - // hintReader allows the EngineControl to provide hints to the reader to - // indicate that the given portion of a song is a potential imminent seek - // target. + /// hintReader allows the EngineControl to provide hints to the reader + /// to indicate that the given portion of a song + /// is a potential imminent seek target. virtual void hintReader(HintVector* pHintList); virtual void setEngineMaster(EngineMaster* pEngineMaster); @@ -66,7 +66,7 @@ class EngineControl : public QObject { mixxx::audio::FramePos endPosition, bool enabled); - // Called to collect player features for effects processing. + /// Collect player features for effects processing. virtual void collectFeatureState(GroupFeatureState* pGroupFeatures) const { Q_UNUSED(pGroupFeatures); } @@ -96,10 +96,10 @@ class EngineControl : public QObject { } void seek(double fractionalPosition); void seekAbs(mixxx::audio::FramePos position); - /// Seek to an exact sample, no quantizing - /// virtual only for testing! + /// Seek to an exact frame, no quantizing + /// virtual only for tests! virtual void seekExact(mixxx::audio::FramePos position); - // Returns an EngineBuffer to target for syncing. Returns nullptr if none found + /// Return an EngineBuffer to target for syncing. Returns nullptr if none found. EngineBuffer* pickSyncTarget(); UserSettingsPointer getConfig(); diff --git a/src/engine/controls/macrocontrol.cpp b/src/engine/controls/macrocontrol.cpp index 1a02f874be14..8b5404b0de1f 100644 --- a/src/engine/controls/macrocontrol.cpp +++ b/src/engine/controls/macrocontrol.cpp @@ -76,28 +76,28 @@ MacroControl::MacroControl(const QString& group, UserSettingsPointer pConfig, in &MacroControl::slotClear); } -void MacroControl::process(const double dRate, const double dCurrentSample, const int iBufferSize) { +void MacroControl::process(const double dRate, + mixxx::audio::FramePos currentPosition, + const int iBufferSize) { Q_UNUSED(dRate); - if (m_queuedJumpTarget >= 0) { + if (m_queuedJumpTarget.value() >= 0) { // if a cue press doesn't change the position, notifySeek isn't called, thus m_queuedJumpTarget isn't reset if (getStatus() == Status::Armed) { // start the recording on a cue press even when there is no jump - notifySeek(dCurrentSample); + notifySeek(currentPosition); } - m_queuedJumpTarget = -1; + m_queuedJumpTarget = mixxx::audio::FramePos(-1); } if (getStatus() != Status::Playing) { return; } - double framePos = dCurrentSample / mixxx::kEngineChannelCount; const MacroAction& nextAction = m_pMacro->getActions().at(m_iNextAction); - double nextActionPos = nextAction.getSourcePosition(); - int bufFrames = iBufferSize / 2; - // The process method is called roughly every iBufferSize/2 samples, the + // The process method is called roughly every iBufferSize frames, the // tolerance range is double that to be safe. It is ahead of the position // because the seek is executed in the next EngineBuffer process cycle. - if (framePos > nextActionPos - bufFrames && framePos < nextActionPos + bufFrames) { - seekExact(nextAction.getTargetPositionSample()); + if (currentPosition > nextAction.getSourcePosition() - iBufferSize && + currentPosition < nextAction.getSourcePosition() + iBufferSize) { + seekExact(nextAction.getTargetPosition()); m_iNextAction++; if (m_iNextAction == m_pMacro->size()) { if (m_pMacro->isLooped()) { @@ -134,26 +134,24 @@ void MacroControl::trackLoaded(TrackPointer pNewTrack) { } } -void MacroControl::notifySeek(double dNewPlaypos) { - if (m_queuedJumpTarget < 0) { +void MacroControl::notifySeek(mixxx::audio::FramePos position) { + if (m_queuedJumpTarget.value() < 0) { return; } - double originalDestFramePos = m_queuedJumpTarget; - m_queuedJumpTarget = -1; + auto queuedTarget = m_queuedJumpTarget; + m_queuedJumpTarget = mixxx::audio::FramePos(-1); if (getStatus() == Status::Armed) { setStatus(Status::Recording); } if (getStatus() != Status::Recording) { return; } - double sourceFramePos = getSampleOfTrack().current / mixxx::kEngineChannelCount; - double destFramePos = dNewPlaypos / mixxx::kEngineChannelCount; - double diff = originalDestFramePos - destFramePos; - m_recordedActions.try_emplace(sourceFramePos + diff, originalDestFramePos); + // Account for quantization so the jump stays in-phase but snaps exactly to the original target + m_recordedActions.try_emplace(frameInfo().currentPosition + (queuedTarget - position), queuedTarget); } -void MacroControl::slotJumpQueued(double samplePos) { - m_queuedJumpTarget = samplePos / mixxx::kEngineChannelCount; +void MacroControl::slotJumpQueued(mixxx::audio::FramePos samplePos) { + m_queuedJumpTarget = samplePos; } MacroControl::Status MacroControl::getStatus() const { @@ -218,13 +216,12 @@ bool MacroControl::stopRecording() { } else { // This will still be the position of the previous track when called from trackLoaded // since trackLoaded is invoked before the SampleOfTrack of the controls is updated. - m_pMacro->setEnd(getSampleOfTrack().current / mixxx::kEngineChannelCount); + m_pMacro->setEnd(frameInfo().currentPosition); if (m_pMacro->getLabel().isEmpty()) { - // Automatically set the start position in seconds as label if there - // is no user-defined one - double secPos = m_pMacro->getStartSamplePos() / - mixxx::kEngineChannelCount / getSampleOfTrack().rate; - m_pMacro->setLabel(QString::number(secPos, 'f', 2)); + // Automatically set the start position in seconds as label + // if there is no user-defined one + auto secPos = m_pMacro->getStart() / frameInfo().sampleRate; + m_pMacro->setLabel(QString::number(secPos.value(), 'f', 2)); } setStatus(Status::Recorded); if (m_pMacro->isEnabled()) { @@ -324,7 +321,7 @@ void MacroControl::slotToggle(double value) { void MacroControl::slotGotoPlay(double value) { if (value > 0 && getStatus() > Status::Recording) { - seekExact(m_pMacro->getStartSamplePos()); + seekExact(m_pMacro->getStart()); play(); } } diff --git a/src/engine/controls/macrocontrol.h b/src/engine/controls/macrocontrol.h index 5cd7b3041e52..9a8d7a0c9583 100644 --- a/src/engine/controls/macrocontrol.h +++ b/src/engine/controls/macrocontrol.h @@ -14,8 +14,10 @@ class MacroControl : public EngineControl { MacroControl(const QString& group, UserSettingsPointer pConfig, int slot); void trackLoaded(TrackPointer pNewTrack) override; - void process(const double dRate, const double dCurrentSample, const int iBufferSize) override; - void notifySeek(double dNewPlaypos) override; + void process(const double dRate, + mixxx::audio::FramePos currentPosition, + const int iBufferSize) override; + void notifySeek(mixxx::audio::FramePos position) override; bool isRecording() const; @@ -42,7 +44,7 @@ class MacroControl : public EngineControl { void slotToggle(double value = 1); void slotClear(double value = 1); - void slotJumpQueued(double samplePos); + void slotJumpQueued(mixxx::audio::FramePos samplePos); private: /// Returns whether a new action was recorded @@ -60,7 +62,7 @@ class MacroControl : public EngineControl { QString m_controlPattern; /// The unquantized FramePos of a jump that is yet to be processed - double m_queuedJumpTarget; + mixxx::audio::FramePos m_queuedJumpTarget; rigtorp::SPSCQueue m_recordedActions; QTimer m_updateRecordingTimer; diff --git a/src/engine/enginebuffer.cpp b/src/engine/enginebuffer.cpp index 7eb706fff5d4..56ffde34ba4f 100644 --- a/src/engine/enginebuffer.cpp +++ b/src/engine/enginebuffer.cpp @@ -44,20 +44,20 @@ namespace { const mixxx::Logger kLogger("EngineBuffer"); -// This value is used to make sure the initial seek after loading a track is -// not omitted. Therefore this value must be different for 0.0 or any likely -// value for the main cue +/// Used to ensure the initial seek after loading a track is not omitted. +/// Therefore this value must stray away from 0.0 or any probably value for the main cue. constexpr auto kInitialPlayPosition = mixxx::audio::FramePos::fromEngineSamplePos( std::numeric_limits::lowest()); -constexpr double kLinearScalerElipsis = - 1.00058; // 2^(0.01/12): changes < 1 cent allows a linear scaler +/// 2^(0.01/12): changes < 1 cent allows a linear scaler +constexpr double kLinearScalerElipsis = 1.00058; -constexpr SINT kSamplesPerFrame = 2; // Engine buffer uses Stereo frames only +/// Engine buffer uses Stereo frames only +constexpr SINT kSamplesPerFrame = 2; -// Rate at which the playpos slider is updated -constexpr int kPlaypositionUpdateRate = 15; // updates per second +/// Updates per second of the playpos slider +constexpr int kPlaypositionUpdateRate = 15; } // anonymous namespace @@ -999,8 +999,8 @@ void EngineBuffer::processTrackLocked( // The way we treat rate inside of EngineBuffer is actually a // description of "sample consumption rate" or percentage of samples // consumed relative to playing back the track at its native sample - // rate and normal speed. pitch_adjust does not change the playback - // rate. + // rate and normal speed. + // pitch_adjust does not change the playback rate. rate = baserate * speed; // Scaler is up to date now. diff --git a/src/engine/enginebuffer.h b/src/engine/enginebuffer.h index 7358808eda28..68905ac42f45 100644 --- a/src/engine/enginebuffer.h +++ b/src/engine/enginebuffer.h @@ -64,13 +64,13 @@ class EngineBuffer : public EngineObject { SEEK_PHASE = 1u, /// Bypass Quantization SEEK_EXACT = 2u, - /// This is an artificial state that happens if an exact seek and a - /// phase seek are scheduled at the same time. + /// This artificial state occurs when an exact seek and a phase seek + /// are scheduled at the same time. SEEK_EXACT_PHASE = SEEK_PHASE | SEEK_EXACT, - /// #SEEK_PHASE if Quantize enables, otherwise SEEK_EXACT + /// SEEK_PHASE if Quantization is enabled, otherwise SEEK_EXACT SEEK_STANDARD = 4u, - /// This is an artificial state that happens if a standard seek and a - /// phase seek are scheduled at the same time. + /// This artificial state occurs when a standard seek and a phase seek + /// are scheduled at the same time. SEEK_STANDARD_PHASE = SEEK_STANDARD | SEEK_PHASE, }; Q_DECLARE_FLAGS(SeekRequests, SeekRequest); @@ -185,7 +185,7 @@ class EngineBuffer : public EngineObject { signals: void trackLoaded(TrackPointer pNewTrack, TrackPointer pOldTrack); void trackLoadFailed(TrackPointer pTrack, const QString& reason); - void cueJumpQueued(double samplePos); + void cueJumpQueued(mixxx::audio::FramePos targetPosition); private slots: void slotTrackLoading(); diff --git a/src/test/macros/macro_test.cpp b/src/test/macros/macro_test.cpp index 054f83eee9ca..6b599d8e42d1 100644 --- a/src/test/macros/macro_test.cpp +++ b/src/test/macros/macro_test.cpp @@ -3,7 +3,7 @@ #include TEST(MacroTest, SerializeMacroActions) { - QList actions{MacroAction(0, 1)}; + QList actions{MacroAction(mixxx::audio::FramePos(0), mixxx::audio::FramePos(1))}; QString filename(QDir::currentPath() % "/src/test/macros/macro_proto"); ASSERT_TRUE(QFile::exists(filename)); diff --git a/src/test/macros/macro_test.h b/src/test/macros/macro_test.h index 438b097f8c2d..437743be2f6e 100644 --- a/src/test/macros/macro_test.h +++ b/src/test/macros/macro_test.h @@ -11,16 +11,16 @@ struct TestMacro { MacroAction action; TestMacro(double sourceFramePos = 25'000, double targetFramePos = 7'500) - : action(sourceFramePos, targetFramePos) { + : action(mixxx::audio::FramePos(sourceFramePos), mixxx::audio::FramePos(targetFramePos)) { } /// Checks that the recorded Macro corresponds to the parameters, /// including the extra first loop action. void checkMacroAction(MacroPointer macro) { ASSERT_EQ(macro->size(), 2); - EXPECT_EQ(macro->getActions().last().getSourcePosition(), action.getSourcePosition()); - EXPECT_EQ(macro->getActions().last().getTargetPosition(), action.getTargetPosition()); + EXPECT_EQ(macro->getActions().last().getSourcePosition().value(), action.getSourcePosition().value()); + EXPECT_EQ(macro->getActions().last().getTargetPosition().value(), action.getTargetPosition().value()); // Loopback action - EXPECT_EQ(macro->getActions().first().getSourcePosition(), action.getTargetPosition()); + EXPECT_EQ(macro->getActions().first().getSourcePosition().value(), action.getTargetPosition().value()); } }; diff --git a/src/test/macros/macrocontrol_test.cpp b/src/test/macros/macrocontrol_test.cpp index 522508b60ed3..ca37b41192ef 100644 --- a/src/test/macros/macrocontrol_test.cpp +++ b/src/test/macros/macrocontrol_test.cpp @@ -20,7 +20,7 @@ class MacroControlTest : public MacroControl, public testing::Test { EXPECT_FALSE(isRecording()); } - MOCK_METHOD1(seekExact, void(double)); + MOCK_METHOD1(seekExact, void(mixxx::audio::FramePos)); const TrackPointer pLoadedTrack = Track::newTemporary(); }; @@ -39,37 +39,37 @@ TEST_F(MacroControlTest, RecordSeek) { EXPECT_TRUE(isRecording()); EXPECT_EQ(getMacro()->getLabel(), " [Recording]"); // Prepare recording - int frameRate = 1'000; - auto seek = [this, frameRate](double position) { + mixxx::audio::SampleRate frameRate(1'000); + auto seek = [this, frameRate](mixxx::audio::FramePos position) { notifySeek(position); - setCurrentSample(position, 99'000, frameRate); - process(0, position, 2); + setFrameInfo(position, mixxx::audio::FramePos(99'000), frameRate); + process(frameRate, position, 1); }; - seek(0); + seek(mixxx::audio::FramePos(0)); ASSERT_EQ(getStatus(), MacroControl::Status::Armed); // Initial jump - double startPos = 1'008; + mixxx::audio::FramePos startPos(504); slotJumpQueued(startPos); seek(startPos); // Jump with quantization adjustment TestMacro testMacro; - double diff = 20; - seek(testMacro.action.getSourcePositionSample() - diff); - slotJumpQueued(testMacro.action.getTargetPositionSample()); - seek(testMacro.action.getTargetPositionSample() - diff); + mixxx::audio::FrameDiff_t diff(10); + seek(testMacro.action.getSourcePosition() - diff); + slotJumpQueued(testMacro.action.getTargetPosition()); + seek(testMacro.action.getTargetPosition() - diff); // Stop recording - seek(testMacro.action.getTargetPositionSample()); + seek(testMacro.action.getTargetPosition()); EXPECT_CALL(*this, seekExact(startPos)); slotRecord(0); EXPECT_EQ(getStatus(), MacroControl::Status::Playing); // Check recording result testMacro.checkMacroAction(getMacro()); - EXPECT_EQ(getMacro()->getActions().first().getTargetPositionSample(), startPos); + EXPECT_EQ(getMacro()->getActions().first().getTargetPosition(), startPos); EXPECT_TRUE(pLoadedTrack->isDirty()); // Check generated label - EXPECT_EQ(startPos / mixxx::kEngineChannelCount / frameRate, 0.504); + EXPECT_EQ((startPos / frameRate).value(), 0.504); EXPECT_EQ(getMacro()->getLabel().toStdString(), "0.50"); // Activate EXPECT_CALL(*this, seekExact(startPos)); @@ -107,8 +107,8 @@ TEST_F(MacroControlTest, ControlObjects) { ControlProxy(kChannelGroup, "macro_2_enable").set(0); ControlProxy(kChannelGroup, "macro_2_loop").set(1); EXPECT_TRUE(getMacro()->isLooped()); - slotJumpQueued(0); - notifySeek(0); + slotJumpQueued(mixxx::audio::FramePos(0)); + notifySeek(mixxx::audio::FramePos(0)); activate.set(1); ASSERT_STATUS(MacroControl::Status::Recorded); EXPECT_EQ(getMacro()->getLabel(), label); @@ -116,7 +116,7 @@ TEST_F(MacroControlTest, ControlObjects) { ASSERT_STATUS(MacroControl::Status::Playing); // Restart - EXPECT_CALL(*this, seekExact(0)); + EXPECT_CALL(*this, seekExact(mixxx::audio::FramePos(0))); activate.set(1); ASSERT_STATUS(MacroControl::Status::Playing); @@ -131,7 +131,7 @@ TEST_F(MacroControlTest, ControlObjects) { } TEST_F(MacroControlTest, LoadTrackAndPlayAndClear) { - MacroAction jumpAction(40'000, 0); + MacroAction jumpAction(mixxx::audio::FramePos(40'000), mixxx::audio::FramePos(0)); TestMacro testMacro; QString label = QStringLiteral("test"); @@ -146,23 +146,23 @@ TEST_F(MacroControlTest, LoadTrackAndPlayAndClear) { slotActivate(); EXPECT_EQ(getStatus(), MacroControl::Status::Playing); EXPECT_CALL(*this, seekExact(jumpAction.getTargetPosition())); - process(0, jumpAction.getSourcePositionSample(), 2); + process(0, jumpAction.getSourcePosition(), 2); EXPECT_EQ(getStatus(), MacroControl::Status::Recorded); // LOOP slotLoop(1); - EXPECT_CALL(*this, seekExact(testMacro.action.getTargetPositionSample())); + EXPECT_CALL(*this, seekExact(testMacro.action.getTargetPosition())); slotActivate(); slotActivate(); // Jump - EXPECT_CALL(*this, seekExact(jumpAction.getTargetPositionSample())); - process(0, jumpAction.getSourcePositionSample(), 2); + EXPECT_CALL(*this, seekExact(jumpAction.getTargetPosition())); + process(0, jumpAction.getSourcePosition(), 2); // Loop back - EXPECT_CALL(*this, seekExact(testMacro.action.getTargetPositionSample())); - process(0, testMacro.action.getSourcePositionSample(), 2); + EXPECT_CALL(*this, seekExact(testMacro.action.getTargetPosition())); + process(0, testMacro.action.getSourcePosition(), 2); // Jump again - EXPECT_CALL(*this, seekExact(jumpAction.getTargetPositionSample())); - process(0, jumpAction.getSourcePositionSample(), 2); + EXPECT_CALL(*this, seekExact(jumpAction.getTargetPosition())); + process(0, jumpAction.getSourcePosition(), 2); EXPECT_EQ(getStatus(), MacroControl::Status::Playing); // Stop playing diff --git a/src/test/macros/macrodao_test.cpp b/src/test/macros/macrodao_test.cpp index 9f090164ddcb..e6d05f4b233f 100644 --- a/src/test/macros/macrodao_test.cpp +++ b/src/test/macros/macrodao_test.cpp @@ -18,7 +18,7 @@ class MacroDAOTest : public MixxxDbTest { TEST_F(MacroDAOTest, SaveAndLoadMacro) { TrackId track(1); - MacroAction action(0, 7); + MacroAction action(mixxx::audio::FramePos(0), mixxx::audio::FramePos(7)); Macro saved(QList{action}, "Test", Macro::StateFlag::Looped); m_macroDAO.saveMacro(track, &saved, 1); diff --git a/src/test/macros/macroplayback_test.cpp b/src/test/macros/macroplayback_test.cpp index 72798aa0b481..39a0052897e9 100644 --- a/src/test/macros/macroplayback_test.cpp +++ b/src/test/macros/macroplayback_test.cpp @@ -5,20 +5,18 @@ class MacroPlaybackTest : public BaseSignalPathTest { }; TEST_F(MacroPlaybackTest, Playback) { - MacroAction action(0, 2000); - QList actions{MacroAction(0, 0), action}; + MacroAction action(mixxx::audio::FramePos(0), mixxx::audio::FramePos(2000)); + QList actions{MacroAction(mixxx::audio::FramePos(0), mixxx::audio::FramePos(0)), action}; TrackPointer pTrack = getTestTrack(); pTrack->setMacros({{1, std::make_shared(actions, "Test1", Macro::StateFlag::Enabled)}}); loadTrack(m_pMixerDeck1, pTrack); EngineBuffer* pEngineBuffer = m_pMixerDeck1->getEngineDeck()->getEngineBuffer(); - EXPECT_EQ(pEngineBuffer->getExactPlayPos() / mixxx::kEngineChannelCount, - action.getSourcePosition()); + EXPECT_EQ(pEngineBuffer->getExactPlayPos(), action.getSourcePosition()); ProcessBuffer(); // We have to do a second call: First one only queues the seek ProcessBuffer(); - EXPECT_EQ(pEngineBuffer->getExactPlayPos() / mixxx::kEngineChannelCount, - action.getTargetPosition()); + EXPECT_EQ(pEngineBuffer->getExactPlayPos(), action.getTargetPosition()); } diff --git a/src/test/macros/macrorecording_test.cpp b/src/test/macros/macrorecording_test.cpp index a0a5edab5540..6089d3bac54b 100644 --- a/src/test/macros/macrorecording_test.cpp +++ b/src/test/macros/macrorecording_test.cpp @@ -4,7 +4,7 @@ #include "test/signalpathtest.h" constexpr int kMacro = 2; -constexpr int kStartPos = 0; +const mixxx::audio::FramePos kStartPos(0); class MacroRecordingTest : public BaseSignalPathTest { public: MacroRecordingTest() @@ -25,17 +25,17 @@ class MacroRecordingTest : public BaseSignalPathTest { } /// Starts recording and performs the initial jump to samplePos with assertions - void prepRecording(double samplePos) { + void prepRecording(mixxx::audio::FramePos samplePos) { m_record.set(1); ASSERT_EQ(getStatus(), MacroControl::Status::Armed); - m_pEngineBuffer1->slotControlSeekExact(kStartPos); + m_pEngineBuffer1->seekExact(kStartPos); ProcessBuffer(); ASSERT_EQ(m_pEngineBuffer1->getExactPlayPos(), kStartPos); - m_pEngineBuffer1->slotControlSeekAbs(kStartPos); + m_pEngineBuffer1->seekAbs(kStartPos); ProcessBuffer(); - m_pEngineBuffer1->slotControlSeekExact(samplePos); + m_pEngineBuffer1->seekExact(samplePos); ProcessBuffer(); ASSERT_EQ(m_pEngineBuffer1->getExactPlayPos(), samplePos); } @@ -47,43 +47,43 @@ class MacroRecordingTest : public BaseSignalPathTest { TEST_F(MacroRecordingTest, RecordSeekAndPlay) { TestMacro testMacro; - prepRecording(testMacro.action.getSourcePositionSample()); + prepRecording(testMacro.action.getSourcePosition()); - m_pEngineBuffer1->slotControlSeekAbs(testMacro.action.getTargetPositionSample()); + m_pEngineBuffer1->seekAbs(testMacro.action.getTargetPosition()); ProcessBuffer(); m_record.set(0); testMacro.checkMacroAction(getMacro()); // Should activate automatically EXPECT_EQ(getStatus(), MacroControl::Status::Playing); - EXPECT_EQ(getMacro()->getActions().first().getTargetPositionSample(), kStartPos); + EXPECT_EQ(getMacro()->getActions().first().getTargetPosition(), kStartPos); ProcessBuffer(); EXPECT_EQ(m_pEngineBuffer1->getExactPlayPos(), kStartPos); - MacroAction action2(1'000, 9'000); + MacroAction action2(mixxx::audio::FramePos(1'000), mixxx::audio::FramePos(9'000)); getMacro()->addAction(action2); - MacroAction action3(action2.getTargetPosition() + 100, 14'000); + MacroAction action3(mixxx::audio::FramePos(action2.getTargetPosition() + 100), mixxx::audio::FramePos(14'000)); getMacro()->addAction(action3); EXPECT_EQ(getMacro()->size(), 4); // Seek to first action - m_pEngineBuffer1->slotControlSeekExact(testMacro.action.getSourcePositionSample()); + m_pEngineBuffer1->seekExact(testMacro.action.getSourcePosition()); ProcessBuffer(); ProcessBuffer(); ProcessBuffer(); - EXPECT_EQ(m_pEngineBuffer1->getExactPlayPos(), testMacro.action.getTargetPositionSample()); + EXPECT_EQ(m_pEngineBuffer1->getExactPlayPos(), testMacro.action.getTargetPosition()); ASSERT_EQ(getStatus(), MacroControl::Status::Playing); // Seek to next action - m_pEngineBuffer1->slotControlSeekExact(action2.getSourcePositionSample()); + m_pEngineBuffer1->seekExact(action2.getSourcePosition()); ProcessBuffer(); // Trigger remaining actions ProcessBuffer(); - EXPECT_EQ(m_pEngineBuffer1->getExactPlayPos(), action2.getTargetPositionSample()); + EXPECT_EQ(m_pEngineBuffer1->getExactPlayPos(), action2.getTargetPosition()); ProcessBuffer(); - EXPECT_EQ(m_pEngineBuffer1->getExactPlayPos(), action3.getTargetPositionSample()); + EXPECT_EQ(m_pEngineBuffer1->getExactPlayPos(), action3.getTargetPosition()); EXPECT_EQ(getStatus(), MacroControl::Status::Recorded); } @@ -92,11 +92,11 @@ TEST_F(MacroRecordingTest, RecordHotcueAndPlay) { ControlObject::set(ConfigKey(kChannelGroup, "hotcue_1_set"), 1); TestMacro testMacro(10'000, 0); - prepRecording(testMacro.action.getSourcePositionSample()); + prepRecording(testMacro.action.getSourcePosition()); ControlObject::set(ConfigKey(kChannelGroup, "hotcue_1_goto"), 1); ProcessBuffer(); - EXPECT_EQ(m_pEngineBuffer1->getExactPlayPos(), testMacro.action.getTargetPositionSample()); + EXPECT_EQ(m_pEngineBuffer1->getExactPlayPos(), testMacro.action.getTargetPosition()); MacroPointer pMacro = getMacro(); // Check that recording stops gracefully when ejecting diff --git a/src/track/beats.h b/src/track/beats.h index 43008b3bcb58..246e440d416d 100644 --- a/src/track/beats.h +++ b/src/track/beats.h @@ -22,7 +22,7 @@ namespace mixxx { class Beats; typedef std::shared_ptr BeatsPointer; -/// A beat marker is denotes the border of a tempo section inside a track. +/// A beat marker denotes the border of a tempo section inside a track. class BeatMarker { public: BeatMarker(mixxx::audio::FramePos position, int beatsTillNextMarker) @@ -194,8 +194,8 @@ class Beats : private std::enable_shared_from_this { /// Returns an iterator pointing to earliest representable beat position /// (which is INT_MIN beats before the first beat marker). /// - /// Warning: Decrementing the iterator returned by this function will - /// result in an integer underflow. + /// Warning: Decrementing the iterator returned by this function + /// will result in an integer underflow. ConstIterator cbegin() const { return ConstIterator(this, m_markers.cbegin(), std::numeric_limits::lowest()); } diff --git a/src/track/macro.cpp b/src/track/macro.cpp index 9399d385ffaf..061d43fb6852 100644 --- a/src/track/macro.cpp +++ b/src/track/macro.cpp @@ -11,7 +11,7 @@ QList Macro::deserialize(const QByteArray& serialized) { QList result; result.reserve(macroProto.actions_size()); for (const proto::Macro_Action& action : macroProto.actions()) { - result.append(MacroAction(action.sourceframe(), action.targetframe())); + result.append(MacroAction(mixxx::audio::FramePos(action.sourceframe()), mixxx::audio::FramePos(action.targetframe()))); } return result; } @@ -88,14 +88,14 @@ void Macro::addAction(const MacroAction& action) { m_actions.append(action); } -double Macro::getStartSamplePos() const { +mixxx::audio::FramePos Macro::getStart() const { VERIFY_OR_DEBUG_ASSERT(!isEmpty()) { - return 0; + return mixxx::audio::FramePos(0); } - return m_actions.first().getTargetPositionSample(); + return m_actions.first().getTargetPosition(); } -void Macro::setEnd(double framePos) { +void Macro::setEnd(mixxx::audio::FramePos framePos) { VERIFY_OR_DEBUG_ASSERT(!isEmpty()) { return; } diff --git a/src/track/macro.h b/src/track/macro.h index 1623c64831c8..913fbe0b348a 100644 --- a/src/track/macro.h +++ b/src/track/macro.h @@ -46,9 +46,9 @@ class Macro { const QList& getActions() const; void addAction(const MacroAction& action); - double getStartSamplePos() const; + mixxx::audio::FramePos getStart() const; /// Sets the end of the Macro (relevant for looping). - void setEnd(double framePos); + void setEnd(mixxx::audio::FramePos framePos); void clear(); diff --git a/src/track/macroaction.cpp b/src/track/macroaction.cpp index 920e70e491db..7ca411bd07b8 100644 --- a/src/track/macroaction.cpp +++ b/src/track/macroaction.cpp @@ -1,9 +1,9 @@ #include "macroaction.h" proto::Macro_Action* MacroAction::serialize() const { - auto serialized = new proto::Macro_Action(); - serialized->set_sourceframe(static_cast(sourceFrame)); - serialized->set_targetframe(static_cast(targetFrame)); + auto *serialized = new proto::Macro_Action(); + serialized->set_sourceframe(static_cast(source.value())); + serialized->set_targetframe(static_cast(target.value())); serialized->set_type(static_cast(type)); return serialized; } diff --git a/src/track/macroaction.h b/src/track/macroaction.h index 05ae43bdbd7d..41354f67b875 100644 --- a/src/track/macroaction.h +++ b/src/track/macroaction.h @@ -1,5 +1,6 @@ #pragma once +#include "audio/frame.h" #include "engine/engine.h" #include "proto/macro.pb.h" namespace proto = mixxx::track::io; @@ -7,41 +8,33 @@ namespace proto = mixxx::track::io; /// A MacroAction is the building block of a Macro. /// It contains a position as well as the action to be taken at that position. /// -/// Note that currently only jumps to a target position are available, but that -/// is subject to change. +/// Note that currently only jumps to a target position are available, +/// but that is subject to change. class MacroAction { public: enum class Type : uint32_t { Jump = 0 }; - MacroAction(double sourceFramePos, double targetFramePos) - : sourceFrame(sourceFramePos), targetFrame(targetFramePos), type(Type::Jump){}; + MacroAction(mixxx::audio::FramePos source, mixxx::audio::FramePos target) + : source(source), target(target), type(Type::Jump){}; Type getType() const { return type; } - double getSourcePosition() const { - return sourceFrame; + mixxx::audio::FramePos getSourcePosition() const { + return source; } - double getTargetPosition() const { - return targetFrame; - } - - double getSourcePositionSample() const { - return sourceFrame * mixxx::kEngineChannelCount; - } - double getTargetPositionSample() const { - return targetFrame * mixxx::kEngineChannelCount; + mixxx::audio::FramePos getTargetPosition() const { + return target; } proto::Macro_Action* serialize() const; private: - // use FramePos once https://github.com/mixxxdj/mixxx/pull/2961 is merged - double sourceFrame; - double targetFrame; + mixxx::audio::FramePos source; + mixxx::audio::FramePos target; Type type; }; diff --git a/src/track/track.cpp b/src/track/track.cpp index 0eb88d269822..2250924a0f8c 100644 --- a/src/track/track.cpp +++ b/src/track/track.cpp @@ -1289,20 +1289,20 @@ void Track::importPendingCueInfosMarkDirtyAndUnlock( } QMap Track::getMacros() const { - QMutexLocker lock(&m_qMutex); + const auto locked = lockMutex(&m_qMutex); return m_macros; } void Track::setMacros(const QMap& macros) { - QMutexLocker lock(&m_qMutex); + const auto locked = lockMutex(&m_qMutex); m_macros = macros; } void Track::addMacro(int slot, const MacroPointer& pMacro) { - QMutexLocker lock(&m_qMutex); + auto locked = lockMutex(&m_qMutex); m_macros.insert(slot, pMacro); if (pMacro->isDirty()) { - markDirtyAndUnlock(&lock); + markDirtyAndUnlock(&locked); } }