diff --git a/src/macros/macromanager.cpp b/src/macros/macromanager.cpp index 189df50ea650..e9b482445ca6 100644 --- a/src/macros/macromanager.cpp +++ b/src/macros/macromanager.cpp @@ -15,10 +15,19 @@ MacroManager::MacroManager(mixxx::DbConnectionPoolPtr pDbConnectionPool) &MacroManager::saveMacro); } -void MacroManager::saveMacro(ChannelHandle channel, Macro macro) { +QByteArray serialize(QVector actions) { + proto::Macro macroProto; + auto actionsProto = macroProto.mutable_actions(); + for (auto action : actions) { + actionsProto->AddAllocated(action.serialize()); + } + auto string = macroProto.SerializeAsString(); + return QByteArray(string.data(), string.length()); +} + +void MacroManager::saveMacro(ChannelHandle channel, QVector actions) { qCDebug(macroLoggingCategory) << "Saving Macro for channel" << channel.handle(); // TODO(xerus) add test - macro.dump(); QSqlQuery query(m_database); query.prepare(QStringLiteral( "INSERT INTO macros " @@ -30,7 +39,7 @@ void MacroManager::saveMacro(ChannelHandle channel, Macro macro) { query.bindValue(":state", 0); // TODO(xerus) proper labels query.bindValue(":label", QString("testch%1").arg(QString::number(channel.handle()))); - query.bindValue(":content", macro.serialize()); + query.bindValue(":content", serialize(actions)); if (!query.exec()) { LOG_FAILED_QUERY(query); return; diff --git a/src/macros/macromanager.h b/src/macros/macromanager.h index 577a7f315251..6f4f4c8c603d 100644 --- a/src/macros/macromanager.h +++ b/src/macros/macromanager.h @@ -13,7 +13,7 @@ class MacroManager : public QObject { MacroRecorder* getRecorder(); public slots: - void saveMacro(ChannelHandle channel, Macro macro); + void saveMacro(ChannelHandle channel, QVector actions); private: std::unique_ptr m_pMacroRecorder; diff --git a/src/macros/macrorecorder.cpp b/src/macros/macrorecorder.cpp index 37dbc1f39275..343f8bdc32bb 100644 --- a/src/macros/macrorecorder.cpp +++ b/src/macros/macrorecorder.cpp @@ -8,6 +8,7 @@ // TODO(xerus) handle track eject while recording namespace { +constexpr uint kMaxMacroSize = 1000; const QString kConfigGroup = QStringLiteral("[MacroRecording]"); } @@ -15,9 +16,8 @@ MacroRecorder::MacroRecorder() : m_COToggleRecording(ConfigKey(kConfigGroup, "recording_toggle")), m_CORecStatus(ConfigKey(kConfigGroup, "recording_status")), m_activeChannel(nullptr), - m_macroRecordingState(State::Disabled), m_pStartRecordingTimer(this), - m_recordedMacro() { + m_pRecordedActions(kMaxMacroSize) { qCDebug(macroLoggingCategory) << "MacroRecorder construct"; connect(&m_COToggleRecording, @@ -33,27 +33,16 @@ MacroRecorder::MacroRecorder() void MacroRecorder::notifyCueJump( ChannelHandle* channel, double sourceFramePos, double destFramePos) { qCDebug(macroLoggingCategory) << "Jump in channel" << channel->handle(); - if (checkOrClaimRecording(channel)) { - m_recordedMacro.appendJump(sourceFramePos, destFramePos); + if (isRecordingActive() && checkOrClaimRecording(channel)) { + m_pRecordedActions.emplace(sourceFramePos, destFramePos); qCDebug(macroLoggingCategory) << "Recorded jump in channel" << channel->handle(); - setState(State::Armed); } } bool MacroRecorder::checkOrClaimRecording(ChannelHandle* channel) { - if (m_activeChannel != nullptr) { - return m_activeChannel->handle() == channel->handle() && claimRecording(); - } else if (claimRecording()) { - m_activeChannel = channel; - qCDebug(macroLoggingCategory) << "Claimed recording for channel" << channel->handle(); - return true; - } - return false; -} - -bool MacroRecorder::claimRecording() { - auto armed = State::Armed; - return m_macroRecordingState.compare_exchange_weak(armed, State::Recording); + ChannelHandle* value = nullptr; + return m_activeChannel.compare_exchange_strong(value, channel) || + value->handle() == channel->handle(); } void MacroRecorder::pollRecordingStart() { @@ -61,55 +50,50 @@ void MacroRecorder::pollRecordingStart() { if (getActiveChannel() == nullptr) { return; } - if (getState() != State::Disabled) { - m_CORecStatus.set(Status::Recording); - } m_pStartRecordingTimer.stop(); + m_CORecStatus.set(Status::Recording); } void MacroRecorder::startRecording() { qCDebug(macroLoggingCategory) << "MacroRecorder recording armed"; m_CORecStatus.set(Status::Armed); - m_recordedMacro.clear(); - setState(State::Armed); m_pStartRecordingTimer.start(300); } void MacroRecorder::stopRecording() { qCDebug(macroLoggingCategory) << "MacroRecorder recording stop"; - auto armed = State::Armed; - while (!m_macroRecordingState.compare_exchange_strong(armed, State::Disabled)) { - QThread::yieldCurrentThread(); - if (getState() == State::Disabled) { - return; - } - armed = State::Armed; - } + m_pStartRecordingTimer.stop(); m_CORecStatus.set(Status::Disabled); - if (m_activeChannel == nullptr) { + + ChannelHandle* channel = m_activeChannel.exchange(nullptr); + if (channel == nullptr) { return; } - auto channel = m_activeChannel; - m_activeChannel = nullptr; - emit saveMacro(*channel, m_recordedMacro); -} -Macro MacroRecorder::getMacro() const { - return m_recordedMacro; + QVector actions; + while (MacroAction* action = m_pRecordedActions.front()) { + m_pRecordedActions.pop(); + actions.append(*action); + } + emit saveMacro(*channel, actions); } -ChannelHandle* MacroRecorder::getActiveChannel() const { - return m_activeChannel; +const MacroAction MacroRecorder::getRecordedAction() { + return *m_pRecordedActions.front(); } -bool MacroRecorder::isRecordingActive() const { - return getState() != State::Disabled; +size_t MacroRecorder::getRecordingSize() const { + return m_pRecordedActions.size(); } -MacroRecorder::State MacroRecorder::getState() const { - return m_macroRecordingState.load(); +const ChannelHandle* MacroRecorder::getActiveChannel() const { + return m_activeChannel.load(); } -void MacroRecorder::setState(State state) { - m_macroRecordingState.store(state); +MacroRecorder::Status MacroRecorder::getStatus() const { + return Status(m_CORecStatus.get()); } + +bool MacroRecorder::isRecordingActive() const { + return getStatus() > 0; +} \ No newline at end of file diff --git a/src/macros/macrorecorder.h b/src/macros/macrorecorder.h index 595fce51a396..aa70357a1b77 100644 --- a/src/macros/macrorecorder.h +++ b/src/macros/macrorecorder.h @@ -1,6 +1,7 @@ #pragma once #include +#include #include #include @@ -11,8 +12,6 @@ #include "macros/macro.h" #include "recording/recordingmanagerbase.h" -// TODO(xerus) add enum for recording_status - /// The MacroRecorder handles the recording of Macros and the [MacroRecording] controls. class MacroRecorder : public RecordingManagerBase { Q_OBJECT @@ -21,7 +20,6 @@ class MacroRecorder : public RecordingManagerBase { void startRecording() override; void stopRecording() override; - bool isRecordingActive() const override; enum Status : uint8_t { /// Nothing is going on @@ -32,54 +30,38 @@ class MacroRecorder : public RecordingManagerBase { Recording = 2, }; + bool isRecordingActive() const override; + Status getStatus() const; + /// Tries to append this cue jump to the currently recorded Macro. /// Returns true if the currently recorded Macro was changed - so only if /// recording is active and the channel handle matches. /// Only called in realtime code. void notifyCueJump(ChannelHandle* channel, double sourceFramePos, double destFramePos); - Macro getMacro() const; - ChannelHandle* getActiveChannel() const; + size_t getRecordingSize() const; + const MacroAction getRecordedAction(); + const ChannelHandle* getActiveChannel() const; signals: - void saveMacro(ChannelHandle channel, Macro macro); + void saveMacro(ChannelHandle channel, QVector actions); private slots: void pollRecordingStart(); private: - FRIEND_TEST(MacroRecordingTest, ClaimRecording); FRIEND_TEST(MacroRecordingTest, RecordCueJump); - FRIEND_TEST(MacroRecordingTest, StopRecordingAsync); - enum class State : uint8_t { - /// Nothing is going on - Disabled, - /// Recording is active, but nothing currently going on - Armed, - /// An Action is currently being recorded - Recording, - }; - - State getState() const; - - /// Checks if ths channel is recording, otherwise tries to claim it. + /// Checks if this channel is recording, otherwise tries to claim it. /// Returns true if this channel is recording. /// Called in realtime code. bool checkOrClaimRecording(ChannelHandle* channel); - /// Claims the recording if it is Armed. - /// Called in realtime code. - bool claimRecording(); - - void setState(State state); - ControlPushButton m_COToggleRecording; ControlObject m_CORecStatus; - ChannelHandle* m_activeChannel; - std::atomic m_macroRecordingState; + std::atomic m_activeChannel; QTimer m_pStartRecordingTimer; - Macro m_recordedMacro; + rigtorp::SPSCQueue m_pRecordedActions; }; diff --git a/src/test/macros_test.cpp b/src/test/macros_test.cpp index d443a7ec767f..3a98640c263f 100644 --- a/src/test/macros_test.cpp +++ b/src/test/macros_test.cpp @@ -29,78 +29,60 @@ TEST(MacrosTest, CreateAndSerializeMacro) { EXPECT_EQ(deserialized, macro); } -TEST(MacroRecordingTest, ClaimRecording) { +TEST(MacroRecordingTest, StartAndStopRecording) { MacroRecorder recorder; - EXPECT_EQ(recorder.isRecordingActive(), false); - recorder.claimRecording(); - EXPECT_EQ(recorder.isRecordingActive(), false); - EXPECT_EQ( - ControlProxy(kConfigGroup, "recording_status").get(), + ASSERT_EQ(ControlProxy(kConfigGroup, "recording_status").get(), MacroRecorder::Status::Disabled); + EXPECT_EQ(recorder.isRecordingActive(), false); recorder.startRecording(); - EXPECT_EQ( - ControlProxy(kConfigGroup, "recording_status").get(), + ASSERT_EQ(ControlProxy(kConfigGroup, "recording_status").get(), MacroRecorder::Status::Armed); - recorder.claimRecording(); EXPECT_EQ(recorder.isRecordingActive(), true); - recorder.setState(MacroRecorder::State::Armed); recorder.stopRecording(); - EXPECT_EQ( - ControlProxy(kConfigGroup, "recording_status").get(), + ASSERT_EQ(ControlProxy(kConfigGroup, "recording_status").get(), MacroRecorder::Status::Disabled); + EXPECT_EQ(recorder.isRecordingActive(), false); } TEST(MacroRecordingTest, RecordCueJump) { MacroRecorder recorder; auto factory = ChannelHandleFactory(); ChannelHandle handle = factory.getOrCreateHandle("test-one"); - EXPECT_EQ(recorder.getState(), MacroRecorder::State::Disabled); + ASSERT_EQ(recorder.getStatus(), MacroRecorder::Status::Disabled); recorder.notifyCueJump(&handle, 0, 1); - EXPECT_EQ(recorder.getActiveChannel(), nullptr); - EXPECT_EQ(recorder.getMacro().getLength(), 0); + ASSERT_EQ(recorder.getActiveChannel(), nullptr); + EXPECT_EQ(recorder.getRecordingSize(), 0); recorder.startRecording(); recorder.notifyCueJump(&handle, 0, 1); EXPECT_EQ(recorder.getActiveChannel()->handle(), handle.handle()); - EXPECT_EQ(recorder.getMacro().actions[0].position, 0); - EXPECT_EQ(recorder.getMacro().actions[0].target, 1); + ASSERT_EQ(recorder.getRecordingSize(), 1); + EXPECT_EQ(recorder.getRecordedAction().position, 0); + EXPECT_EQ(recorder.getRecordedAction().target, 1); auto handle2 = factory.getOrCreateHandle("test-two"); + recorder.notifyCueJump(&handle2, 0, 2); EXPECT_EQ(recorder.checkOrClaimRecording(&handle2), false); EXPECT_EQ(recorder.checkOrClaimRecording(&handle), true); + ASSERT_EQ(recorder.getRecordingSize(), 1); + + recorder.notifyCueJump(&handle, 3, 5); + EXPECT_EQ(recorder.getRecordingSize(), 2); recorder.pollRecordingStart(); - EXPECT_EQ( - ControlProxy(kConfigGroup, "recording_status").get(), + EXPECT_EQ(ControlProxy(kConfigGroup, "recording_status").get(), MacroRecorder::Status::Recording); } -TEST(MacroRecordingTest, StopRecordingAsync) { - MacroRecorder recorder; - recorder.setState(MacroRecorder::State::Recording); - std::atomic state{}; - QtConcurrent::run([&recorder, &state] { - QThread::msleep(100); - state.store(recorder.getState()); - recorder.setState(MacroRecorder::State::Armed); - }); - recorder.stopRecording(); - EXPECT_EQ(state.load(), MacroRecorder::State::Recording); - EXPECT_EQ(recorder.getState(), MacroRecorder::State::Disabled); -} - TEST(MacroRecordingTest, RecordingToggleControl) { MacroRecorder recorder; - ControlObject::set( - ConfigKey(kConfigGroup, "recording_toggle"), 1); - EXPECT_EQ(recorder.isRecordingActive(), true); - ControlObject::set( - ConfigKey(kConfigGroup, "recording_toggle"), 0); - EXPECT_EQ(recorder.isRecordingActive(), true); - ControlObject::set( - ConfigKey(kConfigGroup, "recording_toggle"), 1); - EXPECT_EQ(recorder.isRecordingActive(), false); + ControlObject::toggle( + ConfigKey(kConfigGroup, "recording_toggle")); + ASSERT_EQ(recorder.isRecordingActive(), true); + ControlObject::toggle( + ConfigKey(kConfigGroup, "recording_toggle")); + ASSERT_EQ(recorder.isRecordingActive(), false); } // Integration Tests @@ -116,18 +98,17 @@ TEST_F(MacroRecorderTest, RecordSeek) { ControlObject::toggle( ConfigKey(kConfigGroup, "recording_toggle")); ASSERT_EQ(m_pMacroRecorder->isRecordingActive(), true); - EXPECT_EQ( - ControlProxy(kConfigGroup, "recording_status").get(), + EXPECT_EQ(ControlProxy(kConfigGroup, "recording_status").get(), MacroRecorder::Status::Armed); m_pChannel1->getEngineBuffer()->slotControlSeekExact(50 * mixxx::kEngineChannelCount); ProcessBuffer(); - EXPECT_EQ(m_pMacroRecorder->getMacro().getLength(), 0); + EXPECT_EQ(m_pMacroRecorder->getRecordingSize(), 0); m_pChannel1->getEngineBuffer()->slotControlSeekAbs(10 * mixxx::kEngineChannelCount); ProcessBuffer(); - EXPECT_EQ(m_pMacroRecorder->getMacro().getLength(), 1); - EXPECT_EQ(m_pMacroRecorder->getMacro().actions[0].position, 50); - EXPECT_EQ(m_pMacroRecorder->getMacro().actions[0].target, 10); + EXPECT_EQ(m_pMacroRecorder->getRecordingSize(), 1); + EXPECT_EQ(m_pMacroRecorder->getRecordedAction().position, 50); + EXPECT_EQ(m_pMacroRecorder->getRecordedAction().target, 10); } TEST_F(MacroRecorderTest, RecordHotcueActivation) { @@ -137,10 +118,10 @@ TEST_F(MacroRecorderTest, RecordHotcueActivation) { ProcessBuffer(); m_pChannel1->getEngineBuffer()->slotControlSeekExact(100 * mixxx::kEngineChannelCount); ProcessBuffer(); - EXPECT_EQ(m_pMacroRecorder->getMacro().getLength(), 0); + EXPECT_EQ(m_pMacroRecorder->getRecordingSize(), 0); ControlObject::toggle(ConfigKey("[Channel1]", "hotcue_1_activate")); ProcessBuffer(); - EXPECT_EQ(m_pMacroRecorder->getMacro().getLength(), 1); - EXPECT_EQ(m_pMacroRecorder->getMacro().actions[0].position, 100); - EXPECT_EQ(m_pMacroRecorder->getMacro().actions[0].target, 0); + EXPECT_EQ(m_pMacroRecorder->getRecordingSize(), 1); + EXPECT_EQ(m_pMacroRecorder->getRecordedAction().position, 100); + EXPECT_EQ(m_pMacroRecorder->getRecordedAction().target, 0); }