Feature: Waveform loop lock #293

weavermedia · 2023-08-03T05:59:12Z

(Reimplementation of closed PR #214 with hopefully cleaner code)

Allows you to lock the sample loop points movements together in the waveform view.

Set sample LOOP START and LOOP END as usual
Hold LOOP START and tap LOOP END to engage loop lock [LOCK]
Move either loop point and they're locked together
SHIFT+TURN◀︎▶︎ clockwise to double the loop length [DOUB]
SHIFT+TURN◀︎▶︎ anti-clockwise to half the loop length [HALF]
Hold LOOP START and LOOP END again to release loop lock [FREE]

Usage notes

You can't double the length past the sample end point [CANT]
You can't half the length to less than 2 samples [CANT]
You can't scroll a loop outside of the sample start/end bounds

Todo

Set loopLocked by holding loopStart marker and tapping loopEnd marker
Allow doubling and halving the loop length with SHIFT+TURN◀︎▶︎
Prevent scrolling the loop outside the sample start and end points
Prevent doubling the loop longer than the sample end point
Prevent halving the loop to 0

Questions for the community

✅ ~~What's the Deluge UX way of using SCROLL ▲▼ for loop doubling/halving?~~ SHIFT+TURN◀︎▶︎
Should the loopLocked boolean be a permanent sample attribute or just live in the sample marker editor file?
✅ ~~Does this need a runtime feature setting?~~ No

Note the video shows outdated controls. Text here is up to date.

Deluge-SampleLoopLock.mp4

…aps loop end marker

…nd vice versa.

…ess than 2 samples.

m-m-adams · 2023-08-03T14:45:06Z

To maintain consistency with clip view I think doubling should go on shift+press <> and changing loop length should be on shift+turn <>. Neither of those are used in waveform edit view right now

…tical encoder action back to default.

weavermedia · 2023-08-03T15:41:52Z

To maintain consistency with clip view I think doubling should go on shift+press <> and changing loop length should be on shift+turn <>. Neither of those are used in waveform edit view right now

I moved doubling and halving the loop length to SHIFT+TURN◀︎▶︎ and returned the vertical encoder action back to default.

I'm not sure about using SHIFT+PRESS◀︎▶︎ for doubling the loop length. It is indeed consistent with the clip view action but since there's no clip view equivalent for halving the length it might be confusing.

Do you think changing loop length is OK just on SHIFT+TURN◀︎▶︎?

soymonitus · 2023-08-03T19:45:40Z

This feature is awesome!

m-m-adams · 2023-08-03T20:43:31Z

I think just turning to adjust is fine, having the double there but not a half also seems fine to me since it's like that for clips anyway

weavermedia · 2023-08-03T20:46:56Z

@m-m-adams Do you think this feature needs a runtime setting? It doesn't change any default behavior, just adds some new behavior. It will be documented, if merged, but not sure it needs a runtime feature setting.

m-m-adams · 2023-08-03T23:21:46Z

Nope! Just extending existing behaviour in a way that complements the existing UI

Awesome feature

weavermedia · 2023-08-04T00:18:20Z

Documentation added to Community Features doc. I think is ready to go now. Can I get a review @m-m-adams?

src/deluge/gui/ui/sample_marker_editor.cpp

m-m-adams · 2023-08-04T00:23:08Z

src/deluge/gui/ui/sample_marker_editor.cpp

+			int loopEnd = getCurrentMultisampleRange()->sampleHolder.loopEndPos;
+
+			if (loopEnd + loopLength < end) {
+				loopLength = loopLength * 2;


is this right? This reads like it's doubling/halfing the loop length when you turn the knob instead of incrementing it which is not how I read the conversation

Yes, it's right. I did think we might be at different viewpoints here. The intention is that you'll set a loop length related to the tempo of the sample or track then doubling or halving the loop length will keep it rhythmically intact. Being able to arbitrarily adjust the loop length with SHIFT+TURN<> is fine but I'd still want a want to exactly double or half it. It's a magic way to find cool and unusual looping sections within samples.

Maybe SHIFT+PUSH+TURN<> for doubling/halving? Then use SHIFT+TURN<> for continuous change.

m-m-adams · 2023-08-04T00:27:29Z

Documentation/community_features.md

@@ -67,6 +67,9 @@ Synchronization modes accessible through the "LFO SYNC" shortcut.
 ### Audio Clip View
 - ([#141]) Holding the vertical encoder down while turning the horizontal encoder will shift the clip along the underlying audio file, similar to the same interface for instrument clips.

+### Sample Waveform View
+ - ([#293]) When a sample has loop start and loop end points set, holding down loop start and tapping loop end will lock the loop points together. Moving one will move the other, keeping them the same distance apart. Use the same process to unlock the loop points. Use SHIFT+TURN<> to double or half the loop length.


I think it should be shift+turn to edit loop length, shift+press to double. It feels weird having this little control over length if the loop is longer than a few samples

See my response above about doubling/halving.

m-m-adams · 2023-08-04T00:46:31Z

Mostly good, but doubling and halfing by turning the knob feels wrong with longer loops where that adjustment range is really coarse. It also doesn't match the clip loop length adjustment. I think it should adjust length by one pad at the current zoom level, and if doubling length feels necessary for quick adjustments then put that on shift+push <>.

Trying to move the loop by selecting a new start/end pad that would put loop end past sample end/start also feels weird. I think that should temporarily shorten the loop by truncating at the start/end position without changing the loop length

weavermedia · 2023-08-04T01:00:39Z

The intention behind the feature is that you would set a loop length which is related to the tempo of the sample, or the song, and then move the loop around to find cool looping sections. Doubling and halving the loop length keeps the loop rhythmically intact so both of those features are crucial.

As such I wouldn't want to change the loop length without user intent either. If I got a good loop going then the loop length changed because I tried to move the loop out of bounds it would kill the flow. I'd rather just see a CANT message.

We can use SHIFT+TURN<> for continuous loop length adjustment and SHIFT+PUSH+TURN<> for doubling/halving.

m-m-adams · 2023-08-04T01:32:05Z

Ahh ok that makes more sense to me then, I was playing with it more like a granular synth where the loop length wouldn't be linked to tempo (also made me really wish we could modulate start position!)

What about keeping your current doubling/halfing on the turn but allowing the start/end position to be moved freely by holding the marker and turning <> (like moving a clip in the arranger)? That would keep the tempo matching you want while also allowing the length of a loop to be adjusted more finely if needed

sapphire-arches · 2023-08-04T07:00:05Z

src/deluge/gui/ui/sample_marker_editor.cpp

@@ -51,6 +51,9 @@ extern "C" {

 const uint8_t zeroes[] = {0, 0, 0, 0, 0, 0, 0, 0};

+int loopLength = 0;
+bool loopLocked = false;


these would probably benefit from being members of SampleMarkerEditor rather than globals

sichtbeton · 2023-08-07T12:52:49Z

could this get in before the proposed feature cutoff this week for a comunity release? it looks ready but no pressure. it's a very cool feature.

weavermedia · 2023-08-08T01:56:32Z

@sichtbeton Still trying to figure out how to assign the controls for fine-adjusting the loop length and doubling/halving the loop length. Any input is appreciated 😀

sichtbeton · 2023-08-08T13:13:46Z

@sichtbeton Still trying to figure out how to assign the controls for fine-adjusting the loop length and doubling/halving the loop length. Any input is appreciated 😀

turn <> to shift the loop
shift and turn <> to double or halven (analog to lenght of a clip)

or does this collide in any way with other functions?

m-m-adams · 2023-08-11T20:23:47Z

Looking back at this I think fine adjustments could be a separate PR. Unlocking the loop, adjusting one end, and relocking is ok for now, we should just add this as a separate issue for our pre release bug fixes

m-m-adams

Future work: fine editing

@bobtwinkles suggestion to move the global to class member would be nice but there's only one loop editor at a time

The long term will likely involve moving that parameter into the sample synth as part of a bigger change to allow modulating the start/end/loop positions so I'm not particularly concerned about it's current location

weavermedia · 2023-08-12T18:05:29Z

@m-m-adams What about the doubling and halving of the loop length? Would you rather I remove it from this PR and add as another follow up? Or leave it in here as-is? Currently using SHIIFT+TURN<> as in the first post.

weavermedia · 2023-08-12T18:06:12Z

I'll fix the merge conflicts today.

m-m-adams · 2023-08-12T19:10:56Z

The doubling and halting is fine, it's analogous to changing clip length with the same controls and just locked to sizes that are convenient for loop samples

weavermedia added 8 commits July 31, 2023 19:46

Add variables int loopLength and bool loopLocked

8dee117

Set loopLength and loopLocked when user holds loop start marker and t…

0b74c59

…aps loop end marker

Use loopLength variable to set loop end from a change to loop start a…

8147e84

…nd vice versa.

Allow doubling and halving of the loop length by turning Y_ENC

88d40c2

Prevent default Y_ENC action for instrument view.

ca7a173

Prevent loop length doubling past the loop end marker or halving to l…

86e0347

…ess than 2 samples.

Prevent scrolling locked loop out of sample start/end bounds.

85ef194

Merge branch 'community' into waveform-loop-lock

6c8e205

weavermedia mentioned this pull request Aug 3, 2023

[DRAFT] Sample loop lock #214

Closed

6 tasks

Move doubling and halving loop length to SHIFT+SCROLL◀︎▶︎. Return ver…

96f49cd

…tical encoder action back to default.

Add documentation to Community Features doc.

460a504

m-m-adams requested changes Aug 4, 2023

View reviewed changes

sapphire-arches reviewed Aug 4, 2023

View reviewed changes

weavermedia added 3 commits August 7, 2023 18:42

Address case where loop boundary could be equal to endPosd or startPos

1b6dc94

Show better popup text on OLED

7317e1d

Merge branch 'community' into waveform-loop-lock

2db7410

m-m-adams approved these changes Aug 11, 2023

View reviewed changes

m-m-adams added the merge conflict github reports a merge conflict that should be resolved. label Aug 12, 2023

Merge community

981ddb5

m-m-adams removed the merge conflict github reports a merge conflict that should be resolved. label Aug 12, 2023

weavermedia changed the title ~~Waveform loop lock~~ Feature: Waveform loop lock Aug 13, 2023

jamiefaye added this pull request to the merge queue Aug 14, 2023

Merged via the queue into SynthstromAudible:community with commit 902fb0b Aug 14, 2023
2 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Feature: Waveform loop lock #293

Feature: Waveform loop lock #293

weavermedia commented Aug 3, 2023 •

edited

m-m-adams commented Aug 3, 2023

weavermedia commented Aug 3, 2023

soymonitus commented Aug 3, 2023

m-m-adams commented Aug 3, 2023

weavermedia commented Aug 3, 2023

m-m-adams commented Aug 3, 2023

weavermedia commented Aug 4, 2023

m-m-adams Aug 4, 2023

weavermedia Aug 4, 2023 •

edited

m-m-adams Aug 4, 2023

weavermedia Aug 4, 2023

m-m-adams commented Aug 4, 2023

weavermedia commented Aug 4, 2023 •

edited

m-m-adams commented Aug 4, 2023

sapphire-arches Aug 4, 2023

sichtbeton commented Aug 7, 2023

weavermedia commented Aug 8, 2023

sichtbeton commented Aug 8, 2023

m-m-adams commented Aug 11, 2023

m-m-adams left a comment

weavermedia commented Aug 12, 2023

weavermedia commented Aug 12, 2023

m-m-adams commented Aug 12, 2023

Feature: Waveform loop lock #293

Feature: Waveform loop lock #293

Conversation

weavermedia commented Aug 3, 2023 • edited

m-m-adams commented Aug 3, 2023

weavermedia commented Aug 3, 2023

soymonitus commented Aug 3, 2023

m-m-adams commented Aug 3, 2023

weavermedia commented Aug 3, 2023

m-m-adams commented Aug 3, 2023

weavermedia commented Aug 4, 2023

m-m-adams Aug 4, 2023

Choose a reason for hiding this comment

weavermedia Aug 4, 2023 • edited

Choose a reason for hiding this comment

m-m-adams Aug 4, 2023

Choose a reason for hiding this comment

weavermedia Aug 4, 2023

Choose a reason for hiding this comment

m-m-adams commented Aug 4, 2023

weavermedia commented Aug 4, 2023 • edited

m-m-adams commented Aug 4, 2023

sapphire-arches Aug 4, 2023

Choose a reason for hiding this comment

sichtbeton commented Aug 7, 2023

weavermedia commented Aug 8, 2023

sichtbeton commented Aug 8, 2023

m-m-adams commented Aug 11, 2023

m-m-adams left a comment

Choose a reason for hiding this comment

weavermedia commented Aug 12, 2023

weavermedia commented Aug 12, 2023

m-m-adams commented Aug 12, 2023

weavermedia commented Aug 3, 2023 •

edited

weavermedia Aug 4, 2023 •

edited

weavermedia commented Aug 4, 2023 •

edited