Add slider ranges for nonlooping green segments

[w-winter]

Adds extended and contracted frequency ranges on the Time/Level slider for non-looping green segments.

• Button+slider in the upper 25% region switches the slide pot's minimum value to 8V and its maximum value to "15.84V", so that without added CV this mode gives access to envelope stages ranging from 16 seconds to ~13.4 minutes.*
• Button+slider in the bottom 25% region switches the slide pot's minimum value to 0V and its maximum value to 5.3V, so that without subtracted CV this mode gives access to envelope stages ranging from 1 millisecond to ~2.2 seconds, with more pot precision for values in that range
• Button+slider in the middle gives Stages' default slider range (1 millisecond to 16 seconds)

*Allowing the slider to reach the full internal 16V range causes the segment to get stuck midway, but it can use 99% of that range without issues.

More on the slow range:

• With no external CV and the slider at maximum height, the segment duration is ~13.4 minutes
• With no external CV and the slider at 75% height, the segment duration is ~8.5 minutes
• With no external CV and the slider at 50% height, the segment duration is ~3 minutes
• With no external CV and the slider at 25% height, the segment duration is ~34 seconds
• With no external CV and the slider at minimum height, the segment duration is 16 seconds

[qiemem]

Also, I played around with the max time, and found that at the 16v equiv, the ramp actually gets stuck midway through (this is on the bipolar branch, not your PR):

(the bumps in the line there are me bumping cables combined with my shitty DIY job...)

As you can see, the ramp visibly flatlines after 5 minutes or so. I let it go in the background for a while and it never dropped any further. So that would explain seeing >18minute ramp times. I'll need to check if stock shows the same behavior.

[w-winter]

You're right about the stalling! It behaves well at the equivalent of 15.84V, so I propose scaling the slide pot to 98% of its 8V in the slow range (done via the last commit, and has been tested). With that scaling, you get a segment of ~13.4 minutes that fully completes when the slider is at full height.

I don't see why a UI-accessible maximum stage time of 2 mins would be more useful per se. The A-140 is a simple analog ADSR, while Stages is a complex digital function generator that shares a niche with modules like Zadar, Ornament and Crime's Piqued, and Quadigy that offer 20+ minute stage times straight from the UI without any CV added. And even the analog Mini-Slew by WMD/SSF does ~30 min stages without CV. >2min stages are not only useful for drone work, but also plenty of other applications -- for instance, a slow ramp triggered at the start of a composition that gradually modulates some voice's timbre or frequency, or scrubs through a recording buffer on a sampler or CV recorder. It's the latter use case (a ramp that can drive the phase of my Planar2's and Morphagene's recordings over the course of 10+ minutes) that motivated me to suggest this feature, and it'd kinda defeat the purpose if I still had to patch in external CV.

My personal preference would be to offer the 13 min maximum at full slider height and to document what stage times are yielded by the 75%, 50%, and 25% positions without CV added; then folks can make use of the full range.

That said, I'm of course happy to keep building my Stages from my own fork if these parameters aren't ideal for most other people's use cases in the slower range. Maybe we should collect input on Muffs?

[qiemem]

Ah, that makes total sense. I'd been mostly worried about slight bumps causing huge changes in time, but as you say, super-slow swells are kind of the point. Have you tried using it for the sample scrubbing you describe? Has the precision been okay there?

Anyway, code looks great. I'll test it out today. Thanks again!

[w-winter]

Have you tried using it for the sample scrubbing you describe? Has the precision been okay there?

Yep ! It's working well for me.

[w-winter]

Another option to consider, if we're not attached to the faster range (with the 2.2 second ceiling):

• Button+slider @ bottom = default range (1ms => 16s)
• Button+slider @ center = "mid" range (16s => 2min)
• Button+slider @ top = "long" range (2min => 13.4min)

What do you think?

[qiemem]

Tested and works great. I think the 2.2 seconds feels perfect. The midpoint on the long time seemed to be about 3 minutes, which I think probably works pretty well.

The one thing I noticed: when you switch to looping in a group, you lose your ramp range. This is by design for LFO range, but feels weird in this case. The code for this is here: https://github.com/qiemem/eurorack/blob/f1ec0d85be3a4916a408513316eae65f5140c9fe/stages/chain_state.cc#L630

I think only resetting the range bits if request_.argument[0] (group start) is different than request_.argument[3] (group end) should do the trick (along with checking if the loop bit actually actually changed, of course). If they're the the same, it will be changing between an LFO and ramp, so should clear.

[w-winter]

The one thing I noticed: when you switch to looping in a group, you lose your ramp range. This is by design for LFO range, but feels weird in this case. The code for this is here:

https://github.com/qiemem/eurorack/blob/f1ec0d85be3a4916a408513316eae65f5140c9fe/stages/chain_state.cc#L630

I think only resetting the range bits if request_.argument[0] (group start) is different than request_.argument[3] (group end) should do the trick (along with checking if the loop bit actually actually changed, of course). If they're the the same, it will be changing between an LFO and ramp, so should clear.

Good catch, I can take a look at that tomorrow or Thursday.

[w-winter]

Gave if (new_loop_bit != loop_bit && request_.argument[0] != request_.argument[3]) { a try at https://github.com/qiemem/eurorack/blob/f1ec0d85be3a4916a408513316eae65f5140c9fe/stages/chain_state.cc#L629

Ramp range is retained when switching in and out of looping in a group, but not for the first and last segments in the group - only the ones in between.

[qiemem]

Ack, I meant to say if the group start and group end are the same, not different. Confirmed that it works with if (new_loop_bit != loop_bit && request_.argument[0] == request_.argument[3]) {. Feel free to update the PR or I can just add it when I merge.

Would also love a way to indicate the state a ramp is in when at rest, as I do find I forget, but could add it later if we think of something.

I must say, I've really been enjoying this PR. The precision on the 2.2s is really nice and the 13 min is so great for creating slow evolution.

[w-winter]

Ack, I meant to say if the group start and group end are the same, not different. Confirmed that it works with if (new_loop_bit != loop_bit && request_.argument[0] == request_.argument[3]) {. Feel free to update the PR or I can just add it when I merge.

Ah, that works for me too. Added.

Would also love a way to indicate the state a ramp is in when at rest, as I do find I forget, but could add it later if we think of something.

I think it would be best to stick to the channel LED, as I agree with you that making the slider LED indicate anything other than segment activity would be confusing. We can only work with LED brightness and LED color (green/orange/red), right? I think introducing other colors would also be confusing at this point, so that leaves a different sort of brightness animation that would be easily distinguishable from the looping pattern. Maybe a ramp-shaped one (rather than a sine-shaped one) that plays (at different respective speeds) while the segment is in either the "slow" or "fast" range, but doesn't play when the segment is in the default range. This sounds complicated to implement, but maybe the animation would only play while the slider is being moved (e.g., moving the slider causes one cycle of the ramp animation to play, but each detected movement doesn't reset the animation, so you can get a clear read on the slider range). If we go the latter route, there could be three different animation speeds.

I must say, I've really been enjoying this PR. The precision on the 2.2s is really nice and the 13 min is so great for creating slow evolution.

Happy to hear you're enjoying it!

[qiemem]

Awesome, thanks for adding that!

We can only work with LED brightness and LED color (green/orange/red), right?

Correct. And orange is just the led rapidly flicking between red and green. Emilie has nicely abstracted it out to a RGB representation, but the B doesn't do anything, and under the hood, R and G control amount of time spent on each color.

Maybe a ramp-shaped one (rather than a sine-shaped one) that plays (at different respective speeds) while the segment is in either the "slow" or "fast" range, but doesn't play when the segment is in the default range.

This could work quite well, though not sure how easy it will be to distinguish from looping.

This sounds complicated to implement, but maybe the animation would only play while the slider is being moved (e.g., moving the slider causes one cycle of the ramp animation to play, but each detected movement doesn't reset the animation, so you can get a clear read on the slider range). If we go the latter route, there could be three different animation speeds.

That does sound complicated. Movement detection is a pain given the timescale and natural noise of the slider. You'd have to do something like a moving average or LPF of velocity and set a threshold. Getting those thresholds would be be a pain too. I think I prefer the first idea as both easier to implement and more useful (since it let's you see state at rest).

If you're interested in playing around with LED representation, you can find LED code here. Specifically, this code handles the different LFO speeds: https://github.com/qiemem/eurorack/blob/bipolar/stages/ui.cc#L330

That said, I'm happy to merge at this point. The LED representation would be nice to have, but I don't think it's necessary for this to make it in. Up to you.

[qiemem]

Oh, just a quick thought: In groups, you can have segments that are the ends loops as well as ramps with non-default ranges. In that case, perhaps just multiplying the brightnesses would be fine, especially if the ramp cycle has a longer period than the LFO cycle. That is, the LFO represention would oscillate between dimmer and dimmer values as the the ramp representation ramps down. That does complicate things however.

[w-winter]

Oh, just a quick thought: In groups, you can have segments that are the ends loops as well as ramps with non-default ranges. In that case, perhaps just multiplying the brightnesses would be fine, especially if the ramp cycle has a longer period than the LFO cycle. That is, the LFO represention would oscillate between dimmer and dimmer values as the the ramp representation ramps down. That does complicate things however.

That's a good idea, but I think it would be nicer if we could get a state representation at rest outside of looping groups.

See https://github.com/qiemem/eurorack/pull/30/commits/3e95949f672ecffaf606e9fae2a97276aacbedc9 for my proposal to extend FadePattern like this:

inline uint8_t Ui::FadePattern(uint8_t shift, uint8_t phase, bool ramp) const {
  uint8_t x = system_clock.milliseconds() >> shift;
  x += phase;
  switch (ramp) {
    case 0: // produce a triangular pattern
      x &= 0x1f;
      return x <= 0x10 ? x : 0x1f - x;
    case 1: // produce a downward ramp pattern
      x &= 0x0f;
      return 0x0f - x;
  }
}

(case 1 yields a series like [15, 14, 13, 12 ... 3, 2, 1, 0, 15, 13, 14, 12 ...].)

And then:

        LedColor color = palette_[type];
        uint16_t* seg_config = settings_->mutable_state()->segment_configuration;
        if (settings_->in_seg_gen_mode()) {
          if (chain_state_->loop_status(i) == ChainState::LOOP_STATUS_SELF) {
            brightness = lfo_patterns[configuration >> 8 & 0x3]; // triangular brightness pattern
          } else if ((seg_config[i] & 0x3) == 0 && (seg_config[i] & 0x0300) == 0x0200) {
            brightness = FadePattern(8, 0x08, 1); // slow ramp brightness pattern
          } else if ((seg_config[i] & 0x3) == 0 && (seg_config[i] & 0x0300) == 0x0100) {
            brightness = FadePattern(5, 0x08, 1); // fast ramp brightness pattern
          } else {
            brightness = fade_patterns[0];
          }

I got this to compile, and it works and looks as expected on my Stages. I just think it would be better if it remained at max brightness for a few seconds seconds in between each fade-out, so that it still strongly suggests that you're in a non-looping segment. I'm not entirely sure how to pull that off though 😅

Could that be rewritten in a more performant way? Should I do another performance test with clocked LFOs after this addition?

[qiemem]

This is great! Thanks for your hard work on this!

but I think it would be nicer if we could get a state representation at rest outside of looping groups.

To clarify, I was just talking about how the representation would be be modified for looping segments in particular. With your code, there's no indication of the ends of looping groups. So that would need to get fixed by restoring:

          brightness = chain_state_->loop_status(i) == ChainState::LOOP_STATUS_SELF ?
            lfo_patterns[configuration >> 8 & 0x3] : fade_patterns[chain_state_->loop_status(i)];

and then converting the fade pattern from the ramp to a scalar for that brightness. See the color blind code for an example.

I just think it would be better if it remained at max brightness for a few seconds seconds in between each fade-out, so that it still strongly suggests that you're in a non-looping segment.

Oh, that's a great idea. The way to do it would be to only apply the fade transformation for half the time by looking at the bit that's just beyond the bytes you're using to do the fade. So, to modify FadePattern, that would be something like (untested):

inline uint8_t Ui::FadePattern(uint8_t shift, uint8_t phase, bool ramp) const {
  uint8_t x = system_clock.milliseconds() >> shift;
  x += phase;
  if (ramp) {
      // produce a triangular pattern
      x &= 0x1f;
      return x <= 0x10 ? x : 0x1f - x;
  } else {
    x &= 0x1f;
    return x > 0x0f ? 0x0f : 0x0f - x;
  }
}

Should I do another performance test with clocked LFOs after this addition?

Not a bad idea. That said, I don't totally understand the scheduling of the UI code. It seems to run on a different thread than the main code, but I don't have a good enough understanding of the STM32 architecture to be honest.

I'll put specific code comments inline.

[w-winter]

The delay works really well for the ramp LED patterns!

I fixed up the actual brightness definitions and tested loop groups with all the different segment types. The LEDs of the start and end segments oscillate as expected now. I also did another clocked LFO performance test, and the audio-rate sines within that are holding up just as fine as before.

What I'm not 100% sure about is the brightness scaling. I went with 0.15 for the slow decay range and 0.4 for the default decay range and full brightness for the fast decay range. The reasoning:

• 0.15 for slow: it feels more intuitive to me to present the slower channels as dimmer, and I found it too difficult to visually distinguish 0.2 from 0.4. 0.15 vs. 0.4 works, and anything lower than 0.15 just looks like an on/off blink.
• 0.4 for default: anything higher than 0.4 becomes very difficult to distinguish from full brightness when both are oscillating.
• 1.0 for fast: I chose to make the fast segment range the brightest, so that the brightness scaling is semantically consistent (dimmer = slower)

It might seem a little counterintuitive to not make the default range the brightest, since we are otherwise not modifying the LED indicators of default anythings. I personally think it would be worse if the brightness scaling were inconsistent in its meaning and consequently demanded memorization (where dimmest = slowest, and dimmer than default = fastest; or dimmest = fastest, and dimmer than default = slowest). But what bugs me a little is now we have the default range's pulsing LED scaled down in brightness when it's a loop group start or end, while in other cases the default range's LED just remains static and at full brightness (and only the slow and fast ranges have LED ramp fade patterns). It's probably not worth adding a delayed ramp pattern for the default range, though, because that would also be inconsistent with the rest of the UI and it would be a challenge to make the 3 cases visually distinguishable without making the slow and fast ranges' LED patterns too slow and fast again.

[qiemem]

This is great! You're right: the delay helps a ton.

What I'm not 100% sure about is the brightness scaling. I went with 0.15 for the slow decay range and 0.4 for the default decay range and full brightness for the fast decay range. The reasoning:

To clarify, when I was talking about scaling brightness, I meant that the fade pattern of the ramp should scale the brightness of the segment rather than hard set it. So, for beginning and end of loops, you'll get both the loop oscillations and the ramp down simultaneously. I think that would be something like this:

brightness = brightness * (ramp_patterns[1] + 1) >> 5;

This should be equivalent to b *= r / 31 without having to convert to float. Emilie does something similar for colorblind mode, but with different numbers, so I might be missing something here.

That goes after setting brightness for looping stuff. So something like this altogether:

if (chain_state_->loop_status(i) == ChainState::LOOP_STATUS_SELF) {
  brightness =  lfo_patterns[configuration >> 8 & 0x3];
} else {
  brightness = fade_patterns[chain_state_->loop_status(i)];
  if (type == 0) {
    brightness = brighness * (ramp_patterns[configuration >> 8 & 0x3] + 1) >> 5;
  }
}

That code is completely untested, so almost certainly has mistakes in it, but hopefully that gives you a sense.

[w-winter]

To clarify, when I was talking about scaling brightness, I meant that the fade pattern of the ramp should scale the brightness of the segment rather than hard set it. So, for beginning and end of loops, you'll get both the loop oscillations and the ramp down simultaneously.

Ahhh haha, I see now. I got it working with https://github.com/qiemem/eurorack/pull/30/commits/b2ba3975c8aa987e631f832e1e0377338fbe03fc . It works well with the slow ramp pattern. The faster ramp is a bit subtle, and is particularly subtle at loop start (because of unfortunate phase alignments). But it's still noticeably different enough from the regular loop start/end fade pattern.

By the way, I just tried out shifting the clock by 5 rather than 6 in the fast ramp pattern, and with the delay I think it works really well now. IMO it's not confuseable with any of the LFO patterns anymore, and it's closer to (but still slower than) the speed of the mid-range LFO pattern. (I think it's a bit weird if the fast-range ramp is indicated by a pattern that cycles a lot slower than the mid-range LFO.)

[qiemem]

This is great! Visuals look perfect and really enhance usability. Great job!

But it's still noticeably different enough from the regular loop start/end fade pattern.

Agreed. It's enough to remind you that something else is going on.

By the way, I just tried out shifting the clock by 5 rather than 6 in the fast ramp pattern, and with the delay I think it works really well now.

Agreed, 5 looks great with the delay.

I went ahead rechecked performance, and it does look like it took a significant hit. Here's my setup for testing performance:

Segment 6 is a bipolar clocked lfo with an audio rate square wave coming in. Segment 1-5 are also clocked lfos, with 5 getting it's clock from a square wave lfo (~2hz, but doesn't really matter). Then 5->4, 4->3, etc. in a chain. All set to square, which is important, as non-square clocks can cause a bit of stuttering which can confuse things. See https://forum.mutable-instruments.net/t/stages-sometimes-triggers-twice-on-downward-slopes/17587/2

When I do this on bipolar, I get a click on the clock, but not noise, and the 5 low-freq lfos stay in sync. On this branch, I get noise and the 5 go out of sync. I did a bit of investigation and it looks like the cv_slider modification did cause an initial drop, but, afaict, so did the UI changes, which surprised me. That actually might be a blessing in disguise: that code in general is pretty messy (I mean my code that was already there just to be clear), and there's a lot more potential for cleanup, which might ease the burden on cv_slider.

As for fixing, I think probably the best strategy is to first optimize the UI code. That would mean temporarily reverting the cv_slider stuff (just to get a clean read on the UI code), and then getting the UI code to a point where it looks good, then bring back cv_slider. The potential points of optimization I see in your code are:

• Eliminating ramp_patterns: I think this only helps if a bunch of segments are using it, which is unlikely. So calculating inline like:

          uint8_t speed = configuration >> 8 & 0x3;
...
              if (speed == 1) {
                brightness = brightness * (FadePattern(5, 0x08, true) + 1) >> 5;
              } else if (speed == 2) {
                brightness = brightness * (FadePattern(7, 0x08, true) + 1) >> 5;
              }

might be better. I tried that quickly, and it didn't get there, but might be a start.

• Splitting the new code in FadePattern off into, say, RampPattern, in order to remove the conditional. Conditionals are expensive.

I'll look at the rest of the UI code for possibility optimizations as well, though feel free to include any improvements you happen upon.

If that still doesn't do it, I'm afraid just brute force trying shit is the best I can offer :-/ As I think I've mentioned, I don't have a good enough understanding of STM32 to be able to predict much what will matter. Like, I've had a multiplication in the wrong place (like... not even in a loop) bring the module to it's knees... so yeah... it's a pain.

For cv_slider... just micro-optimizing as much as possible. If the LFO case is fast-trackable, that might help. Right now, there's two conditionals in that code path. Maybe reducing to one with an && would be good enough. The reason I focus on the LFO case is because that's the expensive case where things go wrong, so you want to save as many cpu cycles for running the LFOs as possible, even if that means wasting CPU cycles on other code paths.

One other possibility would be moving some the UI changing stuff to chain_state.cc and then moving the timing modifications into segment_generator.cc, where they will be much more isolated. I stumbled across a draft of a firmware today that incorporated the harmonic oscillator mode as a different type of segment group rather than a separate mode, and it did so by expanding the types of request packet the chaining code uses. I've been mostly syncing segment properties via ChannelState rather than RequestPacket. Looking at that code more, doing it with new types of RequestPackets might be a way more performant way to do it, and not as difficult as I had though. Still pretty complicated though, so let's hold off as a last resort. One extra advantage, though, is that then you can adjust time after the lookup tables, which means CV range expands too.

I'd say, work on this to whatever degree you have time and patience for, and I'll work on it as well.

[qiemem]

Hey just a heads up that I'm having some luck optimizing the switch polling. Feel free off to hold off doing much until I'm done with that as it may be enough.

[w-winter]

Hey just a heads up that I'm having some luck optimizing the switch polling. Feel free off to hold off doing much until I'm done with that as it may be enough.

That's great to hear! Just let me know. It was a bummer to hear that the UI stuff took a toll on performance, and I'm sorry that it's entailed so much additional work. Happy to help further after the switch polling tweaks are done.

[qiemem]

It was a bummer to hear that the UI stuff took a toll on performance, and I'm sorry that it's entailed so much additional work.

No worries at all! Actually turning out to be a good thing; a few simple optimizations of switch polling and the led code and it's working even better than before. I hadn't been able to establish that the UI code had any impact on performance previously, so hadn't focused my efforts there much.

Anyway, I'm almost done (it really wasn't much work at all... just wrapping a few loops in ifs). Maybe it would be best just to merge this and then I'll put my optimizations on top of it? How does that sound to you?

[w-winter]

Sounds good to me!

[qiemem]

Killer. I'm going go to squash and merge this btw, so you might want to:

git fetch <my-repo>
git reset --hard <my-repo>/bipolar

or whatever your preferred method is.

[qiemem]

Oh just realized this is on a branch in your repo, so I guess that makes things a bit simpler.

[qiemem]

Pushed my optimizations, so you might want to pull when you get a chance.