Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Stretch v2 #229

Merged
merged 22 commits into from
Aug 20, 2021
Merged

Stretch v2 #229

merged 22 commits into from
Aug 20, 2021

Conversation

bgold-cosmos
Copy link
Contributor

Update of #149

telephon and others added 15 commits April 8, 2020 11:13
Shall we do a new release? Yes we should!
@bgold-cosmos
Copy link
Contributor Author

One comment on this is that the timestretch algorithm used here doesn't really work with the workaround for issue #196. So if you're timestretching long samples, it may sound weird, but I'm not sure when it becomes noticeable since the timestretching itself is going to sound weird.

The timestretch algorithm unfortunately does not play well with the workaround for dealing with long samples.
Wasn't handling multiple channels correctly, sounds much better now
@yaxu
Copy link
Collaborator

yaxu commented Feb 3, 2021

I'm just trying to getting this running, I think it's just the timescale parameter that needs sending, right?

Maybe we could call that parameter stretch instead? Tidal already has a function with that name, but it seems to only be a supporting function for fit'.

@yaxu
Copy link
Collaborator

yaxu commented Feb 3, 2021

I've got it working now, sounding nice and gnarly! It would be nice to have a version that resizes it relative to the event duration (the delta param). Not sure what that could be called, maybe pack?

@bgold-cosmos
Copy link
Contributor Author

The parameter was originally called stretch and I can see in the commit history when it changed, but I don't remember at all the reason why. I have a vague recollection of some discussion about possible name collisions in SuperCollider.

By your second comment, you mean a version that automatically stretches the sample so that it fills the event duration?

@yaxu
Copy link
Collaborator

yaxu commented Feb 4, 2021

Ah OK, maybe @telephon remembers about the name collision. We could still call the parameter stretch on the tidal side, although it's nice to have things named the same everywhere.

And yes that's right. A bit like legato. In fact it could detect if legato is switched on, and if so change its behaviour so that the eventual duration in seconds is legato * stretch * delta.

@telephon
Copy link
Contributor

telephon commented Feb 5, 2021

The parameter stretch is used in the event system of sclang for a different purpose, so we might want to avoid it.

@telephon
Copy link
Contributor

telephon commented Feb 5, 2021

How should these parameters work for synths?

@yaxu
Copy link
Collaborator

yaxu commented Feb 5, 2021

Perhaps we call it timestretch then, with the alias stretch in tidal.

For synths I guess we'd just multiply legato up using stretch and not use it otherwise? As synths are already stretchy by nature.

@telephon
Copy link
Contributor

telephon commented Feb 5, 2021

For synths I guess we'd just multiply legato up using stretch and not use it otherwise? As synths are already stretchy by nature.

yes. synths may still use the timestretch argument if they like, to add some inexplainably weird distortion :)

@yaxu
Copy link
Collaborator

yaxu commented Feb 5, 2021

Oh I hadn't realised that synths could actually be timestretched with the same effect! That's strange
edit Oh re-reading I think I misunderstood. You're saying individual synths can add timestretch distortion? Heh

@telephon
Copy link
Contributor

telephon commented Feb 5, 2021

You're saying individual synths can add timestretch distortion? Heh

yes, they could do anything with that parameter, so that when you mix synths and samples in one pattern, they have some sort of analogy to each other.

Copy link
Contributor

@telephon telephon left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

just a few things, I think nothing complicated …

classes/DirtEvent.sc Outdated Show resolved Hide resolved
synths/core-synths.scd Outdated Show resolved Hide resolved
synths/core-synths.scd Outdated Show resolved Hide resolved
synths/core-synths.scd Show resolved Hide resolved
defer timescale recalculation until sustain is almost done
Clarify how the window size is chosen, and add a new parameter to make it tweakable
Copy link
Contributor

@telephon telephon left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you! Just some minor things …

synths/core-modules.scd Outdated Show resolved Hide resolved
synths/core-synths.scd Show resolved Hide resolved
// OK for sounds of moderate length (around 0.5 to several seconds long).
// But this is also scaled below to be a bit smaller/larger based on desired playback rate.
// If it's still not good, you can use the "timescalewin" parameter to multiply the window size
windowSize = timescale.clip(0.1,2) * 0.05 * BufSampleRate.ir(bufnum) * timescalewin;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  1. you are clipping the value here, but in line 124 you are using it unclipped. Won't this lead to problems? Also in the line if (~timescale.notNil) { sustain = sustain * ~timescale }; above, you are using the full sustain, while in the synthdef you are clipping it, while the sustain is still the same length.
  2. Further down, there is a line:
windowIndex = phase - (phase.div(timescaleStep) - [1.0, 0.0] * timescaleStep);

Are you sure this is correct? Note that there is no operator precedence in supercollider. I'm unsure right now.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

windowSize and timescale can be varied completely independently and the algorithm still works, but may sound odd for certain combinations. I'm really clipping on windowSize values, but a "good" windowSize tends to scale somewhat with the length of the effect. I'm not sure what you mean about the sustain.

The windowIndex calculation is correct, but unnecessarily obscure. I added a variable to clarify it a bit (and it might save a repeated computation).

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure what you mean about the sustain.

Let's say e.g. you are setting # timescale 0.09 for a sample that is 1 second long. Then your playback synth will have a total duration (=sustain) of 0.09 seconds. But your window will be (0.1 * 0.05). Or, when you set # timescale 3, your synth will be 3 seconds, but the window will be (2 * 0.05). But also, in line 124 (timescaleStep = windowSize / timescale; e.g. for a very large timescale, the timescalestep will be very small, but the window won't get smaller because of the clipping.

I am not sure if all that is a problem, it just looks like a source of hard to trace errors. It might be easier to reason about when you limit the stretching before everything else?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry to have left this dangling. I meant that because timescaleStep is clipped, the overlaps will become larger when timescale < 0.1. Could you check if that is really correct? If that is what you want, it might be good to make a comment.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'll take a look! I have since noticed that a combo of frequency shift (fshift) and speed modification works pretty well for smaller amounts of stretching.

@yaxu yaxu mentioned this pull request Aug 6, 2021
@telephon
Copy link
Contributor

telephon commented Aug 18, 2021

Let me know if you want to add more polish.

@telephon telephon merged commit 6878cde into musikinformatik:develop Aug 20, 2021
ndr-brt pushed a commit to ndr-brt/SuperDirt that referenced this pull request Jan 3, 2022
First implementation for time stretching in tidal.
bgold-cosmos, Cc-authored-by: Julian Rohrhuber
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants