strudel icon indicating copy to clipboard operation
strudel copied to clipboard

Fix legato and duration

Open yaxu opened this issue 3 years ago • 10 comments

The current legato implementation manipulates haps in a way that can lose data.

A fix would be to copy tidal's behaviour - store legato in a normal value key and then deal with it in the scheduler.

yaxu avatar May 08 '22 15:05 yaxu

Isn‘t this problem of loosing data also related to all other methods that use withHapTime? Iirc the problem was there is no way of knowing if an event part ever had an onset or not. But maybe there was another problem I don’t remember anymore. Generally, I like the possibility of controlling the hap length directly, because we then don’t need to react to that param for every output separately

felixroos avatar May 10 '22 10:05 felixroos

It's not a problem with others that I know of, because where withHapTime is expanded, withQueryTime is reduced by the same factor (and vice-versa). The particular problem with this approach to legato is that we don't know by how much the query time should be expanded by.

We can still solve this once for everything though, as a simple pre-processing step that changes the duration of events before calling the callback. The only problem is that superdirt will do this a second time if it sees the legato parameter. So we need to remove it or reset it to 1 when the durations are changed.

yaxu avatar May 10 '22 10:05 yaxu

What are the consequences of not reducing withQueryTime by the same factor withHapTime is expanded? Can you think of an example problem this might cause? The thing with the pre processing step sounds like a good idea

felixroos avatar May 22 '22 21:05 felixroos

The consequence with e.g. fast/slow would be doubling up or missing events.

The consequence in this case is events appearing or not depending on where you query:

A part from 0 > 2:

slowcat(1,silence).legato(2).queryArc(0,1)

Disappeared:

slowcat(1,silence).legato(2).queryArc(1,2)

This will result in glitches and inconsistent results, especially when it comes to combining it with other patterns.

We can't solve this by manipulating query time because we don't know how much to expand the query by.

yaxu avatar Jun 28 '22 08:06 yaxu

The duration method has the same problem at the moment. This is kind of possible to fix for duration, as for query timespan (a,b), you could query (a-duration,b), to be sure of finding events that are expanded into the query timespan.. That would mess with the values returned from continuous events though.

yaxu avatar Aug 11 '22 13:08 yaxu

I've fixed duration in https://github.com/tidalcycles/strudel/commit/961a80dfe7ec912da9e365b2dc9eff2d1ba84d1f, but this breaks the 'jemblung' tune, as it has a pattern of the form "12".duration(0.05).add(12), that doesn't work when duration is promoted from metadata to data.

Perhaps it would be best to change all examples to use explicit parameter names for patterns, to make things clearer for beginners.

yaxu avatar Aug 15 '22 13:08 yaxu

Hm the tune sounds fine now I've merged from main. The test still fails, needs looking closely at the parts and wholes of the mismatching events to work out why..

Am I right that 'expected' is what we generate and 'received' is from the snapshot? If so, I think these should be swapped for clarity.

yaxu avatar Aug 16 '22 06:08 yaxu

are already running the tests with vitest ? it does expect(haps).toMatchSnapshot(); , where it should be flipped around. "expected" is whats in the tunes.test.mjs.snap file and "received" is what the code generates

felixroos avatar Aug 16 '22 06:08 felixroos

For reference: https://strudel.tidalcycles.org?iw5ossp4Sti1 the only effect duration has here is that the bass is short, it hasn't even an effect on the kick (although it might affect the event structure)

felixroos avatar Aug 16 '22 07:08 felixroos

I see thanks, well maybe it's not important to understand the exact difference in haps, as long as it sounds the same!

yaxu avatar Aug 16 '22 12:08 yaxu

just hit an actual bug related using legato. For some reason, it does not create a scheduling error when the .s('triangle') is uncommented..

felixroos avatar Dec 02 '22 20:12 felixroos

Legato causes missed events here:

s("bd*8").legato(0.5).rev()

I think we do need to turn legato into a normal param and deal with it in the scheduler

yaxu avatar Jan 01 '23 13:01 yaxu

Aside from the representation difference legato works a bit differently in superdirt for samples

legato 0.5 in haskell is .legato(0.5).clip(1) in strudel

Is clip redundant, i.e. could we just turn sample clipping on if legato is specified? I like the name 'clip' but it could just be an alias for legato then.

yaxu avatar Jan 16 '23 08:01 yaxu

yep clip(1) is what legato(1) does in tidal i think.. but clip takes just a flag atm, but it would make sense to make it behave as legato does. I think the migration of legato will be much easier when https://github.com/tidalcycles/strudel/issues/288 is fixed first, because it allows using legato with plain values (also color and velocity)

felixroos avatar Jan 16 '23 22:01 felixroos

the nudge param falls into similar territory. It could make sense to handle these properties somewhere between pattern and output. right now they are in the pattern (illegal). They seem to be pretty global / output dependent so it feels a bit wrong to handle them at multiple places separately. so either they are handled automatically somewhere in the scheduler, or there is some function lets call it handlePostControls , that can be called from each output to handle those params. The latter option seems cleaner to me, because the scheduler currently does not care about the hap.value , which would be nice if it stayed that way

felixroos avatar Apr 14 '23 08:04 felixroos

yep clip(1) is what legato(1) does in tidal i think.. but clip takes just a flag atm, but it would make sense to make it behave as legato does.

implemented in https://github.com/tidalcycles/strudel/pull/598

felixroos avatar Jun 12 '23 20:06 felixroos

hello from the future, https://github.com/tidalcycles/strudel/pull/965 should get rid of this mess once and for all :)

legato is now just a synonym of clip + duration is also just a control that sets the duration in cycles. The hap.duration getter returns the duration in cycles based on value.duration and value.clip.

felixroos avatar Feb 29 '24 03:02 felixroos