VCVRack / Befaco

Befaco Eurorack modules for VCV Rack
https://library.vcvrack.com/Befaco
Other
146 stars 27 forks source link

Chaining Muxlicers doesn't work smoothly #32

Closed ZwergNaseXXL closed 2 years ago

ZwergNaseXXL commented 2 years ago

I tried chaining two Muxlicers via EOC -> One-Shot-In connections. Sounds like there's a timing hickup between the two (either way) and also an additional low pitch when going from 1 to 2. When I check the EOC trigger in a scope, it seems to come exactly at the same point as gate 8. Shouldn't it come after gate 8, maybe at the falling edge? Attached screenshot, can't attach saved selection.

grafik

hemmer commented 2 years ago

Thanks for the report, will take a look.

hemmer commented 2 years ago

Confirmed the issue, and have a partial fix. It doesn't yet fix problem when externally clocked though, so I will keep working on it.

From 2c3f723059261536317f413e8afc9c0a2c649d5c Mon Sep 17 00:00:00 2001
From: hemmer <915048+hemmer@users.noreply.github.com>
Date: Mon, 7 Feb 2022 22:00:15 +0000
Subject: [PATCH] WIP fix

---
 src/Muxlicer.cpp | 64 ++++++++++++++++++++++++++++++++++--------------
 1 file changed, 45 insertions(+), 19 deletions(-)

diff --git a/src/Muxlicer.cpp b/src/Muxlicer.cpp
index 8f24a0e..aaf6300 100644
--- a/src/Muxlicer.cpp
+++ b/src/Muxlicer.cpp
@@ -173,6 +173,12 @@ struct MultDivClock {

        return multipliedSeconds;
    }
+
+   void reset() {
+       secondsSinceLastClock = 0.f;
+       dividedProgressSeconds = 0.f;
+       dividerCount = 0;
+   }
 };

 static const std::vector<int> clockOptionsQuadratic = {-16, -8, -4, -2, 1, 2, 4, 8, 16};
@@ -254,8 +260,14 @@ struct Muxlicer : Module {

    uint32_t runIndex;  // which step are we on (0 to 7)
    uint32_t addressIndex = 0;
-   bool playHeadHasReset = false;
    bool tapped = false;
+   // Used to track if a reset has been triggered. Can be from the CV input, or the momentary switch. Note
+   // that behaviour depends on if the Muxlicer is clocked or not. If clocked, the playhead resets but waits
+   // for the next clock tick to start. If not clocked, then the sequence will start immediately (i.e. the
+   // internal clock will be synced at the point where `resetIsRequested` is first true.
+   bool resetIsRequested = false;
+   // used to detect when `resetIsRequested` first becomes true (only used when no external clock)
+   dsp::BooleanTrigger detectResetTrigger;

    // used to track the clock (e.g. if external clock is not connected). NOTE: this clock
    // is defined _prior_ to any clock division/multiplication logic
@@ -430,7 +442,20 @@ struct Muxlicer : Module {
                internalClockLength = tapTime;
            }
            tapTime = 0;
-           internalClockProgress = 0;
+           internalClockProgress = 0.f;
+       }
+
+       // If we have an _internal_ clock, as soon as we get a reset signal (which can come from CV or various
+       // modes of the switch), we will immediately start the sequence and sync up all the various clocks.
+       if (detectResetTrigger.process(resetIsRequested) && !usingExternalClock) {
+           // reset various clocks
+           mainClockMultDiv.reset();
+           outputClockMultDiv.reset();
+           multiClock.reset(mainClockMultDiv.getEffectiveClockLength());
+           // prime the main clock to be ready to receive a clock tick
+           mainClockTrigger.state = false;
+           // set the internal clock to a value that will force a clock tick
+           internalClockProgress = internalClockLength;
        }
        tapTime += args.sampleTime;
        internalClockProgress += args.sampleTime;
@@ -451,20 +476,23 @@ struct Muxlicer : Module {
        // so we must use a BooleanTrigger on the divided/mult'd signal in order to detect rising edge / when to advance the sequence
        const bool dividedMultipliedClockPulseReceived = mainClockTrigger.process(mainClockMultDiv.process(args.sampleTime, clockPulseReceived));

-       // see https://vcvrack.com/manual/VoltageStandards#Timing
-       const bool resetGracePeriodActive = resetTimer.process(args.sampleTime);
+       // see https://vcvrack.com/manual/VoltageStandards#Timing (only relevant if using an external clock)
+       const bool resetGracePeriodActive = resetTimer.process(args.sampleTime) && usingExternalClock;

        if (dividedMultipliedClockPulseReceived) {
            if (isAddressInRunMode && !resetGracePeriodActive && playState != STATE_STOPPED) {
+
                runIndex++;
+
                if (runIndex >= SEQUENCE_LENGTH) {

+                   runIndex = 0;
+
                    // the sequence resets by placing the play head at the final step (so that the next clock pulse
                    // ticks over onto the first step) - so if we are on the final step _because_ we've reset,
-                   // then don't fire EOC
-                   if (playHeadHasReset) {
-                       playHeadHasReset = false;
-                       runIndex = 0;
+                   // then don't fire EOC, just clear the reset status
+                   if (resetIsRequested) {
+                       resetIsRequested = false;
                    }
                    // otherwise we've naturally arrived at the last step so do fire EOC etc
                    else {
@@ -474,12 +502,10 @@ struct Muxlicer : Module {
                        if (playState == STATE_PLAY_ONCE) {
                            playState = STATE_STOPPED;
                        }
-                       else {
-                           runIndex = 0;
-                       }
                    }
                }
            }
+
            multiClock.reset(mainClockMultDiv.getEffectiveClockLength());

            if (isAddressInRunMode) {
@@ -569,8 +595,8 @@ struct Muxlicer : Module {

        const bool resetPulseInReceived = resetTrigger.process(rescale(inputs[RESET_INPUT].getVoltage(), 0.1f, 2.f, 0.f, 1.f));
        if (resetPulseInReceived) {
-           playHeadHasReset = true;
-           runIndex = 8;
+           resetIsRequested = true;
+           runIndex = 7;

            if (playState == STATE_STOPPED) {
                playState = STATE_PLAY_ONCE;
@@ -583,19 +609,19 @@ struct Muxlicer : Module {
        const bool switchIsActive = params[PLAY_PARAM].getValue() != STATE_STOPPED;
        if (playStateTrigger.process(switchIsActive) && switchIsActive) {

-           // if we were stopped, check for activation (normal or one-shot)
+           // if we were stopped, check for activation (normal, up or one-shot, down)
            if (playState == STATE_STOPPED) {
                if (params[PLAY_PARAM].getValue() == STATE_PLAY) {
                    playState = STATE_PLAY;
                }
                else if (params[PLAY_PARAM].getValue() == STATE_PLAY_ONCE) {
                    playState = STATE_PLAY_ONCE;
-                   runIndex = 8;
-                   playHeadHasReset = true;
+                   runIndex = 7;
+                   resetIsRequested = true;
                }
            }
            // otherwise we are in play mode (and we've not just held onto the play switch),
-           // so check for stop or reset
+           // so check for stop (switch up) or reset (switch down)
            else {

                // top switch will stop
@@ -604,8 +630,8 @@ struct Muxlicer : Module {
                }
                // bottom will reset
                else if (params[PLAY_PARAM].getValue() == STATE_PLAY_ONCE) {
-                   playHeadHasReset = true;
-                   runIndex = 8;
+                   resetIsRequested = true;
+                   runIndex = 7;
                }
            }
        }
-- 
2.31.0
hemmer commented 2 years ago

Pushed some further changes to address this.

One thing that might remain, but is more of a design thing - the switch always has output something on COMI/O, so after the sequence resets on one of the Muxlicers it will output the signal of the first step (even though it's not running). If you want to chain COMIO outputs, rather than summing you can redirect them into a third switch (which can be another muxlicer!) which selects which COMIO to use, rather than summing as in your example:

image

ZwergNaseXXL commented 2 years ago

I just checked the new version, unfortunately it's not completely fixed. The extra low pitch is gone, the hickup between Mux 2 and Mux 1 is gone, but the hickup between Mux 1 and Mux 2 is still there. Note that I started the patch by starting Mux 1 in one-shot mode.

The purple gates in the scopes are the combined all-gates outputs, the red and blue trigger are the EOC signals.

grafik

hemmer commented 2 years ago

Not sure I understand. They work chained, EOC can be used to switch between gate outs. I'm starting with one shot.

selection muxs.vcvs.zip

image

ZwergNaseXXL commented 2 years ago

Sorry for the delay and the confusion. Much time has passed and I completely lost track of this. I just tried it again and got slightly different results than in my 2nd screenshot. Then I realized this was only because of the two internal clocks being out of sync. Works fine when I restart VCV or use an external clock. So I can confirm that this is fixed indeed.